Bug 211026 - Commit the scrolling tree from the main thread
Summary: Commit the scrolling tree from the main thread
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks: 210884
  Show dependency treegraph
 
Reported: 2020-04-25 12:09 PDT by Simon Fraser (smfr)
Modified: 2020-04-25 18:24 PDT (History)
9 users (show)

See Also:


Attachments
Patch (14.19 KB, patch)
2020-04-25 12:13 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (15.35 KB, patch)
2020-04-25 12:34 PDT, Simon Fraser (smfr)
darin: review+
Details | Formatted Diff | Diff
Patch (9.94 KB, patch)
2020-04-25 13:25 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (11.03 KB, patch)
2020-04-25 14:57 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (11.07 KB, patch)
2020-04-25 15:19 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].