html_viewer: Make sure window.internals is set-up correctly in layout-tests.

html_viewer: Make sure window.internals is set-up correctly in layout-tests.

Inject window.internals from WebFrameClient::didClearWindowObject() override
instead of right after creation, since that's how //content sets it up as well.

Also, fix a couple of crashes during start-up and tear-down.

BUG=none

Committed: https://crrev.com/b3ad146d98e8e67187e4f072676afbe210e73042
Cr-Commit-Position: refs/heads/master@{#344833}

[sadrul, 2015-08-18 08:00:13]

sadrul@chromium.org changed reviewers:
+ sky@chromium.org

[sadrul, 2015-08-18 08:00:13]

https://codereview.chromium.org/1297173002/diff/1/components/html_viewer/layo...
File components/html_viewer/layout_test_content_handler_impl.cc (right):

https://codereview.chromium.org/1297173002/diff/1/components/html_viewer/layo...
components/html_viewer/layout_test_content_handler_impl.cc:71: web_view =
params->manager->GetWebView();
This does avoid a crash on start-up below (line ~77:79) when proxy->web_view()
is null, but I am actually not sure if this is the right thing to do here.

[sky, 2015-08-18 18:13:57]

https://codereview.chromium.org/1297173002/diff/1/components/html_viewer/layo...
File components/html_viewer/layout_test_content_handler_impl.cc (right):

https://codereview.chromium.org/1297173002/diff/1/components/html_viewer/layo...
components/html_viewer/layout_test_content_handler_impl.cc:26:
blink::WebTestingSupport::injectInternalsObject(frame);
Should you call through to the base class?

https://codereview.chromium.org/1297173002/diff/1/components/html_viewer/layo...
components/html_viewer/layout_test_content_handler_impl.cc:71: web_view =
params->manager->GetWebView();
On 2015/08/18 08:00:13, sadrul wrote:
> This does avoid a crash on start-up below (line ~77:79) when proxy->web_view()
> is null, but I am actually not sure if this is the right thing to do here.

Are you trying to run a test that creates multiple frames? From talking with
Jochen the harness isn't really set up for that to work right now. I'm wondering
if we should only be creating the TestHTMLFrame for the main frame?

[sadrul, 2015-08-18 20:06:34]

https://codereview.chromium.org/1297173002/diff/1/components/html_viewer/layo...
File components/html_viewer/layout_test_content_handler_impl.cc (right):

https://codereview.chromium.org/1297173002/diff/1/components/html_viewer/layo...
components/html_viewer/layout_test_content_handler_impl.cc:26:
blink::WebTestingSupport::injectInternalsObject(frame);
On 2015/08/18 18:13:57, sky wrote:
> Should you call through to the base class?

Done.

https://codereview.chromium.org/1297173002/diff/1/components/html_viewer/layo...
components/html_viewer/layout_test_content_handler_impl.cc:71: web_view =
params->manager->GetWebView();
On 2015/08/18 18:13:57, sky wrote:
> On 2015/08/18 08:00:13, sadrul wrote:
> > This does avoid a crash on start-up below (line ~77:79) when
proxy->web_view()
> > is null, but I am actually not sure if this is the right thing to do here.
> 
> Are you trying to run a test that creates multiple frames? From talking with
> Jochen the harness isn't really set up for that to work right now. I'm
wondering
> if we should only be creating the TestHTMLFrame for the main frame?

Indeed, the test this was failing on did have an iframe. I have changed the code
here to simply return an HTMLFrame for inner-frames, and use a proxy with
TestHTMLFrame for the main-frame. Is this what you had in mind?

[sky, 2015-08-18 20:51:44]

LGTM

https://codereview.chromium.org/1297173002/diff/20001/components/html_viewer/...
File components/html_viewer/layout_test_content_handler_impl.cc (right):

https://codereview.chromium.org/1297173002/diff/20001/components/html_viewer/...
components/html_viewer/layout_test_content_handler_impl.cc:66: if
(params->parent)
Yes, this is what I had in mind. How about a comment as to why we do this.

[sadrul, 2015-08-21 03:28:14]

sadrul@chromium.org changed reviewers:
+ pfeldman@chromium.org

[sadrul, 2015-08-21 03:28:14]

+pfeldman@chromium.org for components/test_runner and content/shell owner
review.

[pfeldman, 2015-08-21 17:41:45]

lgtm

[sadrul, 2015-08-21 19:11:47]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2015-08-21 19:11:47]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1297173002/#ps40001
(title: ".")

[commit-bot: I haz the power, 2015-08-21 19:12:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1297173002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1297173002/40001

[commit-bot: I haz the power, 2015-08-21 19:17:43]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-21 19:18:48]

Patchset 3 (id:??) landed as
https://crrev.com/b3ad146d98e8e67187e4f072676afbe210e73042
Cr-Commit-Position: refs/heads/master@{#344833}