[Site Isolation] Web Inspector: Only LocalFrame should require frame target and inspector controller

https://webkit.org/b/299493
rdar://161281114

Reviewed by BJ Burg.

Given only LocalFrame should end up handling inspector commands and
events, it should be worthwhile to tweak the design so that a frame
target corresponds to a local frame rather than any frame. This should
help simplify our architecture as we start implementing domains for the
frame target (so that e.g. we don't need to check whether the inspected
frame is local or remote all the time).

Here's how frame targets are handled now in each process/component:
- In UI process, it was already the case that the number of frame target
  proxies (WebFrameInspectorTargetProxy) equals to the number of local
  frames there are, due to each proxy associating with a WebFrameProxy.
- In WebCore, only LocalFrame now creates an inspector controller (FrameInspectorController)
  to provide the inspector agents.
- In WebKit, WebFrame now only creates an inspector target (WebFrameInspectorTarget)
  if it's local. This is now also done lazily as inspection is needed.

With this change, future inspector-related objects working with frames
can start targeting local frames instead. There's currently few use
cases needing to adapt to this change, in WindowProxy, DOMWindow, and
FrameConsoleClient, which are updated by this patch as well.

No new tests as there's no intended noticeable changes in behavior.
Existing inspector tests should cover use cases related to this change.

* Source/WebCore/inspector/FrameInspectorController.cpp:
(WebCore::FrameInspectorController::FrameInspectorController):
* Source/WebCore/inspector/FrameInspectorController.h:
* Source/WebCore/page/Frame.cpp:
(WebCore::Frame::Frame):
(WebCore::Frame::protectedInspectorController): Deleted.
* Source/WebCore/page/Frame.h:
(WebCore::Frame::inspectorController): Deleted.
(WebCore::Frame::console): Deleted.
(WebCore::Frame::console const): Deleted.
* Source/WebCore/page/LocalFrame.cpp:
(WebCore::LocalFrame::LocalFrame):
(WebCore::LocalFrame::protectedInspectorController):
* Source/WebCore/page/LocalFrame.h:
   In WebCore, only LocalFrame now has an inspector controller.

* Source/WebCore/page/FrameConsoleClient.cpp:
(WebCore::FrameConsoleClient::FrameConsoleClient):
(WebCore::FrameConsoleClient::addMessage):
(WebCore::FrameConsoleClient::messageWithTypeAndLevel):
(WebCore::FrameConsoleClient::count):
(WebCore::FrameConsoleClient::countReset):
(WebCore::FrameConsoleClient::profile):
(WebCore::FrameConsoleClient::profileEnd):
(WebCore::FrameConsoleClient::takeHeapSnapshot):
(WebCore::FrameConsoleClient::time):
(WebCore::FrameConsoleClient::timeLog):
(WebCore::FrameConsoleClient::timeEnd):
(WebCore::FrameConsoleClient::timeStamp):
(WebCore::FrameConsoleClient::screenshot):
* Source/WebCore/page/FrameConsoleClient.h:
   Given FrameConsoleClient only operated on local frames, make it also
   now only owned by LocalFrame instead.

* Source/WebCore/bindings/js/WindowProxy.cpp:
(WebCore::WindowProxy::setDOMWindow):
* Source/WebCore/page/DOMWindow.cpp:
(WebCore::DOMWindow::console const):
   Adapt to now only LocalFrame has a console client.

* Source/WebKit/WebProcess/Inspector/WebFrameInspectorTarget.cpp:
(WebKit::WebFrameInspectorTarget::connect):
(WebKit::WebFrameInspectorTarget::disconnect):
(WebKit::WebFrameInspectorTarget::sendMessageToTargetBackend):
* Source/WebKit/WebProcess/WebPage/WebFrame.cpp:
(WebKit::WebFrame::WebFrame):
(WebKit::WebFrame::ensureInspectorTarget):
(WebKit::WebFrame::connectInspector):
(WebKit::WebFrame::disconnectInspector):
(WebKit::WebFrame::sendMessageToInspectorTarget):
* Source/WebKit/WebProcess/WebPage/WebFrame.h:
   Make only local WebFrames create an inspector target, which is now
   lazily initialized.

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

Author: the-chenergy

Source/WebCore/bindings/js/WindowProxy.cpp

@@ -189,17 +189,18 @@ void WindowProxy::setDOMWindow(DOMWindow* newDOMWindow)
 
         windowProxy->setWindow(*newDOMWindow);
 
-        ScriptController* scriptController = nullptr;
-        RefPtr page = m_frame->page();
-        if (auto* localFrame = dynamicDowncast<LocalFrame>(*m_frame))
-            scriptController = &localFrame->script();
+        if (RefPtr localFrame = dynamicDowncast<LocalFrame>(m_frame.get())) {
+            CheckedRef scriptController = localFrame->script();
+
+            // ScriptController's m_cacheableBindingRootObject persists between page navigations
+            // so needs to know about the new JSDOMWindow.
+            if (RefPtr cacheableBindingRootObject = scriptController->existingCacheableBindingRootObject())
+                cacheableBindingRootObject->updateGlobalObject(windowProxy->window());
 
-        // ScriptController's m_cacheableBindingRootObject persists between page navigations
-        // so needs to know about the new JSDOMWindow.
-        if (RefPtr cacheableBindingRootObject = scriptController ? scriptController->existingCacheableBindingRootObject() : nullptr)
-            cacheableBindingRootObject->updateGlobalObject(windowProxy->window());
+            windowProxy->window()->setConsoleClient(localFrame->console());
+        }
 
-        windowProxy->window()->setConsoleClient(m_frame->console());
+        RefPtr page = m_frame->page();
         windowProxy->attachDebugger(page ? page->debugger() : nullptr);
         if (page)
             windowProxy->window()->setProfileGroup(page->group().identifier());

Source/WebCore/inspector/FrameInspectorController.cpp

@@ -55,7 +55,7 @@ using namespace Inspector;
 
 WTF_MAKE_TZONE_ALLOCATED_IMPL(FrameInspectorController);
 
-FrameInspectorController::FrameInspectorController(Frame& frame)
+FrameInspectorController::FrameInspectorController(LocalFrame& frame)
     : m_frame(frame)
     , m_instrumentingAgents(InstrumentingAgents::create(*this))
     , m_injectedScriptManager(makeUniqueRef<WebInjectedScriptManager>(*this, WebInjectedScriptHost::create()))

Source/WebCore/inspector/FrameInspectorController.h

@@ -47,18 +47,18 @@ class FrontendRouter;
 
 namespace WebCore {
 
-class Frame;
 class InspectorBackendClient;
 class InspectorFrontendClient;
 class InspectorInstrumentation;
 class InstrumentingAgents;
+class LocalFrame;
 class WebInjectedScriptManager;
 
 class FrameInspectorController final : public Inspector::InspectorEnvironment, public CanMakeWeakPtr<FrameInspectorController> {
     WTF_MAKE_NONCOPYABLE(FrameInspectorController);
     WTF_MAKE_TZONE_ALLOCATED(FrameInspectorController);
 public:
-    FrameInspectorController(Frame&);
+    FrameInspectorController(LocalFrame&);
     ~FrameInspectorController() override;
 
     WEBCORE_EXPORT void ref() const;
@@ -83,7 +83,7 @@ class FrameInspectorController final : public Inspector::InspectorEnvironment, p
 
     void createLazyAgents();
 
-    WeakRef<Frame> m_frame;
+    WeakRef<LocalFrame> m_frame;
     const Ref<InstrumentingAgents> m_instrumentingAgents;
     const UniqueRef<WebInjectedScriptManager> m_injectedScriptManager;
     const Ref<Inspector::FrontendRouter> m_frontendRouter;

Source/WebCore/page/DOMWindow.cpp

@@ -138,7 +138,7 @@ void DOMWindow::close()
 
 FrameConsoleClient* DOMWindow::console() const
 {
-    RefPtr frame = this->frame();
+    RefPtr frame = dynamicDowncast<LocalFrame>(this->frame());
     return frame ? &frame->console() : nullptr;
 }
 

Source/WebCore/page/Frame.cpp

@@ -27,9 +27,7 @@
 #include "Frame.h"
 
 #include "ContainerNodeInlines.h"
-#include "FrameConsoleClient.h"
 #include "FrameInlines.h"
-#include "FrameInspectorController.h"
 #include "FrameLoader.h"
 #include "FrameLoaderClient.h"
 #include "HTMLFrameOwnerElement.h"
@@ -121,8 +119,6 @@ Frame::Frame(Page& page, FrameIdentifier frameID, FrameType frameType, HTMLFrame
     , m_navigationScheduler(makeUniqueRefWithoutRefCountedCheck<NavigationScheduler>(*this))
     , m_opener(opener)
     , m_frameTreeSyncData(WTFMove(frameTreeSyncData))
-    , m_inspectorController(makeUniqueRefWithoutRefCountedCheck<FrameInspectorController>(*this))
-    , m_consoleClient(makeUniqueRef<FrameConsoleClient>(*this))
 {
     relaxAdoptionRequirement();
     if (parent && addToFrameTree == AddToFrameTree::Yes)
@@ -340,11 +336,6 @@ bool Frame::frameCanCreatePaymentSession() const
     return m_frameTreeSyncData->frameCanCreatePaymentSession;
 }
 
-Ref<FrameInspectorController> Frame::protectedInspectorController()
-{
-    return m_inspectorController.get();
-}
-
 bool Frame::isPrinting() const
 {
     return m_isPrinting;

Source/WebCore/page/Frame.h

@@ -41,8 +41,6 @@ namespace WebCore {
 class DOMWindow;
 class Event;
 class FloatSize;
-class FrameConsoleClient;
-class FrameInspectorController;
 class FrameLoaderClient;
 class FrameLoadRequest;
 class FrameView;
@@ -149,11 +147,6 @@ class Frame : public RefCountedAndCanMakeWeakPtr<Frame> {
     FrameTreeSyncData& frameTreeSyncData() const { return m_frameTreeSyncData.get(); }
     WEBCORE_EXPORT virtual RefPtr<SecurityOrigin> frameDocumentSecurityOrigin() const = 0;
 
-    FrameInspectorController& inspectorController() { return m_inspectorController.get(); }
-    WEBCORE_EXPORT Ref<FrameInspectorController> protectedInspectorController();
-    FrameConsoleClient& console() { return m_consoleClient.get(); }
-    const FrameConsoleClient& console() const { return m_consoleClient.get(); }
-
     WEBCORE_EXPORT virtual void setPrinting(bool printing, FloatSize pageSize, FloatSize originalPageSize, float maximumShrinkRatio, AdjustViewSize, NotifyUIProcess = NotifyUIProcess::Yes);
     WEBCORE_EXPORT bool isPrinting() const;
 
@@ -182,9 +175,6 @@ class Frame : public RefCountedAndCanMakeWeakPtr<Frame> {
     bool m_isPrinting { false };
 
     Ref<FrameTreeSyncData> m_frameTreeSyncData;
-
-    const UniqueRef<FrameInspectorController> m_inspectorController;
-    const UniqueRef<FrameConsoleClient> m_consoleClient;
 };
 
 } // namespace WebCore

Source/WebCore/page/FrameConsoleClient.cpp

@@ -36,6 +36,7 @@
 #include "Document.h"
 #include "ElementChildIteratorInlines.h"
 #include "Frame.h"
+#include "FrameInlines.h"
 #include "FrameSnapshotting.h"
 #include "HTMLCanvasElement.h"
 #include "HTMLImageElement.h"
@@ -96,7 +97,7 @@ using namespace Inspector;
 
 WTF_MAKE_TZONE_ALLOCATED_IMPL(FrameConsoleClient);
 
-FrameConsoleClient::FrameConsoleClient(Frame& frame)
+FrameConsoleClient::FrameConsoleClient(LocalFrame& frame)
     : m_frame(frame)
 {
 }
@@ -139,10 +140,7 @@ void FrameConsoleClient::logMessageToSystemConsole(const Inspector::ConsoleMessa
 
 void FrameConsoleClient::addMessage(std::unique_ptr<Inspector::ConsoleMessage>&& consoleMessage)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
+    Ref frame = m_frame.get();
     RefPtr page = frame->page();
     if (!page)
         return;
@@ -173,7 +171,7 @@ void FrameConsoleClient::addMessage(std::unique_ptr<Inspector::ConsoleMessage>&&
 #if ENABLE(WEBDRIVER_BIDI)
     AutomationInstrumentation::addMessageToConsole(consoleMessage);
 #endif
-    InspectorInstrumentation::addMessageToConsole(*frame, WTFMove(consoleMessage));
+    InspectorInstrumentation::addMessageToConsole(frame.get(), WTFMove(consoleMessage));
 }
 
 void FrameConsoleClient::addMessage(MessageSource source, MessageLevel level, const String& message, unsigned long requestIdentifier, Document* document)
@@ -209,10 +207,6 @@ void FrameConsoleClient::addMessage(MessageSource source, MessageLevel level, co
 
 void FrameConsoleClient::messageWithTypeAndLevel(MessageType type, MessageLevel level, JSC::JSGlobalObject* lexicalGlobalObject, Ref<Inspector::ScriptArguments>&& arguments)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
     String messageText;
     std::span<const String> additionalArguments;
     Vector<String> messageArgumentsVector = arguments->getArgumentsAsStrings();
@@ -230,7 +224,8 @@ void FrameConsoleClient::messageWithTypeAndLevel(MessageType type, MessageLevel
 #if ENABLE(WEBDRIVER_BIDI)
     AutomationInstrumentation::addMessageToConsole(message);
 #endif
-    InspectorInstrumentation::addMessageToConsole(*frame, WTFMove(message));
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::addMessageToConsole(frame.get(), WTFMove(message));
 
     RefPtr page = frame->page();
     if (!page)
@@ -252,84 +247,57 @@ void FrameConsoleClient::messageWithTypeAndLevel(MessageType type, MessageLevel
 
 void FrameConsoleClient::count(JSC::JSGlobalObject* lexicalGlobalObject, const String& label)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
-    InspectorInstrumentation::consoleCount(*frame, lexicalGlobalObject, label);
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::consoleCount(frame.get(), lexicalGlobalObject, label);
 }
 
 void FrameConsoleClient::countReset(JSC::JSGlobalObject* lexicalGlobalObject, const String& label)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
-    InspectorInstrumentation::consoleCountReset(*frame, lexicalGlobalObject, label);
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::consoleCountReset(frame.get(), lexicalGlobalObject, label);
 }
 
 void FrameConsoleClient::profile(JSC::JSGlobalObject*, const String& title)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
-    InspectorInstrumentation::startProfiling(*frame, title);
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::startProfiling(frame.get(), title);
 }
 
 void FrameConsoleClient::profileEnd(JSC::JSGlobalObject*, const String& title)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
+    Ref frame = m_frame.get();
     // FIXME: <https://webkit.org/b/153499> Web Inspector: console.profile should use the new Sampling Profiler
-    InspectorInstrumentation::stopProfiling(*frame, title);
+    InspectorInstrumentation::stopProfiling(frame.get(), title);
 }
 
 void FrameConsoleClient::takeHeapSnapshot(JSC::JSGlobalObject*, const String& title)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
-    InspectorInstrumentation::takeHeapSnapshot(*frame, title);
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::takeHeapSnapshot(frame.get(), title);
 }
 
 void FrameConsoleClient::time(JSC::JSGlobalObject* lexicalGlobalObject, const String& label)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
-    InspectorInstrumentation::startConsoleTiming(*frame, lexicalGlobalObject, label);
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::startConsoleTiming(frame.get(), lexicalGlobalObject, label);
 }
 
 void FrameConsoleClient::timeLog(JSC::JSGlobalObject* lexicalGlobalObject, const String& label, Ref<ScriptArguments>&& arguments)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
-    InspectorInstrumentation::logConsoleTiming(*frame, lexicalGlobalObject, label, WTFMove(arguments));
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::logConsoleTiming(frame.get(), lexicalGlobalObject, label, WTFMove(arguments));
 }
 
 void FrameConsoleClient::timeEnd(JSC::JSGlobalObject* lexicalGlobalObject, const String& label)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
-    InspectorInstrumentation::stopConsoleTiming(*frame, lexicalGlobalObject, label);
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::stopConsoleTiming(frame.get(), lexicalGlobalObject, label);
 }
 
 void FrameConsoleClient::timeStamp(JSC::JSGlobalObject*, Ref<ScriptArguments>&& arguments)
 {
-    RefPtr frame = dynamicDowncast<LocalFrame>(m_frame.get());
-    if (!frame)
-        return;
-
-    InspectorInstrumentation::consoleTimeStamp(*frame, WTFMove(arguments));
+    Ref frame = m_frame.get();
+    InspectorInstrumentation::consoleTimeStamp(frame.get(), WTFMove(arguments));
 }
 
 static JSC::JSObject* objectArgumentAt(ScriptArguments& arguments, unsigned index)
@@ -434,10 +402,9 @@ void FrameConsoleClient::screenshot(JSC::JSGlobalObject* lexicalGlobalObject, Re
 
                 if (dataURL.isEmpty()) {
                     if (!snapshot) {
-                        if (RefPtr localFrame = dynamicDowncast<LocalFrame>(m_frame.get())) {
-                            if (RefPtr localMainFrame = localFrame->localMainFrame())
-                                snapshot = WebCore::snapshotNode(*localMainFrame, *node, { { }, PixelFormat::BGRA8, DestinationColorSpace::SRGB() });
-                        }
+                        Ref frame = m_frame.get();
+                        if (RefPtr localMainFrame = frame->localMainFrame())
+                            snapshot = WebCore::snapshotNode(*localMainFrame, *node, { { }, PixelFormat::BGRA8, DestinationColorSpace::SRGB() });
                     }
 
                     if (snapshot)
@@ -469,11 +436,10 @@ void FrameConsoleClient::screenshot(JSC::JSGlobalObject* lexicalGlobalObject, Re
         } else if (auto* rect = JSDOMRectReadOnly::toWrapped(vm, possibleTarget)) {
             target = possibleTarget;
             if (InspectorInstrumentation::hasFrontends()) [[unlikely]] {
-                if (RefPtr localFrame = dynamicDowncast<LocalFrame>(m_frame.get())) {
-                    if (RefPtr localMainFrame = localFrame->localMainFrame()) {
-                        if (auto snapshot = WebCore::snapshotFrameRect(*localMainFrame, enclosingIntRect(rect->toFloatRect()), { { }, PixelFormat::BGRA8, DestinationColorSpace::SRGB() }))
-                            dataURL = snapshot->toDataURL("image/png"_s, std::nullopt, PreserveResolution::Yes);
-                    }
+                Ref frame = m_frame.get();
+                if (RefPtr localMainFrame = frame->localMainFrame()) {
+                    if (auto snapshot = WebCore::snapshotFrameRect(*localMainFrame, enclosingIntRect(rect->toFloatRect()), { { }, PixelFormat::BGRA8, DestinationColorSpace::SRGB() }))
+                        dataURL = snapshot->toDataURL("image/png"_s, std::nullopt, PreserveResolution::Yes);
                 }
             }
         } else {
@@ -487,13 +453,12 @@ void FrameConsoleClient::screenshot(JSC::JSGlobalObject* lexicalGlobalObject, Re
 
     if (InspectorInstrumentation::hasFrontends()) [[unlikely]] {
         if (!target) {
-            if (RefPtr localFrame = dynamicDowncast<LocalFrame>(m_frame.get())) {
-                if (RefPtr localMainFrame = localFrame->localMainFrame()) {
-                    // If no target is provided, capture an image of the viewport.
-                    auto viewportRect = localMainFrame->view()->unobscuredContentRect();
-                    if (auto snapshot = WebCore::snapshotFrameRect(*localMainFrame, viewportRect, { { }, PixelFormat::BGRA8, DestinationColorSpace::SRGB() }))
-                        dataURL = snapshot->toDataURL("image/png"_s, std::nullopt, PreserveResolution::Yes);
-                }
+            Ref frame = m_frame.get();
+            if (RefPtr localMainFrame = frame->localMainFrame()) {
+                // If no target is provided, capture an image of the viewport.
+                auto viewportRect = localMainFrame->view()->unobscuredContentRect();
+                if (auto snapshot = WebCore::snapshotFrameRect(*localMainFrame, viewportRect, { { }, PixelFormat::BGRA8, DestinationColorSpace::SRGB() }))
+                    dataURL = snapshot->toDataURL("image/png"_s, std::nullopt, PreserveResolution::Yes);
             }
         }
 

Source/WebCore/page/FrameConsoleClient.h

@@ -47,14 +47,14 @@ using JSC::MessageType;
 namespace WebCore {
 
 class Document;
-class Frame;
+class LocalFrame;
 class StringCallback;
 
 class WEBCORE_EXPORT FrameConsoleClient final : public JSC::ConsoleClient, public CanMakeCheckedPtr<FrameConsoleClient> {
     WTF_MAKE_TZONE_ALLOCATED_EXPORT(FrameConsoleClient, WEBCORE_EXPORT);
     WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR(FrameConsoleClient);
 public:
-    explicit FrameConsoleClient(Frame&);
+    explicit FrameConsoleClient(LocalFrame&);
     virtual ~FrameConsoleClient();
 
     FrameConsoleClient(const FrameConsoleClient&) = delete;
@@ -93,7 +93,7 @@ class WEBCORE_EXPORT FrameConsoleClient final : public JSC::ConsoleClient, publi
     void recordEnd(JSC::JSGlobalObject*, Ref<Inspector::ScriptArguments>&&) override;
     void screenshot(JSC::JSGlobalObject*, Ref<Inspector::ScriptArguments>&&) override;
 
-    WeakRef<Frame> m_frame;
+    WeakRef<LocalFrame> m_frame;
 };
 
 } // namespace WebCore