Changeset 237267 in webkit

Timestamp:

Oct 18, 2018 2:41:41 PM (6 years ago)

Author:

Chris Dumez

Message:

[PSON] SuspendedPages do not report meaningful domains in Activity Monitor
​https://bugs.webkit.org/show_bug.cgi?id=190721
<rdar://problem/45059699>

Reviewed by Alex Christensen.

Source/WebKit:

SuspendedPages do not report meaningful domains in Activity Monitor, which makes
debugging harder.

• WebProcess/cocoa/WebProcessCocoa.mm:

(WebKit::origin):
(WebKit::WebProcess::updateActivePages):

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessPool.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessProxy.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebProcessProxy.h (modified) (1 diff)
• Source/WebKit/WebProcess/WebProcess.cpp (modified) (1 diff)
• Source/WebKit/WebProcess/WebProcess.h (modified) (1 diff)
• Source/WebKit/WebProcess/WebProcess.messages.in (modified) (1 diff)
• Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm (modified) (3 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-10-18  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] SuspendedPages do not report meaningful domains in Activity Monitor
+        https://bugs.webkit.org/show_bug.cgi?id=190721
+        <rdar://problem/45059699>
+
+        Reviewed by Alex Christensen.
+
+        SuspendedPages do not report meaningful domains in Activity Monitor, which makes
+        debugging harder.
+
+        * WebProcess/cocoa/WebProcessCocoa.mm:
+        (WebKit::origin):
+        (WebKit::WebProcess::updateActivePages):
+
 2018-10-18  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm

@@ -575 +575 @@
 }
 
+- (NSArray<NSString *> *)_getActivePagesOriginsInWebProcessForTesting:(pid_t)pid
+{
+    auto activePagesOrigins = _processPool->activePagesOriginsInWebProcessForTesting(pid);
+
+    NSMutableArray<NSString *> *array = [[NSMutableArray alloc] initWithCapacity:activePagesOrigins.size()];
+    for (auto& origin : activePagesOrigins)
+        [array addObject:(NSString *)origin];
+    return [array autorelease];
+}
+
 @end
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h

@@ -114 +114 @@
 - (void)_registerURLSchemeServiceWorkersCanHandle:(NSString *)scheme WK_API_AVAILABLE(macosx(10.13.4), ios(11.3));
 - (void)_setMaximumNumberOfProcesses:(NSUInteger)value WK_API_AVAILABLE(macosx(10.13.4), ios(11.3));
+- (NSArray<NSString *> *)_getActivePagesOriginsInWebProcessForTesting:(pid_t)pid WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
 
 @property (nonatomic, getter=_isCookieStoragePartitioningEnabled, setter=_setCookieStoragePartitioningEnabled:) BOOL _cookieStoragePartitioningEnabled WK_API_DEPRECATED("Partitioned cookies are no longer supported", macosx(10.12.3, WK_MAC_TBA), ios(10.3, WK_IOS_TBA));

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -1326 +1326 @@
 }
 
+Vector<String> WebProcessPool::activePagesOriginsInWebProcessForTesting(ProcessID pid)
+{
+    for (auto& process : m_processes) {
+        if (process->processIdentifier() == pid)
+            return process->activePagesDomainsForTesting();
+    }
+    return { };
+}
+
 void WebProcessPool::setAlwaysUsesComplexTextCodePath(bool alwaysUseComplexText)
 {

trunk/Source/WebKit/UIProcess/WebProcessPool.h

@@ -216 +216 @@
 
     ProcessID networkProcessIdentifier();
+    Vector<String> activePagesOriginsInWebProcessForTesting(ProcessID);
 
     WebPageGroup& defaultPageGroup() { return m_defaultPageGroup.get(); }

trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -1405 +1405 @@
 }
 
+Vector<String> WebProcessProxy::activePagesDomainsForTesting()
+{
+    Vector<String> activeDomains;
+    sendSync(Messages::WebProcess::GetActivePagesOriginsForTesting(), Messages::WebProcess::GetActivePagesOriginsForTesting::Reply(activeDomains), 0);
+    return activeDomains;
+}
+
 #if PLATFORM(WATCHOS)
 

trunk/Source/WebKit/UIProcess/WebProcessProxy.h

@@ -122 +122 @@
     unsigned visiblePageCount() const { return m_visiblePageCounter.value(); }
 
+    Vector<String> activePagesDomainsForTesting(); // This is what is reported to ActivityMonitor.
+
     virtual bool isServiceWorkerProcess() const { return false; }
 

trunk/Source/WebKit/WebProcess/WebProcess.cpp

@@ -1329 +1329 @@
 }
 
+void WebProcess::getActivePagesOriginsForTesting(Vector<String>&)
+{
+}
+
 void WebProcess::updateCPULimit()
 {

trunk/Source/WebKit/WebProcess/WebProcess.h

@@ -193 +193 @@
 
     void updateActivePages();
+    void getActivePagesOriginsForTesting(Vector<String>&);
     void pageActivityStateDidChange(uint64_t pageID, OptionSet<WebCore::ActivityState::Flag> changed);
 

trunk/Source/WebKit/WebProcess/WebProcess.messages.in

@@ -130 +130 @@
 
     UpdateActivePages()
+    GetActivePagesOriginsForTesting() -> (Vector<String> activeOrigins)
 
 #if PLATFORM(MAC)

trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm

@@ -55 +55 @@
 #import <WebCore/FontCache.h>
 #import <WebCore/FontCascade.h>
+#import <WebCore/HistoryController.h>
+#import <WebCore/HistoryItem.h>
 #import <WebCore/LocalizedStrings.h>
 #import <WebCore/LogInitialization.h>
@@ -410 +412 @@
         return nil;
 
-    URL mainFrameURL(URL(), mainFrame->url());
+    URL mainFrameURL = { URL(), mainFrame->url() };
+    if (page.isSuspended()) {
+        // Suspended page are navigated to about:blank upon suspension so we really want to report the previous URL.
+        if (auto* coreFrame = mainFrame->coreFrame()) {
+            if (auto* backHistoryItem = coreFrame->loader().history().previousItem())
+                mainFrameURL = { URL(), backHistoryItem->urlString() };
+        }
+    }
+
     Ref<SecurityOrigin> mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
     String mainFrameOriginString;
@@ -427 +437 @@
 #endif
 
+#if PLATFORM(MAC)
+static RetainPtr<NSArray<NSString *>> activePagesOrigins(const HashMap<uint64_t, RefPtr<WebPage>>& pageMap)
+{
+    RetainPtr<NSMutableArray<NSString *>> activeOrigins = adoptNS([[NSMutableArray alloc] init]);
+
+    for (auto& page : pageMap.values()) {
+        if (page->usesEphemeralSession())
+            continue;
+
+        if (NSURL *originAsURL = origin(*page))
+            [activeOrigins addObject:userVisibleString(originAsURL)];
+    }
+
+    return activeOrigins;
+}
+#endif
+
 void WebProcess::updateActivePages()
 {
 #if PLATFORM(MAC)
-    auto activePageURLs = adoptNS([[NSMutableArray alloc] init]);
-
-    for (auto& page : m_pageMap.values()) {
-        if (page->usesEphemeralSession() || page->isSuspended())
-            continue;
-
-        if (NSURL *originAsURL = origin(*page))
-            [activePageURLs addObject:userVisibleString(originAsURL)];
-    }
-
-    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), [activePageURLs] {
-        _LSSetApplicationInformationItem(kLSDefaultSessionID, _LSGetCurrentApplicationASN(), CFSTR("LSActivePageUserVisibleOriginsKey"), (__bridge CFArrayRef)activePageURLs.get(), nullptr);
+    auto activeOrigins = activePagesOrigins(m_pageMap);
+
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), [activeOrigins = WTFMove(activeOrigins)] {
+        _LSSetApplicationInformationItem(kLSDefaultSessionID, _LSGetCurrentApplicationASN(), CFSTR("LSActivePageUserVisibleOriginsKey"), (__bridge CFArrayRef)activeOrigins.get(), nullptr);
     });
+#endif
+}
+
+void WebProcess::getActivePagesOriginsForTesting(Vector<String>& activeOrigins)
+{
+#if PLATFORM(MAC)
+    auto activeOriginsAsNSStrings = activePagesOrigins(m_pageMap);
+    activeOrigins.reserveCapacity([activeOriginsAsNSStrings count]);
+    for (NSString* activeOrigin in activeOriginsAsNSStrings.get())
+        activeOrigins.uncheckedAppend(activeOrigin);
+#else
+    UNUSED_PARAM(activeOrigins);
 #endif
 }

trunk/Tools/ChangeLog

@@ +1 @@
+2018-10-18  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] SuspendedPages do not report meaningful domains in Activity Monitor
+        https://bugs.webkit.org/show_bug.cgi?id=190721
+        <rdar://problem/45059699>
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2018-10-18  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

@@ -517 +517 @@
 }
 
+#if PLATFORM(MAC)
+TEST(ProcessSwap, SuspendedPagesInActivityMonitor)
+{
+    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()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [handler addMappingFromURLString:@"pson://www.webkit.org/main.html" toData:testBytes];
+    [handler addMappingFromURLString:@"pson://www.google.com/main.html" toData:testBytes];
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+    RetainPtr<PSONMessageHandler> messageHandler = adoptNS([[PSONMessageHandler alloc] init]);
+    [[webViewConfiguration userContentController] addScriptMessageHandler:messageHandler.get() name:@"pson"];
+
+    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:@"pson://www.webkit.org/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto webkitPID = [webView _webProcessIdentifier];
+    auto* activeDomains = [processPool _getActivePagesOriginsInWebProcessForTesting:webkitPID];
+    EXPECT_EQ(1u, activeDomains.count);
+    EXPECT_WK_STREQ(@"pson://www.webkit.org", activeDomains[0]);
+
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main.html"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto googlePID = [webView _webProcessIdentifier];
+    EXPECT_NE(webkitPID, googlePID);
+
+    activeDomains = [processPool _getActivePagesOriginsInWebProcessForTesting:googlePID];
+    EXPECT_EQ(1u, activeDomains.count);
+    EXPECT_WK_STREQ(@"pson://www.google.com", activeDomains[0]);
+
+    activeDomains = [processPool _getActivePagesOriginsInWebProcessForTesting:webkitPID];
+    EXPECT_EQ(1u, activeDomains.count);
+    EXPECT_WK_STREQ(@"pson://www.webkit.org", activeDomains[0]);
+
+    [webView goBack]; // Back to webkit.org.
+
+    TestWebKitAPI::Util::run(&receivedMessage);
+    receivedMessage = false;
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    auto pidAfterBackNavigation = [webView _webProcessIdentifier];
+    EXPECT_EQ(webkitPID, pidAfterBackNavigation);
+
+    activeDomains = [processPool _getActivePagesOriginsInWebProcessForTesting:googlePID];
+    EXPECT_EQ(1u, activeDomains.count);
+    EXPECT_WK_STREQ(@"pson://www.google.com", activeDomains[0]);
+
+    activeDomains = [processPool _getActivePagesOriginsInWebProcessForTesting:webkitPID];
+    EXPECT_EQ(1u, activeDomains.count);
+    EXPECT_WK_STREQ(@"pson://www.webkit.org", activeDomains[0]);
+}
+#endif // PLATFORM(MAC)
+
 TEST(ProcessSwap, BackWithoutSuspendedPage)
 {