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>
Mon, 7 May 2018 17:10:34 +0000 (17:10 +0000)
committerbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 May 2018 17:10:34 +0000 (17:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184861
<rdar://problem/39153768>

Reviewed by Timothy Hatcher.

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@231439 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 0e101ba..24ed54c 100644 (file)
@@ -1,3 +1,23 @@
+2018-05-07  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 Timothy Hatcher.
+
+        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-07  Eric Carlson  <eric.carlson@apple.com>
 
         Text track cue logging should include cue text
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 6bc61b6..5127ae1 100644 (file)
@@ -1,3 +1,30 @@
+2018-05-07  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 Timothy Hatcher.
+
+        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-04  Tim Horton  <timothy_horton@apple.com>
 
         Shift to a lower-level framework for simplifying URLs
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 a63c75b..5abcf96 100644 (file)
@@ -2074,6 +2074,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 5a2e276..11f4265 100644 (file)
@@ -1,3 +1,17 @@
+2018-05-07  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 Timothy Hatcher.
+
+        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-04  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS] Multiple links in Mail are dropped in a single line, and are difficult to tell apart
index c2d9d52..5db6b85 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()];
+    webViewConfiguration.get().preferences._developerExtrasEnabled = YES;
+    
+    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.
+    WKInspectorShow(WKPageGetInspector([webView _pageRefForTransitionToWKWebView]));
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson2://host/main2.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pid2 = [webView _webProcessIdentifier];
+
+    WKInspectorClose(WKPageGetInspector([webView _pageRefForTransitionToWKWebView]));
+
+    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(