Fix crash when accessing Event::path(). Now with de-flaked test.

Fix crash when accessing Event::path(). Now with de-flaked test.

This is a re-try of crrev.com/516843004. In order to de-flake the test,
the nextIframeLoaded function checks whether finishJSTest (called from
within that function) has already been called, thus ensuring that the
guts of the test will only be executed once.

BUG=400476
R=haraken@chromium.org

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

[haraken, 2014-09-02 10:29:58]

Can we call clearTimeout, stop nextIframeLoaded from getting called, and then
call finishJSTest?

[blink-reviews, 2014-09-02 10:45:06]

On Tue, Sep 2, 2014 at 12:29 PM, <haraken@chromium.org> wrote:

> Can we call clearTimeout, stop nextIframeLoaded from getting called, and
> then
> call finishJSTest?
>

I'm unsure how that would work. To get a handle on things, I put counters
in the various functions to understand what gets executes how often things
get executed. The source of the problem is that iframe.onload gets
triggered repeatedly, around ~10 times in my tries. I guess it's then a
race between all those callbacks which one gets to nextIframeLoaded before
the test terminates. Usually only one, but apparently sometimes two.

So there are ~10 pending timeouts. I guess I could collect all of them in a
list and then clearTimeout all of them. Is that what you mean?

(I'm a bit unsure /why/ iframeOnload gets called so often. I've found some
references to this on stack overflow: Apparently, when the onload handler
is set after the iframe is appended to the document, it gets called exactly
once. In this test this doesn't work, because there is actually no content
to be loaded, and hence onload never gets called at all in this situation.
If the onload handler is set before the iframe is appended to the document,
then... it gets called multiple times. The reason escapes me and, judging
by what I found, most others, too..)

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[haraken, 2014-09-02 10:49:27]

On 2014/09/02 10:45:06, blink-reviews wrote:
> On Tue, Sep 2, 2014 at 12:29 PM, <mailto:haraken@chromium.org> wrote:
> 
> > Can we call clearTimeout, stop nextIframeLoaded from getting called, and
> > then
> > call finishJSTest?
> >
> 
> I'm unsure how that would work. To get a handle on things, I put counters
> in the various functions to understand what gets executes how often things
> get executed. The source of the problem is that iframe.onload gets
> triggered repeatedly, around ~10 times in my tries. I guess it's then a
> race between all those callbacks which one gets to nextIframeLoaded before
> the test terminates. Usually only one, but apparently sometimes two.
> 
> So there are ~10 pending timeouts. I guess I could collect all of them in a
> list and then clearTimeout all of them. Is that what you mean?

Yes, something like that. We should make the test behavior more reliable. In
this case, we should implement the test so that the iframeOnload stops before
calling finishJSTest().

wasFinishJSTest is a hack introduced to implement a leak detector, and we
shouldn't rely on it in layout tests.

[vogelheim, 2014-09-02 11:27:14]

On 2014/09/02 10:29:58, haraken wrote:
> Can we call clearTimeout, stop nextIframeLoaded from getting called, and then
> call finishJSTest?

Done.

[haraken, 2014-09-02 11:30:38]

LGTM

[vogelheim, 2014-09-02 11:31:39]

The CQ bit was checked by vogelheim@chromium.org

[commit-bot: I haz the power, 2014-09-02 11:32:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vogelheim@chromium.org/533633002/40001

[commit-bot: I haz the power, 2014-09-02 12:34:09]

Committed patchset #2 (id:40001) as 181234