Bug 144482 - Expand test infrastructure to support scrolling tests (Part 4): Scroll Snap Support
Summary: Expand test infrastructure to support scrolling tests (Part 4): Scroll Snap S...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on: 144131
Blocks: 146081 142358 144591 172349
  Show dependency treegraph
 
Reported: 2015-04-30 18:06 PDT by Brent Fulgham
Modified: 2017-05-19 01:04 PDT (History)
6 users (show)

See Also:


Attachments
Patch (11.39 KB, patch)
2015-05-01 11:53 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (31.40 KB, patch)
2015-05-01 16:31 PDT, Brent Fulgham
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2015-04-30 18:06:29 PDT
Add tests and adjustments to the scroll testing infrastructure to ensure that the scroll snap feature is fully tested.

These changes involve:
1. Make sure the WheelEventTestTrigger object is properly relayed to the scrolling thread so that scroll animations driven on the scrolling thread also block test runs.
2. Add new scroll-snap tests.
Comment 1 Brent Fulgham 2015-05-01 11:53:42 PDT
Created attachment 252161 [details]
Patch
Comment 2 Brent Fulgham 2015-05-01 16:31:27 PDT
Created attachment 252193 [details]
Patch
Comment 3 Simon Fraser (smfr) 2015-05-01 16:41:46 PDT
Comment on attachment 252193 [details]
Patch

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

> Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:34
> +#include <wtf/RefPtr.h>

Not needed.

> Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:110
>      bool m_requestedScrollPositionRepresentsProgrammaticScroll;
> +    bool m_expectsWheelEventTestTrigger;

Might as well convert these to C++11 initializersL m_expectsWheelEventTestTrigger { false };

> Source/WebCore/page/scrolling/ScrollingTree.h:105
> +    virtual void deferTestsForReason(WheelEventTestTrigger::ScrollableAreaIdentifier, WheelEventTestTrigger::DeferTestTriggerReason) { /* Do nothing */ }
> +    virtual void removeTestDeferralForReason(WheelEventTestTrigger::ScrollableAreaIdentifier, WheelEventTestTrigger::DeferTestTriggerReason) { /* Do nothing */ }

No need for "/* Do nothing */".

> Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:107
> +    bool m_expectsWheelEventTestTrigger;

m_expectsWheelEventTestTrigger { false };
Comment 4 Brent Fulgham 2015-05-01 16:49:31 PDT
Comment on attachment 252193 [details]
Patch

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

>> Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:34
>> +#include <wtf/RefPtr.h>
> 
> Not needed.

I'll remove it.

>> Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:110
>> +    bool m_expectsWheelEventTestTrigger;
> 
> Might as well convert these to C++11 initializersL m_expectsWheelEventTestTrigger { false };

OK.

>> Source/WebCore/page/scrolling/ScrollingTree.h:105
>> +    virtual void removeTestDeferralForReason(WheelEventTestTrigger::ScrollableAreaIdentifier, WheelEventTestTrigger::DeferTestTriggerReason) { /* Do nothing */ }
> 
> No need for "/* Do nothing */".

OK!

>> Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:107
>> +    bool m_expectsWheelEventTestTrigger;
> 
> m_expectsWheelEventTestTrigger { false };

Done.
Comment 5 Brent Fulgham 2015-05-01 17:10:48 PDT
New scroll snap tests are under Bug 142358.
Comment 6 Brent Fulgham 2015-05-01 17:40:51 PDT
Committed r183702: <http://trac.webkit.org/changeset/183702>