Summary: | Add test to check scroll animator events after a history navigation | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | NEW --- | ||||||||||||||||
Severity: | Normal | CC: | buildbot, lforschler, rniwa | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | WebKit Local Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=153404 | ||||||||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2016-02-16 23:30:27 PST
Created attachment 271541 [details]
Patch
Comment on attachment 271541 [details] Patch Attachment 271541 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/843787 New failing tests: fast/scrolling/scroll-animator-events-after-history-navigation.html Created attachment 271544 [details]
Archive of layout-test-results from ews102 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 271541 [details] Patch Attachment 271541 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/843788 New failing tests: fast/scrolling/scroll-animator-events-after-history-navigation.html Created attachment 271546 [details]
Archive of layout-test-results from ews114 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
We need specific results for wk1 because of the main frames, but why is wk1 using plugins for this test? I see this in the results: CONSOLE MESSAGE: line 25: ReferenceError: Trying to access object from destroyed plug-in. I guess we can just skip this test in wk1. It's presumably complaining about DumpRenderTree's injected objects having been destroyed on navigation. Comment on attachment 271541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271541&action=review Looks good, except for the WK1 test failure that needs fixed. > LayoutTests/fast/scrolling/scroll-animator-events-after-history-navigation.html:31 > + delete sessionStorage.didNavigate; Why do you need to do this? The test is about to end! > LayoutTests/fast/scrolling/scroll-animator-events-after-history-navigation.html:37 > + sessionStorage.didNavigate = true; This is needed... why, so that the page enters the page cache? (In reply to comment #8) > Comment on attachment 271541 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271541&action=review > > Looks good, except for the WK1 test failure that needs fixed. We just need to skip this test in wk1, I don't see that a reason to reject the patch. > > LayoutTests/fast/scrolling/scroll-animator-events-after-history-navigation.html:31 > > + delete sessionStorage.didNavigate; > > Why do you need to do this? The test is about to end! To clean up the session storage so that it won't affect other tests that could use also didNavigate. It's quite common to do this in layout tests. > > LayoutTests/fast/scrolling/scroll-animator-events-after-history-navigation.html:37 > > + sessionStorage.didNavigate = true; > > This is needed... why, so that the page enters the page cache? No, this has nothing to do with the page cache. Please, if you have questions about the patch or you didn't understand a part of it, ask any questions before deciding to reject the patch. When a test does a navigation or reload and you want to do different things depending on every navigation you need to store some state somewhere that is alive during navigation, and we normally use the session storage for that. In this case, when the test is loaded the first time we want to navigate to another page that will navigate back. When the page is loaded the second time, we just want to finish the test. The way the test know if it's the first navigation or not is checking the sessionStorage.didNavigate value. Created attachment 276469 [details]
Updated patch
Comment on attachment 276469 [details] Updated patch Attachment 276469 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1162619 New failing tests: fast/scrolling/scroll-animator-events-after-history-navigation.html Created attachment 276470 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 276469 [details] Updated patch Attachment 276469 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1162609 New failing tests: fast/scrolling/scroll-animator-events-after-history-navigation.html Created attachment 276471 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 276469 [details]
Updated patch
I think we need to investigate why this test is failing on Mac WK2 and iOS. I don't give r- to say "this patch is bad," just to say "not ready to commit yet."
|