Respond to feedback from 272882@main

https://bugs.webkit.org/show_bug.cgi?id=267422 rdar://120853915 Reviewed by Charlie Wolfe. Rename remotePageProcessCrashed to remotePageProcessDidTerminate Have the UI process get the ProcessIdentifier instead of trusting a web process to send its own. * Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTree.serialization.in: * Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h: (WebKit::RemoteLayerTreeTransaction::processIdentifier const): Deleted. (WebKit::RemoteLayerTreeTransaction::setProcessIdentifier): Deleted. * Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp: (WebKit::connectionToProcessMap): (WebKit::AuxiliaryProcessProxy::~AuxiliaryProcessProxy): (WebKit::AuxiliaryProcessProxy::didFinishLaunching): (WebKit::AuxiliaryProcessProxy::shutDownProcess): (WebKit::AuxiliaryProcessProxy::fromConnection): * Source/WebKit/UIProcess/AuxiliaryProcessProxy.h: * Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.h: * Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm: (WebKit::RemoteLayerTreeHost::updateLayerTree): * Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm: (WebKit::RemoteLayerTreeDrawingArea::updateRendering): Canonical link: https://commits.webkit.org/278911@main
WebKit · May 17, 2024 · 84c5089 · 84c5089
1 parent c06995e
commit 84c5089
Show file tree

Hide file tree

Showing 11 changed files with 45 additions and 20 deletions.
diff --git a/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTree.serialization.in b/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTree.serialization.in
@@ -113,7 +113,6 @@ header: "RemoteLayerBackingStore.h"
 [LegacyPopulateFromEmptyConstructor] class WebKit::RemoteLayerTreeTransaction {
 {
     WebCore::PlatformLayerIdentifier m_rootLayerID;
-    WebCore::ProcessIdentifier m_processIdentifier;
     WebKit::ChangedLayers m_changedLayers;
 
     Markable<WebCore::LayerHostingContextIdentifier> m_remoteContextHostedIdentifier;

diff --git a/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h b/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h
@@ -39,7 +39,6 @@
 #include <WebCore/LayoutMilestone.h>
 #include <WebCore/MediaPlayerEnums.h>
 #include <WebCore/PlatformCALayer.h>
-#include <WebCore/ProcessIdentifier.h>
 #include <WebCore/ScrollTypes.h>
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
@@ -127,9 +126,6 @@ class RemoteLayerTreeTransaction {
     void setDestroyedLayerIDs(Vector<WebCore::PlatformLayerIdentifier>);
     void setLayerIDsWithNewlyUnreachableBackingStore(Vector<WebCore::PlatformLayerIdentifier>);
 
-    WebCore::ProcessIdentifier processIdentifier() const { return m_processIdentifier; }
-    void setProcessIdentifier(WebCore::ProcessIdentifier processIdentifier) { m_processIdentifier = processIdentifier; }
-
 #if !defined(NDEBUG) || !LOG_DISABLED
     String description() const;
     void dump() const;
@@ -252,7 +248,6 @@ class RemoteLayerTreeTransaction {
     friend struct IPC::ArgumentCoder<RemoteLayerTreeTransaction, void>;
 
     WebCore::PlatformLayerIdentifier m_rootLayerID;
-    WebCore::ProcessIdentifier m_processIdentifier;
     ChangedLayers m_changedLayers;
 
     Markable<WebCore::LayerHostingContextIdentifier> m_remoteContextHostedIdentifier;

diff --git a/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp b/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp
@@ -59,6 +59,12 @@
 
 namespace WebKit {
 
+static HashMap<IPC::Connection::UniqueID, WeakPtr<AuxiliaryProcessProxy>>& connectionToProcessMap()
+{
+    static MainThreadNeverDestroyed<HashMap<IPC::Connection::UniqueID, WeakPtr<AuxiliaryProcessProxy>>> map;
+    return map.get();
+}
+
 static Seconds adjustedTimeoutForThermalState(Seconds timeout)
 {
 #if PLATFORM(VISION)
@@ -93,6 +99,10 @@ AuxiliaryProcessProxy::~AuxiliaryProcessProxy()
 #if ENABLE(EXTENSION_CAPABILITIES)
     ASSERT(m_extensionCapabilityGrants.isEmpty());
 #endif
+    if (m_connection) {
+        ASSERT(connectionToProcessMap().get(m_connection->uniqueID()) == this);
+        connectionToProcessMap().remove(m_connection->uniqueID());
+    }
 }
 
 void AuxiliaryProcessProxy::populateOverrideLanguagesLaunchOptions(ProcessLauncher::LaunchOptions& launchOptions) const
@@ -335,6 +345,8 @@ void AuxiliaryProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::C
 
     RefPtr connection = IPC::Connection::createServerConnection(connectionIdentifier, Thread::QOS::UserInteractive);
     m_connection = connection.copyRef();
+    auto addResult = connectionToProcessMap().add(m_connection->uniqueID(), *this);
+    ASSERT_UNUSED(addResult, addResult.isNewEntry);
 
     connectionWillOpen(*connection);
     connection->open(*this);
@@ -419,10 +431,17 @@ void AuxiliaryProcessProxy::shutDownProcess()
         send(Messages::AuxiliaryProcess::ShutDown(), 0);
 
     connection->invalidate();
+    ASSERT(connectionToProcessMap().get(m_connection->uniqueID()) == this);
+    connectionToProcessMap().remove(m_connection->uniqueID());
     m_connection = nullptr;
     m_responsivenessTimer.invalidate();
 }
 
+AuxiliaryProcessProxy* AuxiliaryProcessProxy::fromConnection(const IPC::Connection& connection)
+{
+    return connectionToProcessMap().get(connection.uniqueID()).get();
+}
+
 void AuxiliaryProcessProxy::setProcessSuppressionEnabled(bool processSuppressionEnabled)
 {
 #if PLATFORM(COCOA)

diff --git a/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h b/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h
@@ -132,6 +132,7 @@ class AuxiliaryProcessProxy
     {
         return m_connection == &connection;
     }
+    static AuxiliaryProcessProxy* fromConnection(const IPC::Connection&);
 
     void addMessageReceiver(IPC::ReceiverName, IPC::MessageReceiver&);
     void addMessageReceiver(IPC::ReceiverName, uint64_t destinationID, IPC::MessageReceiver&);

diff --git a/Source/WebKit/UIProcess/DrawingAreaProxy.h b/Source/WebKit/UIProcess/DrawingAreaProxy.h
@@ -150,7 +150,7 @@ class DrawingAreaProxy : public IPC::MessageReceiver, public IPC::MessageSender,
     virtual void addRemotePageDrawingAreaProxy(RemotePageDrawingAreaProxy&) { }
     virtual void removeRemotePageDrawingAreaProxy(RemotePageDrawingAreaProxy&) { }
 
-    virtual void remotePageProcessCrashed(WebCore::ProcessIdentifier) { }
+    virtual void remotePageProcessDidTerminate(WebCore::ProcessIdentifier) { }
 
 protected:
     DrawingAreaProxy(DrawingAreaType, WebPageProxy&, WebProcessProxy&);

diff --git a/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.h b/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.h
@@ -99,7 +99,7 @@ class RemoteLayerTreeDrawingAreaProxy : public DrawingAreaProxy {
 
 private:
 
-    void remotePageProcessCrashed(WebCore::ProcessIdentifier) final;
+    void remotePageProcessDidTerminate(WebCore::ProcessIdentifier) final;
     void sizeDidChange() final;
     void deviceScaleFactorDidChange() final;
     void windowKindDidChange() final;

diff --git a/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm b/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm
@@ -105,14 +105,14 @@
     sendUpdateGeometry();
 }
 
-void RemoteLayerTreeDrawingAreaProxy::remotePageProcessCrashed(WebCore::ProcessIdentifier processIdentifier)
+void RemoteLayerTreeDrawingAreaProxy::remotePageProcessDidTerminate(WebCore::ProcessIdentifier processIdentifier)
 {
     if (!m_remoteLayerTreeHost)
         return;
 
     if (auto* scrollingCoordinator = m_webPageProxy->scrollingCoordinatorProxy()) {
         scrollingCoordinator->willCommitLayerAndScrollingTrees();
-        m_remoteLayerTreeHost->remotePageProcessCrashed(processIdentifier);
+        m_remoteLayerTreeHost->remotePageProcessDidTerminate(processIdentifier);
         scrollingCoordinator->didCommitLayerAndScrollingTrees();
     }
 }
@@ -281,7 +281,7 @@
         if (layerTreeTransaction.hasAnyLayerChanges())
             ++m_countOfTransactionsWithNonEmptyLayerChanges;
 
-        if (m_remoteLayerTreeHost->updateLayerTree(layerTreeTransaction)) {
+        if (m_remoteLayerTreeHost->updateLayerTree(connection, layerTreeTransaction)) {
             if (!m_replyForUnhidingContent)
                 webPageProxy->setRemoteLayerTreeRootNode(m_remoteLayerTreeHost->rootNode());
             else
@@ -334,7 +334,7 @@
     if (m_debugIndicatorLayerTreeHost) {
         float scale = indicatorScale(layerTreeTransaction.contentsSize());
         webPageProxy->scrollingCoordinatorProxy()->willCommitLayerAndScrollingTrees();
-        bool rootLayerChanged = m_debugIndicatorLayerTreeHost->updateLayerTree(layerTreeTransaction, scale);
+        bool rootLayerChanged = m_debugIndicatorLayerTreeHost->updateLayerTree(connection, layerTreeTransaction, scale);
         webPageProxy->scrollingCoordinatorProxy()->didCommitLayerAndScrollingTrees();
         IntPoint scrollPosition;
 #if PLATFORM(MAC)

diff --git a/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.h b/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.h
@@ -36,6 +36,10 @@
 OBJC_CLASS CAAnimation;
 OBJC_CLASS WKAnimationDelegate;
 
+namespace IPC {
+class Connection;
+}
+
 namespace WebKit {
 
 class RemoteLayerTreeDrawingAreaProxy;
@@ -56,7 +60,7 @@ class RemoteLayerTreeHost {
     RemoteLayerTreeDrawingAreaProxy& drawingArea() const;
 
     // Returns true if the root layer changed.
-    bool updateLayerTree(const RemoteLayerTreeTransaction&, float indicatorScaleFactor  = 1);
+    bool updateLayerTree(const IPC::Connection&, const RemoteLayerTreeTransaction&, float indicatorScaleFactor  = 1);
     void asyncSetLayerContents(WebCore::PlatformLayerIdentifier, ImageBufferBackendHandle&&, const WebCore::RenderingResourceIdentifier&);
 
     void setIsDebugLayerTreeHost(bool flag) { m_isDebugLayerTreeHost = flag; }
@@ -95,7 +99,7 @@ class RemoteLayerTreeHost {
     MonotonicTime animationCurrentTime() const;
 #endif
 
-    void remotePageProcessCrashed(WebCore::ProcessIdentifier);
+    void remotePageProcessDidTerminate(WebCore::ProcessIdentifier);
 
 private:
     void createLayer(const RemoteLayerTreeTransaction::LayerCreationProperties&);

diff --git a/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm b/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm
@@ -26,6 +26,7 @@
 #import "config.h"
 #import "RemoteLayerTreeHost.h"
 
+#import "AuxiliaryProcessProxy.h"
 #import "LayerProperties.h"
 #import "Logging.h"
 #import "RemoteLayerTreeDrawingAreaProxy.h"
@@ -136,11 +137,18 @@
 }
 #endif
 
-bool RemoteLayerTreeHost::updateLayerTree(const RemoteLayerTreeTransaction& transaction, float indicatorScaleFactor)
+bool RemoteLayerTreeHost::updateLayerTree(const IPC::Connection& connection, const RemoteLayerTreeTransaction& transaction, float indicatorScaleFactor)
 {
     if (!m_drawingArea)
         return false;
 
+    auto* sender = AuxiliaryProcessProxy::fromConnection(connection);
+    if (!sender) {
+        ASSERT_NOT_REACHED();
+        return false;
+    }
+    auto processIdentifier = sender->coreProcessIdentifier();
+
     for (const auto& createdLayer : transaction.createdLayers())
         createLayer(createdLayer);
 
@@ -179,7 +187,7 @@
 
     if (auto contextHostedID = transaction.remoteContextHostedIdentifier()) {
         m_hostedLayers.set(*contextHostedID, rootNode->layerID());
-        m_hostedLayersInProcess.ensure(transaction.processIdentifier(), [] {
+        m_hostedLayersInProcess.ensure(processIdentifier, [] {
             return HashSet<WebCore::PlatformLayerIdentifier>();
         }).iterator->value.add(rootNode->layerID());
         rootNode->setRemoteContextHostedIdentifier(*contextHostedID);
@@ -216,7 +224,7 @@
         layerForID(layerAndClone.layerID).contents = layerForID(layerAndClone.cloneLayerID).contents;
 
     for (auto& destroyedLayer : transaction.destroyedLayers())
-        layerWillBeRemoved(transaction.processIdentifier(), destroyedLayer);
+        layerWillBeRemoved(processIdentifier, destroyedLayer);
 
     // Drop the contents of any layers which were unparented; the Web process will re-send
     // the backing store in the commit that reparents them.
@@ -495,7 +503,7 @@ static void recursivelyMapIOSurfaceBackingStore(CALayer *layer)
 }
 #endif
 
-void RemoteLayerTreeHost::remotePageProcessCrashed(WebCore::ProcessIdentifier processIdentifier)
+void RemoteLayerTreeHost::remotePageProcessDidTerminate(WebCore::ProcessIdentifier processIdentifier)
 {
     for (auto layerID : m_hostedLayersInProcess.take(processIdentifier)) {
         if (auto* node = nodeForID(layerID))

diff --git a/Source/WebKit/UIProcess/RemotePageProxy.cpp b/Source/WebKit/UIProcess/RemotePageProxy.cpp
@@ -95,7 +95,7 @@ void RemotePageProxy::processDidTerminate(WebCore::ProcessIdentifier processIden
     if (!m_page)
         return;
     if (auto* drawingArea = m_page->drawingArea())
-        drawingArea->remotePageProcessCrashed(processIdentifier);
+        drawingArea->remotePageProcessDidTerminate(processIdentifier);
     if (RefPtr mainFrame = m_page->mainFrame())
         mainFrame->remoteProcessDidTerminate(process());
 }

diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
@@ -369,7 +369,6 @@
         rootLayer.layer->flushCompositingStateForThisLayerOnly();
 
         RemoteLayerTreeTransaction layerTransaction;
-        layerTransaction.setProcessIdentifier(WebCore::Process::identifier());
         layerTransaction.setTransactionID(transactionID);
         layerTransaction.setCallbackIDs(WTFMove(m_pendingCallbackIDs));