Retrieve NTP visisted items thumb URLs from the right frame

Retrieve NTP visisted items thumb URLs from the right frame

The SearchBox instance, which stores state used in NTP, only lives on the main
frame. In one instance where we looked this up we mistakenly looked in the child
frame.

BUG=681749

[tibell, 2017-01-18 03:13:47]

The CQ bit was checked by tibell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 03:14:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tibell, 2017-01-18 03:15:01]

tibell@chromium.org changed reviewers:
+ jochen@chromium.org, treib@chromium.org

[commit-bot: I haz the power, 2017-01-18 04:17:43]

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

[commit-bot: I haz the power, 2017-01-18 04:17:44]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2017-01-18 09:44:02]

lgtm

[jochen (gone - plz use gerrit), 2017-01-18 12:31:22]

can you add a test please?

[tibell, 2017-01-19 01:49:24]

On 2017/01/18 12:31:22, jochen (at summit until 19th) wrote:
> can you add a test please?

I played around a bit with adding a unittest/browsertest. I will have to punt on
this to the maintainers of the search code (Hi Marc!). I think we need and
end-to-end test that makes sure that the NTP renders correctly.

For context: I'm part of the Mojo team. We're working on converting all Chrome
IPCs to Mojo. This means that we're making changes to lots of code we don't
really understand (like this one) so we rely on other teams having the test
coverage needed. It's unfortunately not feasible for us to add tests to all the
code we touch.

[jochen (gone - plz use gerrit), 2017-01-19 18:59:35]

understood. I'll wait for treib@ to upload a fix with tests then.

[Marc Treib, 2017-01-20 09:07:46]

On 2017/01/19 18:59:35, jochen wrote:
> understood. I'll wait for treib@ to upload a fix with tests then.

A full end-to-end test would sure be nice, but it'll be a major undertaking.
Instant has a bunch of integration tests, but an alarming number of them is
disabled for flakiness :(

I'll see if I can get some smaller unit tests in, but failing that, our only
options for M57 are to either land this fix without tests, or revert the
mojoification CL (which I'd *really* like to avoid).

[jochen (gone - plz use gerrit), 2017-01-20 09:31:29]

I'd be happy with a unit test

[Marc Treib, 2017-01-20 11:48:32]

On 2017/01/20 09:31:29, jochen wrote:
> I'd be happy with a unit test

Maybe it's just my unfamiliarity with all things Blink, but I can't figure out
how to write a meaningful unit test for this (i.e. without making it so
intrusive as to be useless).
I'm looking into writing a browser test, but I'm not optimistic...

[tibell, 2017-01-23 03:53:59]

Close browser RouterProvider binding on renderer death