InstantExtended: assign all cacheable NTPs to the same process.

InstantExtended: assign all cacheable NTPs to the same process.

BUG=333397
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251537

[samarth, 2014-01-11 03:31:56]

PTAL

Thanks,
Samarth

[kmadhusu, 2014-01-13 17:12:18]

https://codereview.chromium.org/135383002/diff/1/chrome/browser/search/search.cc
File chrome/browser/search/search.cc (right):

https://codereview.chromium.org/135383002/diff/1/chrome/browser/search/search...
chrome/browser/search/search.cc:547:
replacements.SetHost(online_ntp_host.c_str(),
Do we need to change the host to "ntp" or something similar?

[samarth, 2014-02-03 22:47:11]

Sorry, dropped this on the floor.  PTAL.

Thanks,
Samarth

https://codereview.chromium.org/135383002/diff/1/chrome/browser/search/search.cc
File chrome/browser/search/search.cc (right):

https://codereview.chromium.org/135383002/diff/1/chrome/browser/search/search...
chrome/browser/search/search.cc:547:
replacements.SetHost(online_ntp_host.c_str(),
On 2014/01/13 17:12:19, kmadhusu wrote:
> Do we need to change the host to "ntp" or something similar?

This will make the host for the cacheable NTP be "online-ntp".  Is that what you
mean?

[kmadhusu, 2014-02-03 22:54:24]

https://codereview.chromium.org/135383002/diff/1/chrome/browser/search/search.cc
File chrome/browser/search/search.cc (right):

https://codereview.chromium.org/135383002/diff/1/chrome/browser/search/search...
chrome/browser/search/search.cc:547:
replacements.SetHost(online_ntp_host.c_str(),
On 2014/02/03 22:47:12, samarth wrote:
> On 2014/01/13 17:12:19, kmadhusu wrote:
> > Do we need to change the host to "ntp" or something similar?
> 
> This will make the host for the cacheable NTP be "online-ntp".  Is that what
you
> mean?

Yes. Do we need to change the host name to "cacheable-ntp"?

[samarth, 2014-02-03 23:32:19]

https://codereview.chromium.org/135383002/diff/1/chrome/browser/search/search.cc
File chrome/browser/search/search.cc (right):

https://codereview.chromium.org/135383002/diff/1/chrome/browser/search/search...
chrome/browser/search/search.cc:547:
replacements.SetHost(online_ntp_host.c_str(),
On 2014/02/03 22:54:25, kmadhusu wrote:
> On 2014/02/03 22:47:12, samarth wrote:
> > On 2014/01/13 17:12:19, kmadhusu wrote:
> > > Do we need to change the host to "ntp" or something similar?
> > 
> > This will make the host for the cacheable NTP be "online-ntp".  Is that what
> you
> > mean?
> 
> Yes. Do we need to change the host name to "cacheable-ntp"?

This name is not persisted anywhere, so it doesn't really matter either way as
long as all NTPs get the same host.  I'm not a huge fan of online-ntp; how about
"remote-ntp".

I also changed the check here to be more precise and only do the rewriting when
the search provider URL was used.

[kmadhusu, 2014-02-04 01:20:08]

lgtm

https://chromiumcodereview.appspot.com/135383002/diff/110001/chrome/browser/s...
File chrome/browser/search/search.cc (right):

https://chromiumcodereview.appspot.com/135383002/diff/110001/chrome/browser/s...
chrome/browser/search/search.cc:631: std::string
remote_ntp_host(chrome::kChromeSearchRemoteNtpHost);
nit: You can move this declaration to line 635.

[samarth, 2014-02-04 02:14:53]

https://chromiumcodereview.appspot.com/135383002/diff/110001/chrome/browser/s...
File chrome/browser/search/search.cc (right):

https://chromiumcodereview.appspot.com/135383002/diff/110001/chrome/browser/s...
chrome/browser/search/search.cc:631: std::string
remote_ntp_host(chrome::kChromeSearchRemoteNtpHost);
On 2014/02/04 01:20:09, kmadhusu wrote:
> nit: You can move this declaration to line 635.

Sadly that doesn't work.  Because of the stupid way the Replacements API works,
this strings needs to be in scope when ReplaceComponents below is called :(

[samarth, 2014-02-04 02:19:12]

The CQ bit was checked by samarth@chromium.org

[commit-bot: I haz the power, 2014-02-04 02:20:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/samarth@chromium.org/135383002/180001

[commit-bot: I haz the power, 2014-02-04 04:28:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 04:28:33]

Retried try job too often on ios_dbg_simulator for step(s) url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[samarth, 2014-02-14 23:48:10]

BTW, had to make some minor updates to tests in PS 6.

Thanks,
Samarth

[samarth, 2014-02-14 23:48:29]

The CQ bit was checked by samarth@chromium.org

[commit-bot: I haz the power, 2014-02-14 23:59:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/samarth@chromium.org/135383002/510001

[commit-bot: I haz the power, 2014-02-15 02:02:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-15 02:02:23]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[samarth, 2014-02-15 02:03:06]

The CQ bit was checked by samarth@chromium.org

[commit-bot: I haz the power, 2014-02-15 02:05:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/samarth@chromium.org/135383002/510001

[commit-bot: I haz the power, 2014-02-15 05:24:03]

Change committed as 251537