Instant: Make clicks on Most Visited items work.

Instant: Make clicks on Most Visited items work.

BUG=175768
R=dcblack@chromium.org
TEST=Clicking on Most Visited items in the NTP should work.

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

[sreeram, 2013-03-11 20:25:35]

@dcblack: Please review.

FYI: @dhollowa, @shishir, @gideonwald

[sreeram, 2013-03-11 20:35:07]

Patchset 2 adds a verbatim hack, to account for robertshield's upcoming change
which nukes searchbox.value when arrowing down
(https://codereview.chromium.org/12319108/).

This is all a big hack to make the dev cut. When Google moves to using separate
namespaces, we'll take it out (as the TODO indicates).

[David Black, 2013-03-11 20:44:46]

https://chromiumcodereview.appspot.com/12771013/diff/3001/chrome/renderer/sea...
File chrome/renderer/searchbox/searchbox_extension.cc (right):

https://chromiumcodereview.appspot.com/12771013/diff/3001/chrome/renderer/sea...
chrome/renderer/searchbox/searchbox_extension.cc:310: // Like
|SetQuerySuggestion| but uses a restricted ID to identify the text.
Please find-replace all instances of "restricted ID" in this and related files
with either "restricted NTP ID" or "restricted searchbox id" or something
similar so there's no confusion about which sort of rid any given function takes
in.

https://chromiumcodereview.appspot.com/12771013/diff/3001/chrome/renderer/sea...
chrome/renderer/searchbox/searchbox_extension.cc:759: if
(SearchBox::Get(render_view)->query().empty() &&
Oh geez this is hacky.  I guess it's going away as soon as we switch GWS over to
the new functions, but for now if you could include a comment describing what's
going on and why that would be good.

[sreeram, 2013-03-11 21:14:04]

https://codereview.chromium.org/12771013/diff/3001/chrome/renderer/searchbox/...
File chrome/renderer/searchbox/searchbox_extension.cc (right):

https://codereview.chromium.org/12771013/diff/3001/chrome/renderer/searchbox/...
chrome/renderer/searchbox/searchbox_extension.cc:310: // Like
|SetQuerySuggestion| but uses a restricted ID to identify the text.
On 2013/03/11 20:44:46, David Black wrote:
> Please find-replace all instances of "restricted ID" in this and related files
> with either "restricted NTP ID" or "restricted searchbox id" or something
> similar so there's no confusion about which sort of rid any given function
takes
> in.

Done.

https://codereview.chromium.org/12771013/diff/3001/chrome/renderer/searchbox/...
chrome/renderer/searchbox/searchbox_extension.cc:759: if
(SearchBox::Get(render_view)->query().empty() &&
On 2013/03/11 20:44:46, David Black wrote:
> Oh geez this is hacky.  I guess it's going away as soon as we switch GWS over
to
> the new functions, but for now if you could include a comment describing
what's
> going on and why that would be good.

Done.

[David Black, 2013-03-11 21:19:30]

https://chromiumcodereview.appspot.com/12771013/diff/8001/chrome/renderer/sea...
File chrome/renderer/searchbox/searchbox.h (right):

https://chromiumcodereview.appspot.com/12771013/diff/8001/chrome/renderer/sea...
chrome/renderer/searchbox/searchbox.h:88: int UrlToRestrictedId(const string16
url);
Function names too, please!  (They're more visible than param names.)

[sreeram, 2013-03-11 21:42:00]

https://chromiumcodereview.appspot.com/12771013/diff/8001/chrome/renderer/sea...
File chrome/renderer/searchbox/searchbox.h (right):

https://chromiumcodereview.appspot.com/12771013/diff/8001/chrome/renderer/sea...
chrome/renderer/searchbox/searchbox.h:88: int UrlToRestrictedId(const string16
url);
On 2013/03/11 21:19:30, David Black wrote:
> Function names too, please!  (They're more visible than param names.)

Done.

[David Black, 2013-03-11 22:01:32]

lgtm

[dhollowa, 2013-03-12 00:00:13]

In https://codereview.chromium.org/12732005 I've followed your naming
convensions on the Most Visited stuff.  Should make it easy for you to merge
that in.

[sreeram, 2013-03-12 00:04:05]

On 2013/03/12 00:00:13, dhollowa wrote:
> In https://codereview.chromium.org/12732005 I've followed your naming
> convensions on the Most Visited stuff.  Should make it easy for you to merge
> that in.

Thanks. Yes, I've been following that. Am waiting for your change to land so I
can rebase and land mine.

[sreeram, 2013-03-12 00:31:04]

Committed patchset #5 manually as r187425 (presubmit successful).