Summary: | REGRESSION (r130783): Going "back" to a cached page from a page with a main resource error breaks scrolling, amongst other issues | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||||||||||||||||||
Component: | Page Loading | Assignee: | Brady Eidson <beidson> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | andersca, ap, buildbot, commit-queue, japhet, rniwa, sam, thorton | ||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||
Attachments: |
|
Description
Brady Eidson
2013-06-01 15:01:16 PDT
Created attachment 203496 [details]
Reduction to reproduce.
Drop the attached files in a PHP enabled web server and then visit index.html.
Go to "slow.php" and while it's loading, go back.
You'll notice that you can't scroll. In certain older builds you'll even crash.
Listening to the load callbacks you'll also see the out-of-orderness I described.
Created attachment 203497 [details]
Patch v1 - Current proposed fix
I'm still playing with this locally and know this isn't ready for review, but currently I think the fix might be this simple.
And since layout tests are flaky in general on my local machine, I'd like to see was EWS has to say. Comment on attachment 203497 [details] Patch v1 - Current proposed fix Attachment 203497 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/659321 New failing tests: media/video-controls-in-media-document.html http/tests/media/media-document.html webarchive/loading/video-in-webarchive.html media/click-volume-bar-not-pausing.html media/video-element-other-namespace-crash.html media/video-click-dblckick-standalone.html fast/events/media-focus-in-standalone-media-document.html media/media-document-audio-size.html Created attachment 203499 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
(In reply to comment #4) > (From update of attachment 203497 [details]) > Attachment 203497 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/659321 > > New failing tests: > media/video-controls-in-media-document.html > http/tests/media/media-document.html > webarchive/loading/video-in-webarchive.html > media/click-volume-bar-not-pausing.html > media/video-element-other-namespace-crash.html > media/video-click-dblckick-standalone.html > fast/events/media-focus-in-standalone-media-document.html > media/media-document-audio-size.html Most of these fail on my machine locally *WITHOUT* this patch. I'll try to wade through the differences. Comment on attachment 203497 [details] Patch v1 - Current proposed fix Attachment 203497 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/661388 New failing tests: media/video-controls-in-media-document.html http/tests/media/media-document.html webarchive/loading/video-in-webarchive.html media/click-volume-bar-not-pausing.html media/video-element-other-namespace-crash.html media/video-click-dblckick-standalone.html fast/events/media-focus-in-standalone-media-document.html media/media-document-audio-size.html Created attachment 203504 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Comment on attachment 203497 [details] Patch v1 - Current proposed fix Attachment 203497 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/750396 New failing tests: media/video-controls-in-media-document.html http/tests/media/media-document.html webarchive/loading/video-in-webarchive.html media/click-volume-bar-not-pausing.html media/video-element-other-namespace-crash.html media/video-click-dblckick-standalone.html fast/events/media-focus-in-standalone-media-document.html media/media-document-audio-size.html Created attachment 203507 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.3
That last patch did indeed change the equation a bit in media-land. Not at all clear why - they must use the parser in some unconventional ways. This new patch limits the change, and also includes a layout test and changelog - It can be landed if EWS checks out (I'm also in the process of running tests locally) Created attachment 203543 [details]
Patch v2 - Landable if tests pass
Comment on attachment 203543 [details] Patch v2 - Landable if tests pass View in context: https://bugs.webkit.org/attachment.cgi?id=203543&action=review r=me > LayoutTests/http/tests/loading/unfinished-load-back-to-cached-page-callbacks-expected.txt:26 > +This test makes sure that going back to a cached page from a page that is still loading produces the correct frame load callbacks. It is only useful inside of WebKitTestRunner. Would be helpful to have criteria for passing explained. How will we know if any change in the long list of callbacks above means a horrible failure? (In reply to comment #13) > (From update of attachment 203543 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203543&action=review > > r=me > > > LayoutTests/http/tests/loading/unfinished-load-back-to-cached-page-callbacks-expected.txt:26 > > +This test makes sure that going back to a cached page from a page that is still loading produces the correct frame load callbacks. It is only useful inside of WebKitTestRunner. > > Would be helpful to have criteria for passing explained. How will we know if any change in the long list of callbacks above means a horrible failure? The list of callbacks is long, but fortunately the target of the fix is only the last three - I can definitely add an explanation. Thanks for the review - I'll make sure the layouttest situation is fully understood before landing. Comment on attachment 203543 [details] Patch v2 - Landable if tests pass Attachment 203543 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/704594 New failing tests: http/tests/loading/unfinished-load-back-to-cached-page-callbacks.html Created attachment 203545 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Comment on attachment 203543 [details] Patch v2 - Landable if tests pass Attachment 203543 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/755243 New failing tests: http/tests/loading/unfinished-load-back-to-cached-page-callbacks.html Created attachment 203547 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
(In reply to comment #17) > (From update of attachment 203543 [details]) > Attachment 203543 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/755243 > > New failing tests: > http/tests/loading/unfinished-load-back-to-cached-page-callbacks.html Yah, my new test is flawed somehow. I don't know why. Passes 100% standalone, flakes with http/tests/loading, and seems to fail when run with much more than ~30 tests. I suspect a bug in the testing harness with frame load callbacks, actually. But don't have time to pursue that - I have a different strategy for the test for now. Created attachment 203549 [details]
Patch for landing
Figured out the layouttest mess. Will let EWS chew on this.
|