Bug 121628 – Flaky Test: fast/history/history-back-while-pdf-in-pagecache.html

• Component: Tools / Tests
• Status: ASSIGNED

[zalan]

ssia

[zalan]

Created attachment 212093 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 212093 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=212093&action=review

> LayoutTests/fast/history/history-back-while-pdf-in-pagecache.html:20
> +            // Inject this iframe to figure out when the pdf load is finished as the original <iframe> does not fire onload on history.back().

Can we use pageshow event after back()? Not sure if that would help, but this is what replaces the load event when a page comes from cache.

[zalan]

(In reply to comment #2)
> (From update of attachment 212093 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=212093&action=review
> 
> > LayoutTests/fast/history/history-back-while-pdf-in-pagecache.html:20
> > +            // Inject this iframe to figure out when the pdf load is finished as the original <iframe> does not fire onload on history.back().
> 
> Can we use pageshow event after back()? Not sure if that would help, but this is what replaces the load event when a page comes from cache.

Thanks for the hint. Using pageshow does make the testcase a bit better when it comes to differentiating WK1 and WK2 behaviour, but unfortunately it can't replace the iframe inject as the pageshow event on history navigation has no dependency on the iframe load AFAICT, and the test could still be flaky if we finish it before the actual pdf is finished loading.

[zalan]

Created attachment 212152 [details]
Patch

[Alexey Proskuryakov]

> unfortunately it can't replace the iframe inject as the pageshow event on history navigation has no dependency on the iframe load AFAICT

The iframe itself will not need to load, as it comes from the b/f cache. But plug-ins certainly do break the model, as they are reconstructed later. Plug-ins always break it, because they can do loading at any time (e.g. the actual plug-in resource could be a manifest, and all resources would be loaded later).

[zalan]

(In reply to comment #5)
> > unfortunately it can't replace the iframe inject as the pageshow event on history navigation has no dependency on the iframe load AFAICT
> 
> The iframe itself will not need to load, as it comes from the b/f cache. But plug-ins certainly do break the model, as they are reconstructed later. Plug-ins always break it, because they can do loading at any time (e.g. the actual plug-in resource could be a manifest, and all resources would be loaded later).

Correct, I meant the re-constructed plugin load.

[Alexey Proskuryakov]

Comment on attachment 212152 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=212152&action=review

> LayoutTests/fast/history/history-back-while-pdf-in-pagecache-expected.html:16
> +        }, 300);

I really don't like changing the timeout. If it needs to be longer than 0, the test _will_ fail on bots. There are lots of processes competing for CPU and memory, and being randomly frozen for a few seconds is normal.

> LayoutTests/fast/history/history-back-while-pdf-in-pagecache.html:21
> +            document.getElementById('container').innerHTML += '<iframe onload=\'done();\' src=\'../images/resources/green_rectangle.pdf\'></iframe>';

I don't think that this helps in a clean way. Iframe onload is fired before a plug-in displays its content, and there is no guarantee of ordering anyway.

So, I think that this just adds a delay essentially.

[zalan]

(In reply to comment #7)
> (From update of attachment 212152 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=212152&action=review
> 
> > LayoutTests/fast/history/history-back-while-pdf-in-pagecache-expected.html:16
> > +        }, 300);
> 
> I really don't like changing the timeout. If it needs to be longer than 0, the test _will_ fail on bots. There are lots of processes competing for CPU and memory, and being randomly frozen for a few seconds is normal.
> 
> > LayoutTests/fast/history/history-back-while-pdf-in-pagecache.html:21
> > +            document.getElementById('container').innerHTML += '<iframe onload=\'done();\' src=\'../images/resources/green_rectangle.pdf\'></iframe>';
> 
> I don't think that this helps in a clean way. Iframe onload is fired before a plug-in displays its content, and there is no guarantee of ordering anyway.
The forced layout should guarantee the ordering as it initiates the plugin content load before the inject happens, but I agree with you in general that the inject essentially just adds some delay. Though this delay is probably the best guess to cover the original pdf load as opposed to any timers. (while the best non-guessing would be some proper signalling and I am happy to use one if available)

[Alexey Proskuryakov]

I'd like to know what Tim thinks about adding an Internals hook for when PDF content is ready. We don't have any other PDF subframe tests yet, but we should, and it would be nice to make them straightforward to write.

[zalan]

(In reply to comment #9)
> I'd like to know what Tim thinks about adding an Internals hook for when PDF content is ready. We don't have any other PDF subframe tests yet, but we should, and it would be nice to make them straightforward to write.
Couldn't agree more. I was surprised to see that this was the first.

[Alexey Proskuryakov]

Marked as flaky in r156853.

> I'd like to know what Tim thinks about adding an Internals hook for when PDF content is ready. <

[Tim Horton]

(In reply to comment #11)
> Marked as flaky in r156853.
> 
> > I'd like to know what Tim thinks about adding an Internals hook for when PDF content is ready. <

Doesn't seem unreasonable to me. Couldn't it be a more generic plugin thing, or is PDFPlugin the only one that receives data in the roundabout way?

[Alexey Proskuryakov]

This test also asserts in debug builds, which is tracked in <rdar://problem/18216390>.