Bug 193165 – Factor legacy WK1 code for fixed and scrolling layers into their own helper class

• Component: New Bugs
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Factor legacy WK1 code for fixed and scrolling layers into their own helper class

[Simon Fraser (smfr)]

Created attachment 358403 [details]
Patch

[EWS Watchlist]

Attachment 358403 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:265:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 358403 [details]
Patch

Attachment 358403 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10635524

New failing tests:
http/wpt/css/css-animations/start-animation-001.html

[EWS Watchlist]

Created attachment 358415 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 358403 [details]
Patch

Attachment 358403 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10638292

New failing tests:
http/wpt/css/css-animations/start-animation-001.html

[EWS Watchlist]

Created attachment 358431 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Frédéric Wang (:fredw)]

Comment on attachment 358403 [details]
Patch

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

> Source/WebCore/page/ChromeClient.h:265
> +    virtual void updateViewportConstrainedLayers(HashMap<PlatformLayer*, std::unique_ptr<ViewportConstraints>>&, const HashMap<PlatformLayer*, PlatformLayer*>&) { }

It seems this is causing a style error but it was probably already present before this change.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:280
> +        m_legacyScrollingLayerCoordinator = std::make_unique<LegacyWebKitScrollingLayerCoordinator>(page().chrome().client(), m_renderView.frameView().frame().isMainFrame());

It seems you can just use isMainFrameCompositor(). The old code for registerAllViewportConstrainedLayers/unregisterAllViewportConstrainedLayers also used to do an early return when scrollingCoordinator() is non-null, can you please explain in the ChangeLog why it is safe to remove it? I guess this because it is always null on WK1 (i.e. when m_renderView.frameView().platformWidget() is non-null)?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3994
> +    ASSERT(m_renderView.document().pageCacheState() == Document::NotInPageCache);

We used to do an early return and now just ASSERT. Can you please explain this in the ChangeLog too?

> Source/WebCore/rendering/RenderLayerCompositor.h:118
> +    bool m_coordinateViewportConstainedLayers;

It looks like that member can be const.

> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:70
> +//    ASSERT(m_creationThread.ptr() == &Thread::current());

It seems this change was not intentional.

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/239658/webkit

[Radar WebKit Bug Importer]

<rdar://problem/47067632>