Remove testRunner.globalFlag use from layout tests outside http/security

Remove testRunner.globalFlag use from layout tests outside http/security

BUG=405846
R=esprehn@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180788

[Dirk Pranke, 2014-08-21 15:54:43]

On 2014/08/21 08:23:12, dcheng (OOO) wrote:

I have no idea what testRunner.globalflag does or what these tests are meant to
do, so at best I can stare at them for a while and try to rubber stamp this, but
perhaps there is some other reviewer that might be more familiar with them?

[dcheng, 2014-08-21 18:25:12]

OK, I thought you might have some context because of
https://bugs.webkit.org/show_bug.cgi?id=32761, sorry!

esprehn, do you mind taking a look instead?

[esprehn, 2014-08-21 19:41:22]

lgtm, we should really get rid of that globalFlag thing entirely.

[esprehn, 2014-08-21 19:44:07]

https://codereview.chromium.org/475933003/diff/1/LayoutTests/http/tests/misc/...
File LayoutTests/http/tests/misc/resources/content-iframe.html (right):

https://codereview.chromium.org/475933003/diff/1/LayoutTests/http/tests/misc/...
LayoutTests/http/tests/misc/resources/content-iframe.html:5:
window.addEventListener('message', reply);
Why not do this the other way around and window.opener.postMessage() in the
onload here.

https://codereview.chromium.org/475933003/diff/1/LayoutTests/http/tests/misc/...
File LayoutTests/http/tests/misc/set-window-opener-to-null.html (right):

https://codereview.chromium.org/475933003/diff/1/LayoutTests/http/tests/misc/...
LayoutTests/http/tests/misc/set-window-opener-to-null.html:26: w.onload =
function() { maybeStartTest(w); };
This looks flakey, just add an onmessage listener here and then wait for the
other document to message you back. You don't need this maybe start test thing I
don't think.

[dcheng, 2014-08-21 20:18:10]

https://codereview.chromium.org/475933003/diff/1/LayoutTests/http/tests/misc/...
File LayoutTests/http/tests/misc/resources/content-iframe.html (right):

https://codereview.chromium.org/475933003/diff/1/LayoutTests/http/tests/misc/...
LayoutTests/http/tests/misc/resources/content-iframe.html:5:
window.addEventListener('message', reply);
On 2014/08/21 19:44:07, esprehn wrote:
> Why not do this the other way around and window.opener.postMessage() in the
> onload here.
> 

Because window.opener is (or should be) null here.

https://codereview.chromium.org/475933003/diff/1/LayoutTests/http/tests/misc/...
File LayoutTests/http/tests/misc/set-window-opener-to-null.html (right):

https://codereview.chromium.org/475933003/diff/1/LayoutTests/http/tests/misc/...
LayoutTests/http/tests/misc/set-window-opener-to-null.html:26: w.onload =
function() { maybeStartTest(w); };
On 2014/08/21 19:44:07, esprehn wrote:
> This looks flakey, just add an onmessage listener here and then wait for the
> other document to message you back. You don't need this maybe start test thing
I
> don't think.

Unfortunately the other window doesn't have a connection back to this window,
since we set window.opener to null. So we have to grab it out of the postMessage
source so we can postMessage back. I wasn't sure if this would get hit twice,
but it doesn't seem to.

The problem is this doesn't seem to work outside of run_webkit_tests.sh -- if I
run content_shell --dump-render-tree
http/tests/misc/set-window-opener-to-null.html, it just hangs forever. I'll
investigate some more...

[dcheng, 2014-08-21 20:43:14]

OK, after some further testing, I realized I forgot to manually start the test
server.

So I could simplify the test a little: I could just use w.onload to run the
entire test instead of postMessage() back and forth. The problem is I want to be
absolutely sure the correct target page is loaded, hence the use of
postMessage--if the wrong target is loaded, then the test will timeout and fail
instead of giving a bogus pass result. Given that, are you OK with the way this
is structured?

[dcheng, 2014-08-21 22:31:19]

Actually I just changed this to use onload. If w.onload breaks for some reason,
we'll have a lot of problems.

[esprehn, 2014-08-22 17:02:28]

lgtm

[dcheng, 2014-08-22 17:03:20]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-08-22 17:04:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dcheng@chromium.org/475933003/20001

[commit-bot: I haz the power, 2014-08-22 18:02:01]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[commit-bot: I haz the power, 2014-08-22 18:04:22]

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

[commit-bot: I haz the power, 2014-08-22 18:04:23]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_chromium_gn_comp...)

[dcheng, 2014-08-22 19:11:26]

Committed patchset #2 manually as 180788 (presubmit successful).