Scrolling state nodes should hold references to GraphicsLayers
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Mar 2019 02:05:23 +0000 (02:05 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Mar 2019 02:05:23 +0000 (02:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195844
<rdar://problem/48949634>

Reviewed by Tim Horton.

GraphicsLayers are refcounted, and the scrolling tree keeps GraphicsLayer pointers,
so for safely the scrolling tree should store RefPtr<GraphicsLayer> instead.

I removed the union (since it would be weird with a RefPtr and raw pointer). This code
should probably use WTF::Variant<> in future.

* page/scrolling/ScrollingStateNode.h:
(WebCore::LayerRepresentation::LayerRepresentation):
(WebCore::LayerRepresentation::operator GraphicsLayer* const):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243126 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingStateNode.h

index c5e607c..9562b62 100644 (file)
@@ -1,3 +1,21 @@
+2019-03-18  Simon Fraser  <simon.fraser@apple.com>
+
+        Scrolling state nodes should hold references to GraphicsLayers
+        https://bugs.webkit.org/show_bug.cgi?id=195844
+        <rdar://problem/48949634>
+
+        Reviewed by Tim Horton.
+
+        GraphicsLayers are refcounted, and the scrolling tree keeps GraphicsLayer pointers,
+        so for safely the scrolling tree should store RefPtr<GraphicsLayer> instead.
+
+        I removed the union (since it would be weird with a RefPtr and raw pointer). This code
+        should probably use WTF::Variant<> in future.
+
+        * page/scrolling/ScrollingStateNode.h:
+        (WebCore::LayerRepresentation::LayerRepresentation):
+        (WebCore::LayerRepresentation::operator GraphicsLayer* const):
+
 2019-03-18  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r243092 and r243096.
index 358648f..532c0b7 100644 (file)
@@ -58,11 +58,7 @@ public:
         PlatformLayerIDRepresentation
     };
 
-    LayerRepresentation()
-        : m_graphicsLayer(nullptr)
-        , m_layerID(0)
-        , m_representation(EmptyRepresentation)
-    { }
+    LayerRepresentation() = default;
 
     LayerRepresentation(GraphicsLayer* graphicsLayer)
         : m_graphicsLayer(graphicsLayer)
@@ -72,15 +68,13 @@ public:
 
     LayerRepresentation(PlatformLayer* platformLayer)
         : m_typelessPlatformLayer(makePlatformLayerTypeless(platformLayer))
-        , m_layerID(0)
         , m_representation(PlatformLayerRepresentation)
     {
         retainPlatformLayer(m_typelessPlatformLayer);
     }
 
     LayerRepresentation(GraphicsLayer::PlatformLayerID layerID)
-        : m_graphicsLayer(nullptr)
-        , m_layerID(layerID)
+        : m_layerID(layerID)
         , m_representation(PlatformLayerIDRepresentation)
     {
     }
@@ -103,7 +97,7 @@ public:
     operator GraphicsLayer*() const
     {
         ASSERT(m_representation == GraphicsLayerRepresentation);
-        return m_graphicsLayer;
+        return m_graphicsLayer.get();
     }
 
     operator PlatformLayer*() const
@@ -179,13 +173,10 @@ private:
     WEBCORE_EXPORT static PlatformLayer* makePlatformLayerTyped(void* typelessPlatformLayer);
     WEBCORE_EXPORT static void* makePlatformLayerTypeless(PlatformLayer*);
 
-    union {
-        GraphicsLayer* m_graphicsLayer;
-        void* m_typelessPlatformLayer;
-    };
-
-    GraphicsLayer::PlatformLayerID m_layerID;
-    Type m_representation;
+    RefPtr<GraphicsLayer> m_graphicsLayer;
+    void* m_typelessPlatformLayer { nullptr };
+    GraphicsLayer::PlatformLayerID m_layerID { 0 };
+    Type m_representation { EmptyRepresentation };
 };
 
 class ScrollingStateNode : public RefCounted<ScrollingStateNode> {