Web Inspector: opt out of process swap on navigation if a Web Inspector frontend...
authorbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 May 2018 19:17:49 +0000 (19:17 +0000)
committerbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 May 2018 19:17:49 +0000 (19:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184861
<rdar://problem/39153768>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Notify the client of the current connection count whenever a frontend connects or disconnects.

Covered by new API test.

* inspector/InspectorClient.h:
(WebCore::InspectorClient::frontendCountChanged):
* inspector/InspectorController.cpp:
(WebCore::InspectorController::connectFrontend):
(WebCore::InspectorController::disconnectFrontend):
(WebCore::InspectorController::disconnectAllFrontends):
* inspector/InspectorController.h:

Source/WebKit:

We need to track how many frontends are attached to the web page (both local and remote).
InspectorController propagates this out to WebKit via InspectorClient. This is then
kept in UIProcess as a member of WebPageProxy. When making a decision whether to use a
new process for a navigation, return early with "no" if any frontends are open for the
page being navigated.

* UIProcess/WebPageProxy.h:
(WebKit::WebPageProxy::didChangeInspectorFrontendCount):
(WebKit::WebPageProxy::inspectorFrontendCount const):
* UIProcess/WebPageProxy.messages.in:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigation):
* WebProcess/WebCoreSupport/WebInspectorClient.cpp:
(WebKit::WebInspectorClient::frontendCountChanged):
* WebProcess/WebCoreSupport/WebInspectorClient.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::inspectorFrontendCountChanged):
* WebProcess/WebPage/WebPage.h:

Tools:

Add a new test that checks whether a new process is used for navigation when
an Inspector is shown. Also check that the behavior reverts to normal after
the Inspector has been closed.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

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

14 files changed:
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorClient.h
Source/WebCore/inspector/InspectorController.cpp
Source/WebCore/inspector/InspectorController.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/WebPageProxy.messages.in
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebInspectorClient.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebInspectorClient.h
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index 2472a2f..e3b8f89 100644 (file)
@@ -1,3 +1,23 @@
+2018-05-02  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: opt out of process swap on navigation if a Web Inspector frontend is connected
+        https://bugs.webkit.org/show_bug.cgi?id=184861
+        <rdar://problem/39153768>
+
+        Reviewed by Ryosuke Niwa.
+
+        Notify the client of the current connection count whenever a frontend connects or disconnects.
+
+        Covered by new API test.
+
+        * inspector/InspectorClient.h:
+        (WebCore::InspectorClient::frontendCountChanged):
+        * inspector/InspectorController.cpp:
+        (WebCore::InspectorController::connectFrontend):
+        (WebCore::InspectorController::disconnectFrontend):
+        (WebCore::InspectorController::disconnectAllFrontends):
+        * inspector/InspectorController.h:
+
 2018-05-02  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 
         [GStreamer] Remove unneeded include of gstgldisplay_wayland.h after r228866 and r229022
index b5ea4b2..736edca 100644 (file)
@@ -44,6 +44,7 @@ public:
     virtual ~InspectorClient() = default;
 
     virtual void inspectedPageDestroyed() = 0;
+    virtual void frontendCountChanged(unsigned) { }
 
     virtual Inspector::FrontendChannel* openLocalFrontend(InspectorController*) = 0;
     virtual void bringFrontendToFront() = 0;
index e9ea1c5..20eb10f 100644 (file)
@@ -274,6 +274,8 @@ void InspectorController::connectFrontend(Inspector::FrontendChannel* frontendCh
         m_agents.didCreateFrontendAndBackend(&m_frontendRouter.get(), &m_backendDispatcher.get());
     }
 
+    m_inspectorClient->frontendCountChanged(m_frontendRouter->frontendCount());
+
 #if ENABLE(REMOTE_INSPECTOR)
     if (!m_frontendRouter->hasRemoteFrontend())
         m_page.remoteInspectorInformationDidChange();
@@ -302,6 +304,8 @@ void InspectorController::disconnectFrontend(FrontendChannel* frontendChannel)
         InspectorInstrumentation::unregisterInstrumentingAgents(m_instrumentingAgents.get());
     }
 
+    m_inspectorClient->frontendCountChanged(m_frontendRouter->frontendCount());
+
 #if ENABLE(REMOTE_INSPECTOR)
     if (!m_frontendRouter->hasFrontends())
         m_page.remoteInspectorInformationDidChange();
@@ -338,6 +342,8 @@ void InspectorController::disconnectAllFrontends()
     m_isAutomaticInspection = false;
     m_pauseAfterInitialization = false;
 
+    m_inspectorClient->frontendCountChanged(m_frontendRouter->frontendCount());
+
 #if ENABLE(REMOTE_INSPECTOR)
     m_page.remoteInspectorInformationDidChange();
 #endif
index 9a129e0..b45d6a3 100644 (file)
@@ -94,7 +94,6 @@ public:
     WEBCORE_EXPORT void connectFrontend(Inspector::FrontendChannel*, bool isAutomaticInspection = false, bool immediatelyPause = false);
     WEBCORE_EXPORT void disconnectFrontend(Inspector::FrontendChannel*);
     WEBCORE_EXPORT void disconnectAllFrontends();
-    void setProcessId(long);
 
     void inspect(Node*);
     WEBCORE_EXPORT void drawHighlight(GraphicsContext&) const;
index 5dffb97..5d7b912 100644 (file)
@@ -1,3 +1,30 @@
+2018-05-02  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: opt out of process swap on navigation if a Web Inspector frontend is connected
+        https://bugs.webkit.org/show_bug.cgi?id=184861
+        <rdar://problem/39153768>
+
+        Reviewed by Ryosuke Niwa.
+
+        We need to track how many frontends are attached to the web page (both local and remote).
+        InspectorController propagates this out to WebKit via InspectorClient. This is then
+        kept in UIProcess as a member of WebPageProxy. When making a decision whether to use a
+        new process for a navigation, return early with "no" if any frontends are open for the
+        page being navigated.
+
+        * UIProcess/WebPageProxy.h:
+        (WebKit::WebPageProxy::didChangeInspectorFrontendCount):
+        (WebKit::WebPageProxy::inspectorFrontendCount const):
+        * UIProcess/WebPageProxy.messages.in:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::processForNavigation):
+        * WebProcess/WebCoreSupport/WebInspectorClient.cpp:
+        (WebKit::WebInspectorClient::frontendCountChanged):
+        * WebProcess/WebCoreSupport/WebInspectorClient.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::inspectorFrontendCountChanged):
+        * WebProcess/WebPage/WebPage.h:
+
 2018-05-02  Jer Noble  <jer.noble@apple.com>
 
         Adopt -destinationWindowToExitFullScreenForWindow:
index 9d1355d..561d2b3 100644 (file)
@@ -362,6 +362,9 @@ public:
 
     WebInspectorProxy* inspector() const;
 
+    void didChangeInspectorFrontendCount(unsigned count) { m_inspectorFrontendCount = count; }
+    unsigned inspectorFrontendCount() const { return m_inspectorFrontendCount; }
+
     bool isControlledByAutomation() const { return m_controlledByAutomation; }
     void setControlledByAutomation(bool);
 
@@ -2009,6 +2012,7 @@ private:
     bool m_allowsRemoteInspection { true };
     String m_remoteInspectionNameOverride;
 #endif
+    unsigned m_inspectorFrontendCount { 0 };
 
 #if PLATFORM(COCOA)
     bool m_isSmartInsertDeleteEnabled { false };
index ac7aa76..999678d 100644 (file)
@@ -410,6 +410,8 @@ messages -> WebPageProxy {
     DisableInspectorNodeSearch()
 #endif
 
+    DidChangeInspectorFrontendCount(uint64_t count)
+
     # Search popup menus
     SaveRecentSearches(String name, Vector<WebCore::RecentSearch> searchItems)
     LoadRecentSearches(String name) -> (Vector<WebCore::RecentSearch> result)
index ca548be..6b12175 100644 (file)
@@ -2068,6 +2068,9 @@ Ref<WebProcessProxy> WebProcessPool::processForNavigationInternal(WebPageProxy&
     if (!m_configuration->processSwapsOnNavigation())
         return page.process();
 
+    if (page.inspectorFrontendCount() > 0)
+        return page.process();
+
     if (!page.process().hasCommittedAnyProvisionalLoads())
         return page.process();
 
index d5c90e5..0f1d53c 100644 (file)
@@ -84,6 +84,11 @@ void WebInspectorClient::inspectedPageDestroyed()
     delete this;
 }
 
+void WebInspectorClient::frontendCountChanged(unsigned count)
+{
+    m_page->inspectorFrontendCountChanged(count);
+}
+
 Inspector::FrontendChannel* WebInspectorClient::openLocalFrontend(InspectorController* controller)
 {
     m_page->inspector()->openFrontendConnection(controller->isUnderTest());
index 96b9a26..9c214ce 100644 (file)
@@ -50,6 +50,7 @@ public:
 private:
     // WebCore::InspectorClient
     void inspectedPageDestroyed() override;
+    void frontendCountChanged(unsigned) override;
 
     Inspector::FrontendChannel* openLocalFrontend(WebCore::InspectorController*) override;
     void bringFrontendToFront() override;
index a82871d..21e6e2f 100644 (file)
@@ -3265,6 +3265,11 @@ RemoteWebInspectorUI* WebPage::remoteInspectorUI()
     return m_remoteInspectorUI.get();
 }
 
+void WebPage::inspectorFrontendCountChanged(unsigned count)
+{
+    send(Messages::WebPageProxy::DidChangeInspectorFrontendCount(count));
+}
+
 #if (PLATFORM(IOS) && HAVE(AVKIT)) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE))
 PlaybackSessionManager& WebPage::playbackSessionManager()
 {
index 9780cfc..8e53da5 100644 (file)
@@ -290,6 +290,8 @@ public:
     RemoteWebInspectorUI* remoteInspectorUI();
     bool isInspectorPage() { return !!m_inspectorUI || !!m_remoteInspectorUI; }
 
+    void inspectorFrontendCountChanged(unsigned);
+
 #if PLATFORM(IOS) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE))
     PlaybackSessionManager& playbackSessionManager();
     VideoFullscreenManager& videoFullscreenManager();
index 4fe6730..35977e0 100644 (file)
@@ -1,3 +1,17 @@
+2018-05-02  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: opt out of process swap on navigation if a Web Inspector frontend is connected
+        https://bugs.webkit.org/show_bug.cgi?id=184861
+        <rdar://problem/39153768>
+
+        Reviewed by Ryosuke Niwa.
+
+        Add a new test that checks whether a new process is used for navigation when
+        an Inspector is shown. Also check that the behavior reverts to normal after
+        the Inspector has been closed.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2018-05-02  Valerie R Young  <valerie@bocoup.com>
 
         test262/Runner.pm: save summary to file
index c2d9d52..45a6698 100644 (file)
@@ -27,6 +27,7 @@
 
 #import "PlatformUtilities.h"
 #import "Test.h"
+#import <WebKit/WKInspector.h>
 #import <WebKit/WKNavigationDelegate.h>
 #import <WebKit/WKNavigationPrivate.h>
 #import <WebKit/WKPreferencesPrivate.h>
@@ -1066,6 +1067,59 @@ TEST(ProcessSwap, LoadUnload)
     EXPECT_TRUE([receivedMessages.get()[5] isEqualToString:@"pson1://host/main.html - unload" ]);
     EXPECT_TRUE([receivedMessages.get()[6] isEqualToString:@"pson2://host/main.html - load" ]);
 }
+
+TEST(ProcessSwap, DisableForInspector)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().processSwapsOnNavigation = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    RetainPtr<PSONScheme> handler = adoptNS([[PSONScheme alloc] init]);
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON1"];
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON2"];
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:delegate.get()];
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson1://host/main1.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid1 = [webView _webProcessIdentifier];
+
+    // FIXME: use ObjC equivalent for WKInspectorRef when available.
+    WKRetainPtr<WKPageRef> page = adoptWK([webView _pageRefForTransitionToWKWebView]);
+    WKRetainPtr<WKInspectorRef> inspector = adoptWK(WKPageGetInspector(page.get()));
+    WKInspectorShow(inspector.get());
+    
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson2://host/main2.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid2 = [webView _webProcessIdentifier];
+
+    WKInspectorClose(inspector.get());
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson1://host/main2.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid3 = [webView _webProcessIdentifier];
+
+    EXPECT_EQ(pid1, pid2);
+    EXPECT_FALSE(pid2 == pid3);
+    EXPECT_EQ(numberOfDecidePolicyCalls, 3);
+}
+
 #endif // !TARGET_OS_IPHONE
 
 static const char* sameOriginBlobNavigationTestBytes = R"PSONRESOURCE(