Bug 171974 - Unify hasTouchScrollableOverflow/needsCompositedScrolling concepts
Summary: Unify hasTouchScrollableOverflow/needsCompositedScrolling concepts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on:
Blocks: 149264 171667
  Show dependency treegraph
 
Reported: 2017-05-11 09:14 PDT by Frédéric Wang (:fredw)
Modified: 2017-05-16 07:37 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.30 KB, patch)
2017-05-11 09:32 PDT, Frédéric Wang (:fredw)
simon.fraser: review+
simon.fraser: commit-queue-
Details | Formatted Diff | Diff
Patch (5.30 KB, patch)
2017-05-11 11:28 PDT, Frédéric Wang (:fredw)
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Patch (5.30 KB, patch)
2017-05-11 11:33 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2017-05-11 09:14:29 PDT
The following pattern is found in various places:

#if PLATFORM(IOS)
...hasTouchScrollableOverflow()...
#else
...needsCompositedScrolling()...
#endif

We can refactor the code to do that in one function.

@Simon: I don't know if we can merge these functions a bit further... It seems that to fix bug 149264 we would consider composited scrolling on iOS too?
Comment 1 Frédéric Wang (:fredw) 2017-05-11 09:32:13 PDT
Created attachment 309716 [details]
Patch
Comment 2 Michael Catanzaro 2017-05-11 11:10:50 PDT
Comment on attachment 309716 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:2191
> +bool RenderLayer::useCoordinatedScrolling() const

Would RenderLayer::usesCoordinatedScrolling be a better name for this?
Comment 3 Frédéric Wang (:fredw) 2017-05-11 11:19:04 PDT
(In reply to Michael Catanzaro from comment #2)
> > Source/WebCore/rendering/RenderLayer.cpp:2191
> > +bool RenderLayer::useCoordinatedScrolling() const
> 
> Would RenderLayer::usesCoordinatedScrolling be a better name for this?

Right, I think it will be more consistent with the names of other member functions.
Comment 4 Simon Fraser (smfr) 2017-05-11 11:19:36 PDT
Comment on attachment 309716 [details]
Patch

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

>> Source/WebCore/rendering/RenderLayer.cpp:2191
>> +bool RenderLayer::useCoordinatedScrolling() const
> 
> Would RenderLayer::usesCoordinatedScrolling be a better name for this?

Yes!

Hopefully this won't be confused with coordinated graphics. Maybe "acceleratedScrolling"?
Comment 5 Frédéric Wang (:fredw) 2017-05-11 11:28:47 PDT
Created attachment 309733 [details]
Patch
Comment 6 WebKit Commit Bot 2017-05-11 11:30:53 PDT
Comment on attachment 309733 [details]
Patch

Rejecting attachment 309733 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 309733, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/3719277
Comment 7 Frédéric Wang (:fredw) 2017-05-11 11:33:26 PDT
Created attachment 309734 [details]
Patch

Setting correct reviewer...
Comment 8 WebKit Commit Bot 2017-05-11 11:49:34 PDT
Comment on attachment 309734 [details]
Patch

Clearing flags on attachment: 309734

Committed r216688: <http://trac.webkit.org/changeset/216688>
Comment 9 Frédéric Wang (:fredw) 2017-05-16 07:37:03 PDT
This is fixed.