Bug 154333

Summary: Add test to check scroll animator events after a history navigation
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: Tools / TestsAssignee: 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 Flags
Patch
mcatanzaro: review-, buildbot: commit-queue-
Archive of layout-test-results from ews102 for mac-yosemite
none
Archive of layout-test-results from ews114 for mac-yosemite
none
Updated patch
mcatanzaro: review-, buildbot: commit-queue-
Archive of layout-test-results from ews106 for mac-yosemite-wk2
none
Archive of layout-test-results from ews125 for ios-simulator-wk2 none

Description Carlos Garcia Campos 2016-02-16 23:30:27 PST
We want to check that not only scrollable areas are still scrollable but also that the scroll animator is notified when the mouse enters/exists the scrollable areas.
Comment 1 Carlos Garcia Campos 2016-02-16 23:33:42 PST
Created attachment 271541 [details]
Patch
Comment 2 Build Bot 2016-02-17 00:21:22 PST
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
Comment 3 Build Bot 2016-02-17 00:21:26 PST
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 4 Build Bot 2016-02-17 00:43:58 PST
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
Comment 5 Build Bot 2016-02-17 00:44:01 PST
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
Comment 6 Carlos Garcia Campos 2016-02-17 08:00:37 PST
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.
Comment 7 Alexey Proskuryakov 2016-02-17 09:28:58 PST
It's presumably complaining about DumpRenderTree's injected objects having been destroyed on navigation.
Comment 8 Michael Catanzaro 2016-02-19 08:22:28 PST
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?
Comment 9 Carlos Garcia Campos 2016-02-20 00:47:49 PST
(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.
Comment 10 Carlos Garcia Campos 2016-04-15 02:42:09 PDT
Created attachment 276469 [details]
Updated patch
Comment 11 Build Bot 2016-04-15 03:29:46 PDT
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
Comment 12 Build Bot 2016-04-15 03:29:49 PDT
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 13 Build Bot 2016-04-15 03:33:33 PDT
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
Comment 14 Build Bot 2016-04-15 03:33:37 PDT
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 15 Michael Catanzaro 2016-05-17 08:32:34 PDT
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."