One and only one top-loading-frame across all renderers.

One and only one top-loading-frame across all renderers.

This CL prevents an OOPIF from being tracked as the top-loading-frame,
when another renderer already tracks the main frame as the
top-loading-frame.  Without this CL, BlinkTestRunner::TestFinished would
be called for an OOPIF (and we would just get lucky and fail to deliver
notification to the browser because of swapped-out filtering of
ShellViewHostMsg_TestFinishedInSecondaryRenderer).

BUG=595089
Committed: https://crrev.com/cb2ee5f8033a278716249a0ccc095ddd3f7a675f
Cr-Commit-Position: refs/heads/master@{#390387}

[Łukasz Anforowicz, 2016-04-25 23:19:02]

Description was changed from

==========
One and only one top-loading-frame across all renderers.

BUG=595089
==========

to

==========
One and only one top-loading-frame across all renderers.

This CL prevents an OOPIF from being tracked as the top-loading-frame,
when another renderer already tracks the main frame as the
top-loading-frame.  Without this CL, BlinkTestRunner::TestFinished would
be called for an OOPIF (and we would just get lucky and fail to deliver
notification to the browser because of swapped-out filtering of
ShellViewHostMsg_TestFinishedInSecondaryRenderer).

BUG=595089
==========

[Łukasz Anforowicz, 2016-04-25 23:19:58]

lukasza@chromium.org changed reviewers:
+ jochen@chromium.org

[Łukasz Anforowicz, 2016-04-25 23:19:59]

jochen@, could you please take a look?

https://codereview.chromium.org/1908233002/diff/20001/components/test_runner/...
File components/test_runner/test_runner.cc (left):

https://codereview.chromium.org/1908233002/diff/20001/components/test_runner/...
components/test_runner/test_runner.cc:1835: DCHECK(test_is_running_);
This DCHECK was introduced by me mainly to state that ConfigureForTestWithURL is
called in the renderer before it starts to handle flag replication (i.e. that
ShellViewMsg_SetupSecondaryRenderer or ShellViewMsg_ReplicateTestConfiguration
or ShellViewMsg_SetTestConfiguration is delivered before the first
LayoutTestMsg_ReplicateLayoutTestRuntimeFlagsChanges).

This DCHECK is problematic, because a msg to reset have-top-loading-frame sent
by renderer1 can be already in-flight when renderer2 calls
testRunner.notifyDone, sets test_is_running to false and only *then* receives
the message.  I thought about it a little bit and I think defensive programming
is the best option here.  Things should still quiet down between tests because
of 1) closing of all windows except the main window in-between tests and 2)
because of BlinkTestRunner::OnReset.

https://codereview.chromium.org/1908233002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/navigation/resources/notify-done.html
(left):

https://codereview.chromium.org/1908233002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/navigation/resources/notify-done.html:3:
if (window.testRunner)
I plan to land changes to this file separately - in
https://codereview.chromium.org/1907413003/

https://codereview.chromium.org/1908233002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html
(left):

https://codereview.chromium.org/1908233002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html:18:
testRunner.notifyDone();
I plan to land changes to this file separately - in
https://codereview.chromium.org/1907413003/

[jochen (gone - plz use gerrit), 2016-04-27 12:00:52]

lgtm anyway

https://codereview.chromium.org/1908233002/diff/40001/components/test_runner/...
File components/test_runner/layout_test_runtime_flags.h (right):

https://codereview.chromium.org/1908233002/diff/40001/components/test_runner/...
components/test_runner/layout_test_runtime_flags.h:163:
DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(have_top_loading_frame)
maybe track_top_loading_frame_in_this_process?

[Łukasz Anforowicz, 2016-04-27 22:59:03]

https://codereview.chromium.org/1908233002/diff/40001/components/test_runner/...
File components/test_runner/layout_test_runtime_flags.h (right):

https://codereview.chromium.org/1908233002/diff/40001/components/test_runner/...
components/test_runner/layout_test_runtime_flags.h:163:
DEFINE_BOOL_LAYOUT_TEST_RUNTIME_FLAG(have_top_loading_frame)
On 2016/04/27 12:00:51, jochen wrote:
> maybe track_top_loading_frame_in_this_process?

I've expanded the comment to try to clarify the meaning of this flag.

[Łukasz Anforowicz, 2016-04-28 14:40:42]

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

[commit-bot: I haz the power, 2016-04-28 14:40:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1908233002/70004
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1908233002/70004

[commit-bot: I haz the power, 2016-04-28 15:16:09]

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

[commit-bot: I haz the power, 2016-04-28 15:16:10]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2016-04-28 15:33:47]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-04-28 15:33:47]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1908233002/#ps70004
(title: "Comment tweak on |have_top_loading_frame| flag.")

[commit-bot: I haz the power, 2016-04-28 15:33:58]

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

[commit-bot: I haz the power, 2016-04-28 15:37:53]

Committed patchset #5 (id:70004)

[commit-bot: I haz the power, 2016-04-30 17:18:43]

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

[commit-bot: I haz the power, 2016-04-30 17:55:56]

Description was changed from

==========
One and only one top-loading-frame across all renderers.

This CL prevents an OOPIF from being tracked as the top-loading-frame,
when another renderer already tracks the main frame as the
top-loading-frame.  Without this CL, BlinkTestRunner::TestFinished would
be called for an OOPIF (and we would just get lucky and fail to deliver
notification to the browser because of swapped-out filtering of
ShellViewHostMsg_TestFinishedInSecondaryRenderer).

BUG=595089
==========

to

==========
One and only one top-loading-frame across all renderers.

This CL prevents an OOPIF from being tracked as the top-loading-frame,
when another renderer already tracks the main frame as the
top-loading-frame.  Without this CL, BlinkTestRunner::TestFinished would
be called for an OOPIF (and we would just get lucky and fail to deliver
notification to the browser because of swapped-out filtering of
ShellViewHostMsg_TestFinishedInSecondaryRenderer).

BUG=595089

Committed: https://crrev.com/cb2ee5f8033a278716249a0ccc095ddd3f7a675f
Cr-Commit-Position: refs/heads/master@{#390387}
==========