Improve safety of accessing RemoteLayerTreeContext's LayerPool

https://bugs.webkit.org/show_bug.cgi?id=274188
rdar://127480646

Reviewed by Geoffrey Garen.

GraphicsLayerCARemote and PlatformCALayerRemote currently have raw pointers to
a RemoteLayerTreeContext. In some rare cases, we try to access the LayerPool
through the pointer after it was cleared, and this causes a crash.

This patch avoids the crash by changing the PlatformCALayer::layerPool
signature so it returns a pointer that we can null-check. And we replace the
RemoteLayerTreeContext raw pointers with WeakPtr for additional safety.

* Source/WebCore/platform/graphics/ca/PlatformCALayer.h:
* Source/WebCore/platform/graphics/ca/PlatformCALayer.mm:
(WebCore::PlatformCALayer::createCompatibleLayerOrTakeFromPool):
(WebCore::PlatformCALayer::moveToLayerPool):
(WebCore::PlatformCALayer::layerPool):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.mm:
(WebKit::GraphicsLayerCARemote::~GraphicsLayerCARemote):
(WebKit::GraphicsLayerCARemote::moveToContext):
(WebKit::GraphicsLayerCARemote::createAsyncContentsDisplayDelegate):
(WebKit::GraphicsLayerCARemote::layerMode const):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h:
(WebKit::PlatformCALayerRemote::context const):
(WebKit::PlatformCALayerRemote::clearContext): Deleted.
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.mm:
(WebKit::PlatformCALayerRemote::~PlatformCALayerRemote):
(WebKit::PlatformCALayerRemote::moveToContext):
(WebKit::PlatformCALayerRemote::shouldIncludeDisplayListInBackingStore const):
(WebKit::PlatformCALayerRemote::updateBackingStore):
(WebKit::PlatformCALayerRemote::copyContentsFromLayer):
(WebKit::PlatformCALayerRemote::addAnimationForKey):
(WebKit::PlatformCALayerRemote::layerPool):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.h:
(WebKit::RemoteLayerTreeContext::create):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.mm:
(WebKit::RemoteLayerTreeContext::~RemoteLayerTreeContext):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::RemoteLayerTreeDrawingArea):
(WebKit::RemoteLayerTreeDrawingArea::graphicsLayerFactory):
(WebKit::RemoteLayerTreeDrawingArea::adoptLayersFromDrawingArea):

Canonical link: https://commits.webkit.org/278916@main

Source/WebCore/platform/graphics/ca/PlatformCALayer.h

@@ -343,7 +343,7 @@ class WEBCORE_EXPORT PlatformCALayer : public ThreadSafeRefCounted<PlatformCALay
 protected:
     PlatformCALayer(LayerType, PlatformCALayerClient* owner);
 
-    virtual LayerPool& layerPool();
+    virtual LayerPool* layerPool();
 
     const LayerType m_layerType;
     const PlatformLayerIdentifier m_layerID;

Source/WebCore/platform/graphics/ca/PlatformCALayer.mm

@@ -158,7 +158,7 @@
 
 Ref<PlatformCALayer> PlatformCALayer::createCompatibleLayerOrTakeFromPool(PlatformCALayer::LayerType layerType, PlatformCALayerClient* client, IntSize size)
 {
-    if (auto layerFromPool = layerPool().takeLayerWithSize(size)) {
+    if (auto layerFromPool = layerPool() ? layerPool()->takeLayerWithSize(size) : nullptr) {
         layerFromPool->setOwner(client);
         return layerFromPool.releaseNonNull();
     }
@@ -171,13 +171,14 @@
 void PlatformCALayer::moveToLayerPool()
 {
     ASSERT(!superlayer());
-    layerPool().addLayer(this);
+    if (auto pool = layerPool())
+        pool->addLayer(this);
 }
 
-LayerPool& PlatformCALayer::layerPool()
+LayerPool* PlatformCALayer::layerPool()
 {
     static LayerPool* sharedPool = new LayerPool;
-    return *sharedPool;
+    return sharedPool;
 }
 
 void PlatformCALayer::clearContents()

Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.h

@@ -41,7 +41,6 @@ class GraphicsLayerCARemote final : public WebCore::GraphicsLayerCA, public CanM
     bool filtersCanBeComposited(const WebCore::FilterOperations& filters) override;
 
     void moveToContext(RemoteLayerTreeContext&);
-    void clearContext() { m_context = nullptr; }
     LayerMode layerMode() const final;
 
 private:
@@ -66,7 +65,7 @@ class GraphicsLayerCARemote final : public WebCore::GraphicsLayerCA, public CanM
 
     RefPtr<WebCore::GraphicsLayerAsyncContentsDisplayDelegate> createAsyncContentsDisplayDelegate(WebCore::GraphicsLayerAsyncContentsDisplayDelegate*) final;
 
-    RemoteLayerTreeContext* m_context;
+    WeakPtr<RemoteLayerTreeContext> m_context;
 };
 
 } // namespace WebKit

Source/WebKit/WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.mm

@@ -52,8 +52,8 @@
 
 GraphicsLayerCARemote::~GraphicsLayerCARemote()
 {
-    if (m_context)
-        m_context->graphicsLayerWillLeaveContext(*this);
+    if (RefPtrAllowingPartiallyDestroyed<RemoteLayerTreeContext> protectedContext = m_context.get())
+        protectedContext->graphicsLayerWillLeaveContext(*this);
 }
 
 bool GraphicsLayerCARemote::filtersCanBeComposited(const FilterOperations& filters)
@@ -63,6 +63,7 @@
 
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayer(PlatformCALayer::LayerType layerType, PlatformCALayerClient* owner)
 {
+    RELEASE_ASSERT(m_context.get());
     auto result = PlatformCALayerRemote::create(layerType, owner, *m_context);
 
     if (result->canHaveBackingStore())
@@ -73,29 +74,34 @@
 
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayer(PlatformLayer* platformLayer, PlatformCALayerClient* owner)
 {
+    RELEASE_ASSERT(m_context.get());
     return PlatformCALayerRemote::create(platformLayer, owner, *m_context);
 }
 
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayer(WebCore::LayerHostingContextIdentifier layerHostingContextIdentifier, PlatformCALayerClient* owner)
 {
+    RELEASE_ASSERT(m_context.get());
     return PlatformCALayerRemote::create(layerHostingContextIdentifier.toUInt64(), owner, *m_context);
 }
 
 #if ENABLE(MODEL_ELEMENT)
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayer(Ref<WebCore::Model> model, PlatformCALayerClient* owner)
 {
+    RELEASE_ASSERT(m_context.get());
     return PlatformCALayerRemote::create(model, owner, *m_context);
 }
 #endif
 
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayerHost(WebCore::LayerHostingContextIdentifier identifier, PlatformCALayerClient* owner)
 {
+    RELEASE_ASSERT(m_context.get());
     return PlatformCALayerRemoteHost::create(identifier, owner, *m_context);
 }
 
 #if HAVE(AVKIT)
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformVideoLayer(WebCore::HTMLVideoElement& videoElement, PlatformCALayerClient* owner)
 {
+    RELEASE_ASSERT(m_context.get());
     return PlatformCALayerRemote::create(videoElement, owner, *m_context);
 }
 #endif
@@ -107,8 +113,8 @@
 
 void GraphicsLayerCARemote::moveToContext(RemoteLayerTreeContext& context)
 {
-    if (m_context)
-        m_context->graphicsLayerWillLeaveContext(*this);
+    if (RefPtr protectedContext = m_context.get())
+        protectedContext->graphicsLayerWillLeaveContext(*this);
 
     m_context = &context;
 
@@ -178,7 +184,8 @@ void setDestinationLayerID(WebCore::PlatformLayerIdentifier layerID)
 
 RefPtr<WebCore::GraphicsLayerAsyncContentsDisplayDelegate> GraphicsLayerCARemote::createAsyncContentsDisplayDelegate(GraphicsLayerAsyncContentsDisplayDelegate* existing)
 {
-    if (!m_context || !m_context->drawingAreaIdentifier() || !WebProcess::singleton().parentProcessConnection())
+    RefPtr protectedContext = m_context.get();
+    if (!protectedContext || !protectedContext->drawingAreaIdentifier() || !WebProcess::singleton().parentProcessConnection())
         return nullptr;
 
     RefPtr<GraphicsLayerCARemoteAsyncContentsDisplayDelegate> delegate;
@@ -187,7 +194,7 @@ void setDestinationLayerID(WebCore::PlatformLayerIdentifier layerID)
 
     if (!delegate) {
         ASSERT(!existing);
-        delegate = adoptRef(new GraphicsLayerCARemoteAsyncContentsDisplayDelegate(*WebProcess::singleton().parentProcessConnection(), m_context->drawingAreaIdentifier()));
+        delegate = adoptRef(new GraphicsLayerCARemoteAsyncContentsDisplayDelegate(*WebProcess::singleton().parentProcessConnection(), protectedContext->drawingAreaIdentifier()));
     }
 
     auto layerID = setContentsToAsyncDisplayDelegate(delegate, ContentsLayerPurpose::Canvas);
@@ -198,7 +205,7 @@ void setDestinationLayerID(WebCore::PlatformLayerIdentifier layerID)
 
 GraphicsLayer::LayerMode GraphicsLayerCARemote::layerMode() const
 {
-    if (m_context->layerHostingMode() == LayerHostingMode::InProcess)
+    if (m_context && m_context->layerHostingMode() == LayerHostingMode::InProcess)
         return GraphicsLayer::LayerMode::PlatformLayer;
     return GraphicsLayer::LayerMode::LayerHostingContextId;
 }

Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h

@@ -26,6 +26,7 @@
 #pragma once
 
 #include "LayerProperties.h"
+#include "RemoteLayerTreeContext.h"
 #include "RemoteLayerTreeTransaction.h"
 #include <WebCore/HTMLMediaElementIdentifier.h>
 #include <WebCore/PlatformCALayer.h>
@@ -43,7 +44,6 @@ struct AcceleratedEffectValues;
 
 namespace WebKit {
 
-class RemoteLayerTreeContext;
 
 using LayerHostingContextID = uint32_t;
 
@@ -249,8 +249,7 @@ class PlatformCALayerRemote : public WebCore::PlatformCALayer, public CanMakeWea
     void didCommit();
 
     void moveToContext(RemoteLayerTreeContext&);
-    void clearContext() { m_context = nullptr; }
-    RemoteLayerTreeContext* context() const { return m_context; }
+    RemoteLayerTreeContext* context() const { return m_context.get(); }
 
     void markFrontBufferVolatileForTesting() override;
     virtual void populateCreationProperties(RemoteLayerTreeTransaction::LayerCreationProperties&, const RemoteLayerTreeContext&, WebCore::PlatformCALayer::LayerType);
@@ -278,7 +277,7 @@ class PlatformCALayerRemote : public WebCore::PlatformCALayer, public CanMakeWea
 
     bool requiresCustomAppearanceUpdateOnBoundsChange() const;
 
-    WebCore::LayerPool& layerPool() override;
+    WebCore::LayerPool* layerPool() override;
 
     LayerProperties m_properties;
     WebCore::PlatformCALayerList m_children;
@@ -288,7 +287,7 @@ class PlatformCALayerRemote : public WebCore::PlatformCALayer, public CanMakeWea
     bool m_acceleratesDrawing { false };
     bool m_wantsDeepColorBackingStore { false };
 
-    RemoteLayerTreeContext* m_context;
+    WeakPtr<RemoteLayerTreeContext> m_context;
 };
 
 } // namespace WebKit

Source/WebKit/WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.mm

@@ -118,6 +118,7 @@
 
 Ref<PlatformCALayer> PlatformCALayerRemote::clone(PlatformCALayerClient* owner) const
 {
+    RELEASE_ASSERT(m_context.get());
     auto clone = PlatformCALayerRemote::create(*this, owner, *m_context);
 
     updateClonedLayerProperties(clone);
@@ -131,14 +132,14 @@
     for (const auto& layer : m_children)
         downcast<PlatformCALayerRemote>(*layer).m_superlayer = nullptr;
 
-    if (m_context)
-        m_context->layerWillLeaveContext(*this);
+    if (RefPtrAllowingPartiallyDestroyed<RemoteLayerTreeContext> protectedContext = m_context.get())
+        protectedContext->layerWillLeaveContext(*this);
 }
 
 void PlatformCALayerRemote::moveToContext(RemoteLayerTreeContext& context)
 {
-    if (m_context)
-        m_context->layerWillLeaveContext(*this);
+    if (RefPtr protectedContext = m_context.get())
+        protectedContext->layerWillLeaveContext(*this);
 
     m_context = &context;
 
@@ -272,7 +273,7 @@
 #if ENABLE(RE_DYNAMIC_CONTENT_SCALING)
 RemoteLayerBackingStore::IncludeDisplayList PlatformCALayerRemote::shouldIncludeDisplayListInBackingStore() const
 {
-    if (!m_context->useDynamicContentScalingDisplayListsForDOMRendering())
+    if (m_context && !m_context->useDynamicContentScalingDisplayListsForDOMRendering())
         return RemoteLayerBackingStore::IncludeDisplayList::No;
     if (containsBitmapOnly())
         return RemoteLayerBackingStore::IncludeDisplayList::No;
@@ -294,7 +295,7 @@
 #if PLATFORM(IOS_FAMILY)
     parameters.colorSpace = m_wantsDeepColorBackingStore ? DestinationColorSpace { extendedSRGBColorSpaceRef() } : DestinationColorSpace::SRGB();
 #else
-    if (auto displayColorSpace = m_context->displayColorSpace())
+    if (auto displayColorSpace = m_context ? m_context->displayColorSpace() : std::nullopt)
         parameters.colorSpace = displayColorSpace.value();
 #endif
 
@@ -334,8 +335,8 @@
 {
     ASSERT(m_properties.clonedLayerID == layer->layerID());
 
-    if (!m_properties.changedProperties)
-        m_context->layerPropertyChangedWhileBuildingTransaction(*this);
+    if (RefPtr protectedContext = m_context.get(); protectedContext && !m_properties.changedProperties)
+        protectedContext->layerPropertyChangedWhileBuildingTransaction(*this);
 
     m_properties.notePropertiesChanged(LayerChange::ClonedContentsChanged);
 }
@@ -457,8 +458,8 @@
 
     m_properties.notePropertiesChanged(LayerChange::AnimationsChanged);
 
-    if (m_context)
-        m_context->willStartAnimationOnLayer(*this);
+    if (RefPtr protectedContext = m_context.get())
+        protectedContext->willStartAnimationOnLayer(*this);
 }
 
 void PlatformCALayerRemote::removeAnimationForKey(const String& key)
@@ -1074,6 +1075,7 @@ static inline bool isEquivalentLayer(const PlatformCALayer* layer, const std::op
 
 Ref<PlatformCALayer> PlatformCALayerRemote::createCompatibleLayer(PlatformCALayer::LayerType layerType, PlatformCALayerClient* client) const
 {
+    RELEASE_ASSERT(m_context.get());
     return PlatformCALayerRemote::create(layerType, client, *m_context);
 }
 
@@ -1096,9 +1098,9 @@ static inline bool isEquivalentLayer(const PlatformCALayer* layer, const std::op
     return m_properties.backingStoreOrProperties.store->bytesPerPixel();
 }
 
-LayerPool& PlatformCALayerRemote::layerPool()
+LayerPool* PlatformCALayerRemote::layerPool()
 {
-    return m_context->layerPool();
+    return m_context ? &m_context->layerPool() : nullptr;
 }
 
 #if ENABLE(THREADED_ANIMATION_RESOLUTION)

Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.h

@@ -46,9 +46,14 @@ class WebFrame;
 class WebPage;
 
 // FIXME: This class doesn't do much now. Roll into RemoteLayerTreeDrawingArea?
-class RemoteLayerTreeContext : public WebCore::GraphicsLayerFactory {
+class RemoteLayerTreeContext : public WebCore::GraphicsLayerFactory,  public RefCounted<RemoteLayerTreeContext>, public CanMakeWeakPtr<RemoteLayerTreeContext> {
+    WTF_MAKE_FAST_ALLOCATED;
 public:
-    explicit RemoteLayerTreeContext(WebPage&);
+    static Ref<RemoteLayerTreeContext> create(WebPage& webpage)
+    {
+        return adoptRef(*new RemoteLayerTreeContext(webpage));
+    }
+
     ~RemoteLayerTreeContext();
 
     void layerDidEnterContext(PlatformCALayerRemote&, WebCore::PlatformCALayer::LayerType);
@@ -101,6 +106,8 @@ class RemoteLayerTreeContext : public WebCore::GraphicsLayerFactory {
     WebPage& webPage() { return m_webPage; }
 
 private:
+    explicit RemoteLayerTreeContext(WebPage&);
+
     // WebCore::GraphicsLayerFactory
     Ref<WebCore::GraphicsLayer> createGraphicsLayer(WebCore::GraphicsLayer::Type, WebCore::GraphicsLayerClient&) override;
 

Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.mm

@@ -54,14 +54,6 @@
 
 RemoteLayerTreeContext::~RemoteLayerTreeContext()
 {
-    for (auto& layer : m_livePlatformLayers.values())
-        layer->clearContext();
-
-
-    auto graphicsLayers = m_liveGraphicsLayers;
-    for (auto& layer : graphicsLayers)
-        Ref { layer.get() }->clearContext();
-
     // Make sure containers are empty before destruction to avoid hitting the assertion in CanMakeCheckedPtr.
     m_livePlatformLayers.clear();
     m_liveGraphicsLayers.clear();

Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h

@@ -162,7 +162,7 @@ class RemoteLayerTreeDrawingArea : public DrawingArea, public WebCore::GraphicsL
         std::atomic<bool> m_hasPendingFlush { false };
     };
 
-    std::unique_ptr<RemoteLayerTreeContext> m_remoteLayerTreeContext;
+    Ref<RemoteLayerTreeContext> m_remoteLayerTreeContext;
 
     struct RootLayerInfo {
         Ref<WebCore::GraphicsLayer> layer;

Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm

@@ -65,7 +65,7 @@
 
 RemoteLayerTreeDrawingArea::RemoteLayerTreeDrawingArea(WebPage& webPage, const WebPageCreationParameters& parameters)
     : DrawingArea(DrawingAreaType::RemoteLayerTree, parameters.drawingAreaIdentifier, webPage)
-    , m_remoteLayerTreeContext(makeUnique<RemoteLayerTreeContext>(webPage))
+    , m_remoteLayerTreeContext(RemoteLayerTreeContext::create(webPage))
     , m_updateRenderingTimer(*this, &RemoteLayerTreeDrawingArea::updateRendering)
     , m_commitQueue(WorkQueue::create("com.apple.WebKit.WebContent.RemoteLayerTreeDrawingArea.CommitQueue"_s, WorkQueue::QOS::UserInteractive))
     , m_backingStoreFlusher(BackingStoreFlusher::create(*WebProcess::singleton().parentProcessConnection()))
@@ -92,7 +92,7 @@
 
 GraphicsLayerFactory* RemoteLayerTreeDrawingArea::graphicsLayerFactory()
 {
-    return m_remoteLayerTreeContext.get();
+    return m_remoteLayerTreeContext.ptr();
 }
 
 RefPtr<DisplayRefreshMonitor> RemoteLayerTreeDrawingArea::createDisplayRefreshMonitor(PlatformDisplayID displayID)
@@ -537,7 +537,7 @@
 
     RemoteLayerTreeDrawingArea& oldRemoteDrawingArea = static_cast<RemoteLayerTreeDrawingArea&>(oldDrawingArea);
 
-    m_remoteLayerTreeContext->adoptLayersFromContext(*oldRemoteDrawingArea.m_remoteLayerTreeContext);
+    m_remoteLayerTreeContext->adoptLayersFromContext(oldRemoteDrawingArea.m_remoteLayerTreeContext);
 }
 
 void RemoteLayerTreeDrawingArea::scheduleRenderingUpdateTimerFired()