Crash under RemoteLayerTreePropertyApplier::applyProperties when reat…

…taching to old process

https://bugs.webkit.org/show_bug.cgi?id=194845
<rdar://problem/47944579>

Reviewed by Antti Koivisto.

Source/WebKit:

New test: ProcessSwap.PageOverlayLayerPersistence

* Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:
(WebKit::RemoteLayerTreeTransaction::LayerProperties::notePropertiesChanged):
Keep track of all properties that have ever changed on a layer, so we
can re-send them if a layer moves between contexts.

* WebProcess/WebPage/DrawingArea.cpp:
(WebKit::DrawingArea::~DrawingArea):
(WebKit::DrawingArea::removeMessageReceiverIfNeeded):
* WebProcess/WebPage/DrawingArea.h:
(WebKit::DrawingArea::layerHostDidFlushLayers):
Make it possible to tear down DrawingArea's MessageReceiver before it is destroyed,
so that we can keep two DrawingAreas alive in a single process for a short time.

(WebKit::DrawingArea::adoptLayersFromDrawingArea):
Add adoptLayersFromDrawingArea; see below for its only useful implementation.

* WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.cpp:
(WebKit::GraphicsLayerCARemote::GraphicsLayerCARemote):
(WebKit::GraphicsLayerCARemote::~GraphicsLayerCARemote):
(WebKit::GraphicsLayerCARemote::createPlatformCALayer):
(WebKit::GraphicsLayerCARemote::createPlatformCALayerForEmbeddedView):
(WebKit::GraphicsLayerCARemote::moveToContext):
* WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.h:
Make it possible to move a GraphicsLayerCARemote between RemoteLayerTreeContexts.

* WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp:
(WebKit::PlatformCALayerRemote::create):
(WebKit::PlatformCALayerRemote::createForEmbeddedView):
(WebKit::PlatformCALayerRemote::~PlatformCALayerRemote):
(WebKit::PlatformCALayerRemote::moveToContext):
* WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h:
* WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemoteCustom.mm:
(WebKit::PlatformCALayerRemoteCustom::create):
(WebKit::PlatformCALayerRemoteCustom::clone const):
Make it possible to move a PlatformCALayerRemote between RemoteLayerTreeContexts.

* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.h:
* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.mm:
(WebKit::RemoteLayerTreeContext::~RemoteLayerTreeContext):
(WebKit::RemoteLayerTreeContext::adoptLayersFromContext):
(WebKit::RemoteLayerTreeContext::layerDidEnterContext):
(WebKit::RemoteLayerTreeContext::layerWillLeaveContext):
(WebKit::RemoteLayerTreeContext::graphicsLayerDidEnterContext):
(WebKit::RemoteLayerTreeContext::graphicsLayerWillLeaveContext):
(WebKit::RemoteLayerTreeContext::layerWasCreated): Deleted.
(WebKit::RemoteLayerTreeContext::layerWillBeDestroyed): Deleted.
Keep track of all GraphicsLayerCARemote instances in the context, like we
do for PlatformCALayerRemote, so that we can update their context backpointers if needed.

Also make it possible to move all outstanding layers to a new context.

* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::adoptLayersFromDrawingArea):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::reinitializeWebPage):
When doing a DrawingArea swap, transition all layers from the old context
to the new one. In order to do this, we temporarily keep both DrawingAreas
alive, but make use of the new mechanism to remove the old one's MessageReceiver
before installing the new one, so that destroying the old one later doesn't
remove it (avoiding re-introducing bug 189481).

Tools:

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:


Canonical link: https://commits.webkit.org/209273@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241899 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog

@@ -1,3 +1,75 @@
+2019-02-21  Tim Horton  <timothy_horton@apple.com>
+
+        Crash under RemoteLayerTreePropertyApplier::applyProperties when reattaching to old process
+        https://bugs.webkit.org/show_bug.cgi?id=194845
+        <rdar://problem/47944579>
+
+        Reviewed by Antti Koivisto.
+
+        New test: ProcessSwap.PageOverlayLayerPersistence
+
+        * Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:
+        (WebKit::RemoteLayerTreeTransaction::LayerProperties::notePropertiesChanged):
+        Keep track of all properties that have ever changed on a layer, so we
+        can re-send them if a layer moves between contexts.
+
+        * WebProcess/WebPage/DrawingArea.cpp:
+        (WebKit::DrawingArea::~DrawingArea):
+        (WebKit::DrawingArea::removeMessageReceiverIfNeeded):
+        * WebProcess/WebPage/DrawingArea.h:
+        (WebKit::DrawingArea::layerHostDidFlushLayers):
+        Make it possible to tear down DrawingArea's MessageReceiver before it is destroyed,
+        so that we can keep two DrawingAreas alive in a single process for a short time.
+
+        (WebKit::DrawingArea::adoptLayersFromDrawingArea):
+        Add adoptLayersFromDrawingArea; see below for its only useful implementation.
+
+        * WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.cpp:
+        (WebKit::GraphicsLayerCARemote::GraphicsLayerCARemote):
+        (WebKit::GraphicsLayerCARemote::~GraphicsLayerCARemote):
+        (WebKit::GraphicsLayerCARemote::createPlatformCALayer):
+        (WebKit::GraphicsLayerCARemote::createPlatformCALayerForEmbeddedView):
+        (WebKit::GraphicsLayerCARemote::moveToContext):
+        * WebProcess/WebPage/RemoteLayerTree/GraphicsLayerCARemote.h:
+        Make it possible to move a GraphicsLayerCARemote between RemoteLayerTreeContexts.
+
+        * WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.cpp:
+        (WebKit::PlatformCALayerRemote::create):
+        (WebKit::PlatformCALayerRemote::createForEmbeddedView):
+        (WebKit::PlatformCALayerRemote::~PlatformCALayerRemote):
+        (WebKit::PlatformCALayerRemote::moveToContext):
+        * WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemote.h:
+        * WebProcess/WebPage/RemoteLayerTree/PlatformCALayerRemoteCustom.mm:
+        (WebKit::PlatformCALayerRemoteCustom::create):
+        (WebKit::PlatformCALayerRemoteCustom::clone const):
+        Make it possible to move a PlatformCALayerRemote between RemoteLayerTreeContexts.
+
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.h:
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeContext.mm:
+        (WebKit::RemoteLayerTreeContext::~RemoteLayerTreeContext):
+        (WebKit::RemoteLayerTreeContext::adoptLayersFromContext):
+        (WebKit::RemoteLayerTreeContext::layerDidEnterContext):
+        (WebKit::RemoteLayerTreeContext::layerWillLeaveContext):
+        (WebKit::RemoteLayerTreeContext::graphicsLayerDidEnterContext):
+        (WebKit::RemoteLayerTreeContext::graphicsLayerWillLeaveContext):
+        (WebKit::RemoteLayerTreeContext::layerWasCreated): Deleted.
+        (WebKit::RemoteLayerTreeContext::layerWillBeDestroyed): Deleted.
+        Keep track of all GraphicsLayerCARemote instances in the context, like we
+        do for PlatformCALayerRemote, so that we can update their context backpointers if needed.
+
+        Also make it possible to move all outstanding layers to a new context.
+
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
+        * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
+        (WebKit::RemoteLayerTreeDrawingArea::adoptLayersFromDrawingArea):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::reinitializeWebPage):
+        When doing a DrawingArea swap, transition all layers from the old context
+        to the new one. In order to do this, we temporarily keep both DrawingAreas
+        alive, but make use of the new mechanism to remove the old one's MessageReceiver
+        before installing the new one, so that destroying the old one later doesn't
+        remove it (avoiding re-introducing bug 189481).
+
 2019-02-21  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed API test fix after r241855.

Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h

@@ -119,6 +119,7 @@ class RemoteLayerTreeTransaction {
         void notePropertiesChanged(OptionSet<LayerChange> changeFlags)
         {
             changedProperties.add(changeFlags);
+            everChangedProperties.add(changeFlags);
         }
 
         void resetChangedProperties()
@@ -127,6 +128,7 @@ class RemoteLayerTreeTransaction {
         }
 
         OptionSet<LayerChange> changedProperties;
+        OptionSet<LayerChange> everChangedProperties;
 
         String name;
         std::unique_ptr<WebCore::TransformationMatrix> transform;

Source/WebKit/WebProcess/WebPage/DrawingArea.cpp

@@ -78,7 +78,7 @@ DrawingArea::DrawingArea(DrawingAreaType type, WebPage& webPage)
 
 DrawingArea::~DrawingArea()
 {
-    WebProcess::singleton().removeMessageReceiver(Messages::DrawingArea::messageReceiverName(), m_webPage.pageID());
+    removeMessageReceiverIfNeeded();
 }
 
 void DrawingArea::dispatchAfterEnsuringUpdatedScrollPosition(WTF::Function<void ()>&& function)
@@ -94,4 +94,12 @@ RefPtr<WebCore::DisplayRefreshMonitor> DrawingArea::createDisplayRefreshMonitor(
 }
 #endif
 
+void DrawingArea::removeMessageReceiverIfNeeded()
+{
+    if (m_hasRemovedMessageReceiver)
+        return;
+    m_hasRemovedMessageReceiver = true;
+    WebProcess::singleton().removeMessageReceiver(Messages::DrawingArea::messageReceiverName(), m_webPage.pageID());
+}
+
 } // namespace WebKit

Source/WebKit/WebProcess/WebPage/DrawingArea.h

@@ -140,7 +140,7 @@ class DrawingArea : public IPC::MessageReceiver {
     virtual void updateGeometry(const WebCore::IntSize& viewSize, bool flushSynchronously, const WTF::MachSendRight& fencePort) { }
 #endif
 
-    virtual void layerHostDidFlushLayers() { };
+    virtual void layerHostDidFlushLayers() { }
 
 #if USE(COORDINATED_GRAPHICS)
     virtual void didChangeViewportAttributes(WebCore::ViewportAttributes&&) = 0;
@@ -150,6 +150,10 @@ class DrawingArea : public IPC::MessageReceiver {
     virtual void deviceOrPageScaleFactorChanged() = 0;
 #endif
 
+    virtual void adoptLayersFromDrawingArea(DrawingArea&) { }
+
+    void removeMessageReceiverIfNeeded();
+
 protected:
     DrawingArea(DrawingAreaType, WebPage&);
 
@@ -186,6 +190,8 @@ class DrawingArea : public IPC::MessageReceiver {
     virtual void setNativeSurfaceHandleForCompositing(uint64_t) = 0;
     virtual void destroyNativeSurfaceHandleForCompositing(bool&) = 0;
 #endif
+
+    bool m_hasRemovedMessageReceiver { false };
 };
 
 } // namespace WebKit

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

@@ -28,13 +28,23 @@
 
 #include "PlatformCAAnimationRemote.h"
 #include "PlatformCALayerRemote.h"
+#include "RemoteLayerTreeContext.h"
 #include <WebCore/PlatformScreen.h>
 
 namespace WebKit {
 using namespace WebCore;
 
+GraphicsLayerCARemote::GraphicsLayerCARemote(Type layerType, GraphicsLayerClient& client, RemoteLayerTreeContext& context)
+    : GraphicsLayerCA(layerType, client)
+    , m_context(&context)
+{
+    context.graphicsLayerDidEnterContext(*this);
+}
+
 GraphicsLayerCARemote::~GraphicsLayerCARemote()
 {
+    if (m_context)
+        m_context->graphicsLayerWillLeaveContext(*this);
 }
 
 bool GraphicsLayerCARemote::filtersCanBeComposited(const FilterOperations& filters)
@@ -44,7 +54,7 @@ bool GraphicsLayerCARemote::filtersCanBeComposited(const FilterOperations& filte
 
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayer(PlatformCALayer::LayerType layerType, PlatformCALayerClient* owner)
 {
-    auto result = PlatformCALayerRemote::create(layerType, owner, m_context);
+    auto result = PlatformCALayerRemote::create(layerType, owner, *m_context);
 
     if (result->canHaveBackingStore())
         result->setWantsDeepColorBackingStore(screenSupportsExtendedColor());
@@ -54,17 +64,27 @@ Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayer(PlatformCALaye
 
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayer(PlatformLayer* platformLayer, PlatformCALayerClient* owner)
 {
-    return PlatformCALayerRemote::create(platformLayer, owner, m_context);
+    return PlatformCALayerRemote::create(platformLayer, owner, *m_context);
 }
 
 Ref<PlatformCALayer> GraphicsLayerCARemote::createPlatformCALayerForEmbeddedView(PlatformCALayer::LayerType layerType, GraphicsLayer::EmbeddedViewID embeddedViewID, PlatformCALayerClient* owner)
 {
-    return PlatformCALayerRemote::createForEmbeddedView(layerType, embeddedViewID, owner, m_context);
+    return PlatformCALayerRemote::createForEmbeddedView(layerType, embeddedViewID, owner, *m_context);
 }
 
 Ref<PlatformCAAnimation> GraphicsLayerCARemote::createPlatformCAAnimation(PlatformCAAnimation::AnimationType type, const String& keyPath)
 {
     return PlatformCAAnimationRemote::create(type, keyPath);
 }
 
+void GraphicsLayerCARemote::moveToContext(RemoteLayerTreeContext& context)
+{
+    if (m_context)
+        m_context->graphicsLayerWillLeaveContext(*this);
+
+    m_context = &context;
+
+    context.graphicsLayerDidEnterContext(*this);
+}
+
 } // namespace WebKit

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

@@ -34,16 +34,14 @@ class RemoteLayerTreeContext;
 
 class GraphicsLayerCARemote final : public WebCore::GraphicsLayerCA {
 public:
-    GraphicsLayerCARemote(Type layerType, WebCore::GraphicsLayerClient& client, RemoteLayerTreeContext& context)
-        : GraphicsLayerCA(layerType, client)
-        , m_context(context)
-    {
-    }
-
+    GraphicsLayerCARemote(Type layerType, WebCore::GraphicsLayerClient&, RemoteLayerTreeContext&);
     virtual ~GraphicsLayerCARemote();
 
     bool filtersCanBeComposited(const WebCore::FilterOperations& filters) override;
 
+    void moveToContext(RemoteLayerTreeContext&);
+    void clearContext() { m_context = nullptr; }
+
 private:
     bool isGraphicsLayerCARemote() const override { return true; }
 
@@ -55,7 +53,7 @@ class GraphicsLayerCARemote final : public WebCore::GraphicsLayerCA {
     // PlatformCALayerRemote can't currently proxy directly composited image contents, so opt out of this optimization.
     bool shouldDirectlyCompositeImage(WebCore::Image*) const override { return false; }
 
-    RemoteLayerTreeContext& m_context;
+    RemoteLayerTreeContext* m_context;
 };
 
 } // namespace WebKit

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

@@ -51,7 +51,7 @@ Ref<PlatformCALayerRemote> PlatformCALayerRemote::create(LayerType layerType, Pl
     else
         layer = adoptRef(new PlatformCALayerRemote(layerType, owner, context));
 
-    context.layerWasCreated(*layer, layerType);
+    context.layerDidEnterContext(*layer, layerType);
 
     return layer.releaseNonNull();
 }
@@ -64,15 +64,15 @@ Ref<PlatformCALayerRemote> PlatformCALayerRemote::create(PlatformLayer *platform
 Ref<PlatformCALayerRemote> PlatformCALayerRemote::createForEmbeddedView(LayerType layerType, GraphicsLayer::EmbeddedViewID embeddedViewID, PlatformCALayerClient* owner, RemoteLayerTreeContext& context)
 {
     RefPtr<PlatformCALayerRemote> layer = adoptRef(new PlatformCALayerRemote(layerType, embeddedViewID, owner, context));
-    context.layerWasCreated(*layer, layerType);
+    context.layerDidEnterContext(*layer, layerType);
     return layer.releaseNonNull();
 }
 
 Ref<PlatformCALayerRemote> PlatformCALayerRemote::create(const PlatformCALayerRemote& other, WebCore::PlatformCALayerClient* owner, RemoteLayerTreeContext& context)
 {
     auto layer = adoptRef(*new PlatformCALayerRemote(other, owner, context));
 
-    context.layerWasCreated(layer.get(), other.layerType());
+    context.layerDidEnterContext(layer.get(), other.layerType());
 
     return layer;
 }
@@ -116,7 +116,19 @@ PlatformCALayerRemote::~PlatformCALayerRemote()
         downcast<PlatformCALayerRemote>(*layer).m_superlayer = nullptr;
 
     if (m_context)
-        m_context->layerWillBeDestroyed(*this);
+        m_context->layerWillLeaveContext(*this);
+}
+
+void PlatformCALayerRemote::moveToContext(RemoteLayerTreeContext& context)
+{
+    if (m_context)
+        m_context->layerWillLeaveContext(*this);
+
+    m_context = &context;
+
+    context.layerDidEnterContext(*this, layerType());
+
+    m_properties.notePropertiesChanged(m_properties.everChangedProperties);
 }
 
 void PlatformCALayerRemote::updateClonedLayerProperties(PlatformCALayerRemote& clone, bool copyContents) const

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

@@ -195,6 +195,7 @@ class PlatformCALayerRemote : public WebCore::PlatformCALayer {
 
     void didCommit();
 
+    void moveToContext(RemoteLayerTreeContext&);
     void clearContext() { m_context = nullptr; }
     RemoteLayerTreeContext* context() const { return m_context; }
 

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

@@ -48,7 +48,7 @@
 Ref<PlatformCALayerRemote> PlatformCALayerRemoteCustom::create(PlatformLayer *platformLayer, PlatformCALayerClient* owner, RemoteLayerTreeContext& context)
 {
     auto layer = adoptRef(*new PlatformCALayerRemoteCustom(PlatformCALayerCocoa::layerTypeForPlatformLayer(platformLayer), platformLayer, owner, context));
-    context.layerWasCreated(layer.get(), layer->layerType());
+    context.layerDidEnterContext(layer.get(), layer->layerType());
     return WTFMove(layer);
 }
 
@@ -126,7 +126,7 @@
     }
 
     auto clone = adoptRef(*new PlatformCALayerRemoteCustom(layerType(), clonedLayer.get(), owner, *context()));
-    context()->layerWasCreated(clone.get(), clone->layerType());
+    context()->layerDidEnterContext(clone.get(), clone->layerType());
 
     updateClonedLayerProperties(clone.get(), copyContents);
 

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

@@ -35,6 +35,7 @@
 
 namespace WebKit {
 
+class GraphicsLayerCARemote;
 class PlatformCALayerRemote;
 class WebPage;
 
@@ -44,8 +45,11 @@ class RemoteLayerTreeContext : public WebCore::GraphicsLayerFactory {
     explicit RemoteLayerTreeContext(WebPage&);
     ~RemoteLayerTreeContext();
 
-    void layerWasCreated(PlatformCALayerRemote&, WebCore::PlatformCALayer::LayerType);
-    void layerWillBeDestroyed(PlatformCALayerRemote&);
+    void layerDidEnterContext(PlatformCALayerRemote&, WebCore::PlatformCALayer::LayerType);
+    void layerWillLeaveContext(PlatformCALayerRemote&);
+
+    void graphicsLayerDidEnterContext(GraphicsLayerCARemote&);
+    void graphicsLayerWillLeaveContext(GraphicsLayerCARemote&);
 
     void backingStoreWasCreated(RemoteLayerBackingStore&);
     void backingStoreWillBeDestroyed(RemoteLayerBackingStore&);
@@ -72,6 +76,8 @@ class RemoteLayerTreeContext : public WebCore::GraphicsLayerFactory {
     void setNextFlushIsForImmediatePaint(bool nextFlushIsForImmediatePaint) { m_nextFlushIsForImmediatePaint = nextFlushIsForImmediatePaint; }
     bool nextFlushIsForImmediatePaint() const { return m_nextFlushIsForImmediatePaint; }
 
+    void adoptLayersFromContext(RemoteLayerTreeContext&);
+
 private:
     // WebCore::GraphicsLayerFactory
     Ref<WebCore::GraphicsLayer> createGraphicsLayer(WebCore::GraphicsLayer::Type, WebCore::GraphicsLayerClient&) override;
@@ -81,9 +87,11 @@ class RemoteLayerTreeContext : public WebCore::GraphicsLayerFactory {
     HashMap<WebCore::GraphicsLayer::PlatformLayerID, RemoteLayerTreeTransaction::LayerCreationProperties> m_createdLayers;
     Vector<WebCore::GraphicsLayer::PlatformLayerID> m_destroyedLayers;
 
-    HashMap<WebCore::GraphicsLayer::PlatformLayerID, PlatformCALayerRemote*> m_liveLayers;
+    HashMap<WebCore::GraphicsLayer::PlatformLayerID, PlatformCALayerRemote*> m_livePlatformLayers;
     HashMap<WebCore::GraphicsLayer::PlatformLayerID, PlatformCALayerRemote*> m_layersWithAnimations;
 
+    HashSet<GraphicsLayerCARemote*> m_liveGraphicsLayers;
+
     RemoteLayerBackingStoreCollection m_backingStoreCollection;
 
     RemoteLayerTreeTransaction* m_currentTransaction;