Bug 211026

Summary: Commit the scrolling tree from the main thread
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: ScrollingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, darin, ews-watchlist, fred.wang, jamesr, luiz, simon.fraser, tonikitoo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 210884    
Attachments:
Description Flags
Patch
none
Patch
darin: review+
Patch
none
Patch
none
Patch none

Description Simon Fraser (smfr) 2020-04-25 12:09:55 PDT
Commit the scrolling tree from the main thread
Comment 1 Simon Fraser (smfr) 2020-04-25 12:13:27 PDT
Created attachment 397572 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2020-04-25 12:14:10 PDT
<rdar://problem/62374855>
Comment 3 Simon Fraser (smfr) 2020-04-25 12:34:03 PDT
Created attachment 397573 [details]
Patch
Comment 4 Darin Adler 2020-04-25 13:05:41 PDT
Comment on attachment 397573 [details]
Patch

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

> Source/WebCore/page/scrolling/ScrollingCoordinator.h:71
> +using PlatformDisplayID = uint32_t;

I think this name would work just as well without the word "platform" in it.

> Source/WebCore/page/scrolling/ScrollingTree.h:57
> +#if ENABLE(SCROLLING_THREAD)
> +using PlatformDisplayID = uint32_t;
> +#endif

Same comment as above that I don’t think the word "platform" is helpful in this name.

Is there some way to avoid defining this type separately in multiple headers?

Please add a blank line before this since it doesn’t fit great into the paragraph of forward-declared classes.

> Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm:108
> +    auto stateTree = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation);
> +    scrollingTree()->commitTreeState(WTFMove(stateTree));

Would this read better as a single line?

> Source/WebCore/page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp:98
> +    auto stateTree = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation);
> +    scrollingTree()->commitTreeState(WTFMove(stateTree));

Would this read better as a single line?
Comment 5 Simon Fraser (smfr) 2020-04-25 13:22:01 PDT
Oops, I didn't mean to include the windowScreenDidChange changes in this page (webkit-patch upload strikes again)
Comment 6 Simon Fraser (smfr) 2020-04-25 13:25:07 PDT
Created attachment 397578 [details]
Patch
Comment 7 Simon Fraser (smfr) 2020-04-25 14:57:30 PDT
Created attachment 397587 [details]
Patch
Comment 8 Simon Fraser (smfr) 2020-04-25 15:19:09 PDT
Created attachment 397591 [details]
Patch
Comment 9 EWS 2020-04-25 18:24:07 PDT
Committed r260716: <https://trac.webkit.org/changeset/260716>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397591 [details].