Bug 121628 - Flaky Test: fast/history/history-back-while-pdf-in-pagecache.html
Summary: Flaky Test: fast/history/history-back-while-pdf-in-pagecache.html
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-19 13:08 PDT by zalan
Modified: 2015-08-07 13:45 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.63 KB, patch)
2013-09-19 13:29 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (5.04 KB, patch)
2013-09-20 05:25 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2013-09-19 13:08:04 PDT
ssia
Comment 1 zalan 2013-09-19 13:29:11 PDT
Created attachment 212093 [details]
Patch
Comment 2 Alexey Proskuryakov 2013-09-19 15:54:19 PDT
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.
Comment 3 zalan 2013-09-20 05:24:26 PDT
(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.
Comment 4 zalan 2013-09-20 05:25:20 PDT
Created attachment 212152 [details]
Patch
Comment 5 Alexey Proskuryakov 2013-09-20 11:22:17 PDT
> 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).
Comment 6 zalan 2013-09-20 11:24:21 PDT
(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.
Comment 7 Alexey Proskuryakov 2013-09-20 11:26:01 PDT
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.
Comment 8 zalan 2013-09-20 11:36:19 PDT
(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)
Comment 9 Alexey Proskuryakov 2013-09-20 11:54:00 PDT
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.
Comment 10 zalan 2013-09-20 11:56:22 PDT
(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.
Comment 11 Alexey Proskuryakov 2013-10-03 13:35:34 PDT
Marked as flaky in r156853.

> I'd like to know what Tim thinks about adding an Internals hook for when PDF content is ready. <
Comment 12 Tim Horton 2013-10-03 13:39:05 PDT
(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?
Comment 13 Alexey Proskuryakov 2014-12-14 18:25:15 PST
This test also asserts in debug builds, which is tracked in <rdar://problem/18216390>.