Changeset 286505 in webkit

Timestamp:

Dec 3, 2021, 11:39:02 AM (5 years ago)

Author:

Chris Dumez

Message:

Follow-up to r286479 to add API test and address issues found by the test
​https://bugs.webkit.org/show_bug.cgi?id=233798

Reviewed by Darin Adler.

Source/WebKit:

Add functionality needed for API testing and fix issues found by the API test.

• UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
• UIProcess/API/Cocoa/WKWebViewTesting.mm:

(-[WKWebView _isLayerTreeFrozenForTesting:]):

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::destroyProvisionalPage):
(WebKit::WebPageProxy::isLayerTreeFrozen):

• UIProcess/WebPageProxy.h:
• WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::isLayerTreeFrozen):

• WebProcess/WebPage/WebPage.h:
• WebProcess/WebPage/WebPage.messages.in:

Add new SPI to check if the layer tree is frozen in the WebProcess so that I could
write an API test for this.

• UIProcess/ProvisionalPageProxy.cpp:

(WebKit::ProvisionalPageProxy::ProvisionalPageProxy):
Make sure m_provisionalLoadURL gets initialized when the ProvisionalPageProxy gets
constructed *after* the provisional load has already started (which is the case
when the process swap is triggered by COOP).

(WebKit::ProvisionalPageProxy::didFailProvisionalLoadForFrame):
If the provisional load load fails in the provisional process, and the ProvisionalPageProxy
was constructed on resource response (COOP case), then no longer forward the
didFailProvisionalLoadForFrame() to the WebPageProxy. Instead, we destroy the
ProvisionalPageProxy. This is to avoid duplicate calls to didFailProvisionalLoadForFrame().
In this case, there is still a provisional load ongoing in the committed process and the
ProvisionalPageProxy destructor will take care of stopping that provisional load (due to
r286479), which will cause the committed process to send its own
didFailProvisionalLoadForFrame IPC.

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm (modified) (1 diff)
• Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/WebPageProxy.h (modified) (2 diffs)
• Source/WebKit/WebProcess/WebPage/WebPage.cpp (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/WebPage.h (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/WebPage.messages.in (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-12-03  Chris Dumez  <cdumez@apple.com>
+
+        Follow-up to r286479 to add API test and address issues found by the test
+        https://bugs.webkit.org/show_bug.cgi?id=233798
+
+        Reviewed by Darin Adler.
+
+        Add functionality needed for API testing and fix issues found by the API test.
+
+        * UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h:
+        * UIProcess/API/Cocoa/WKWebViewTesting.mm:
+        (-[WKWebView _isLayerTreeFrozenForTesting:]):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::destroyProvisionalPage):
+        (WebKit::WebPageProxy::isLayerTreeFrozen):
+        * UIProcess/WebPageProxy.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::isLayerTreeFrozen):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        Add new SPI to check if the layer tree is frozen in the WebProcess so that I could
+        write an API test for this.
+
+        * UIProcess/ProvisionalPageProxy.cpp:
+        (WebKit::ProvisionalPageProxy::ProvisionalPageProxy):
+        Make sure m_provisionalLoadURL gets initialized when the ProvisionalPageProxy gets
+        constructed *after* the provisional load has already started (which is the case
+        when the process swap is triggered by COOP).
+
+        (WebKit::ProvisionalPageProxy::didFailProvisionalLoadForFrame):
+        If the provisional load load fails in the provisional process, and the ProvisionalPageProxy
+        was constructed on resource response (COOP case), then no longer forward the
+        didFailProvisionalLoadForFrame() to the WebPageProxy. Instead, we destroy the
+        ProvisionalPageProxy. This is to avoid duplicate calls to didFailProvisionalLoadForFrame().
+        In this case, there is still a provisional load ongoing in the committed process and the
+        ProvisionalPageProxy destructor will take care of stopping that provisional load (due to
+        r286479), which will cause the committed process to send its own
+        didFailProvisionalLoadForFrame IPC.
+
 2021-12-03  Wenson Hsieh  <wenson_hsieh@apple.com>
 

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

@@ -117 +117 @@
 - (void)_gpuToWebProcessConnectionCountForTesting:(void(^)(NSUInteger))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
 
+- (void)_isLayerTreeFrozenForTesting:(void (^)(BOOL frozen))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
+
 @end
 

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

@@ -425 +425 @@
 }
 
+- (void)_isLayerTreeFrozenForTesting:(void (^)(BOOL frozen))completionHandler
+{
+    _page->isLayerTreeFrozen([completionHandler = makeBlockPtr(completionHandler)](bool isFrozen) {
+        completionHandler(isFrozen);
+    });
+}
+
 - (void)_gpuToWebProcessConnectionCountForTesting:(void(^)(NSUInteger))completionHandler
 {

trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp

@@ -68 +68 @@
     , m_processSwapRequestedByClient(processSwapRequestedByClient)
     , m_isProcessSwappingOnNavigationResponse(isProcessSwappingOnNavigationResponse)
+    , m_provisionalLoadURL(isProcessSwappingOnNavigationResponse ? request.url() : URL())
 #if PLATFORM(IOS_FAMILY)
     , m_provisionalLoadActivity(m_process->throttler().foregroundActivity("Provisional Load"_s))
@@ -297 +298 @@
     m_provisionalLoadURL = { };
 
+    if (m_isProcessSwappingOnNavigationResponse) {
+        // If the provisional load fails and we were process-swapping on navigation response, then we simply destroy ourselves.
+        // In this case, the provisional load is still ongoing in the committed process and the ProvisionalPageProxy destructor
+        // will stop it and cause the committed process to send its own DidFailProvisionalLoadForFrame IPC.
+        ASSERT(m_page.provisionalPageProxy() == this);
+        m_page.destroyProvisionalPage();
+        return;
+    }
+
     // Make sure the Page's main frame's expectedURL gets cleared since we updated it in didStartProvisionalLoad.
     if (auto* pageMainFrame = m_page.mainFrame())

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -3588 +3588 @@
 }
 
+void WebPageProxy::destroyProvisionalPage()
+{
+    m_provisionalPage = nullptr;
+}
+
 void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, std::unique_ptr<SuspendedPageProxy>&& suspendedPage, Ref<WebProcessProxy>&& newProcess, ProcessSwapRequestedByClient processSwapRequestedByClient, ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad, RefPtr<API::WebsitePolicies>&& websitePolicies, std::optional<NetworkResourceLoadIdentifier> existingNetworkResourceLoadIdentifierToResume)
 {
@@ -10801 +10806 @@
 }
 
+void WebPageProxy::isLayerTreeFrozen(CompletionHandler<void(bool)>&& completionHandler)
+{
+    sendWithAsyncReply(Messages::WebPage::IsLayerTreeFrozen(), WTFMove(completionHandler));
+}
+
 void WebPageProxy::requestSpeechRecognitionPermission(WebCore::SpeechRecognitionRequest& request, CompletionHandler<void(std::optional<SpeechRecognitionError>&&)>&& completionHandler)
 {

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -1503 +1503 @@
     std::optional<WebCore::ScrollbarOverlayStyle> overlayScrollbarStyle() const { return m_scrollbarOverlayStyle; }
 
+    void isLayerTreeFrozen(CompletionHandler<void(bool)>&&);
+
     // When the state of the window changes such that the WebPage needs immediate update, the UIProcess sends a new
     // ActivityStateChangeID to the WebProcess through the SetActivityState message. The UIProcess will wait till it
@@ -1771 +1773 @@
     ProvisionalPageProxy* provisionalPageProxy() const { return m_provisionalPage.get(); }
     void commitProvisionalPage(WebCore::FrameIdentifier, FrameInfoData&&, WebCore::ResourceRequest&&, uint64_t navigationID, const String& mimeType, bool frameHasCustomContentProvider, WebCore::FrameLoadType, const WebCore::CertificateInfo&, bool usedLegacyTLS, bool containsPluginDocument, std::optional<WebCore::HasInsecureContent> forcedHasInsecureContent, WebCore::MouseEventPolicy, const UserData&);
+    void destroyProvisionalPage();
 
     // Logic shared between the WebPageProxy and the ProvisionalPageProxy.

trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -2788 +2788 @@
 }
 
+void WebPage::isLayerTreeFrozen(CompletionHandler<void(bool)>&& completionHandler)
+{
+    completionHandler(!!m_layerTreeFreezeReasons);
+}
+
 void WebPage::updateDrawingAreaLayerTreeFreezeState()
 {

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -860 +860 @@
     void unfreezeLayerTree(LayerTreeFreezeReason);
 
+    void isLayerTreeFrozen(CompletionHandler<void(bool)>&&);
+
     void markLayersVolatile(CompletionHandler<void(bool)>&& completionHandler = { });
     void cancelMarkLayersVolatile();

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

@@ -403 +403 @@
     FreezeLayerTreeDueToSwipeAnimation()
     UnfreezeLayerTreeDueToSwipeAnimation()
+    IsLayerTreeFrozen() -> (bool isFrozen) Async
 
     # Printing.

trunk/Tools/ChangeLog

@@ +1 @@
+2021-12-03  Chris Dumez  <cdumez@apple.com>
+
+        Follow-up to r286479 to add API test and address issues found by the test
+        https://bugs.webkit.org/show_bug.cgi?id=233798
+
+        Reviewed by Darin Adler.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2021-12-03  Alex Christensen  <achristensen@webkit.org>
 

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

@@ -7265 +7265 @@
     // Clear the WebProcess cache while the processes are being checked for responsiveness.
     [processPool _clearWebProcessCache];
+}
+
+TEST(ProcessSwap, ResponsePolicyDownloadAfterCOOPProcessSwap)
+{
+    using namespace TestWebKitAPI;
+
+    HTTPServer server({
+        { "/source.html", { "foo" } },
+        { "/destination.html", { { { "Content-Type", "text/html" }, { "Cross-Origin-Opener-Policy", "same-origin" } }, "bar" } },
+    }, HTTPServer::Protocol::Https);
+
+    auto processPoolConfiguration = psonProcessPoolConfiguration();
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    for (_WKExperimentalFeature *feature in [WKPreferences _experimentalFeatures]) {
+        if ([feature.key isEqualToString:@"CrossOriginOpenerPolicyEnabled"])
+            [[webViewConfiguration preferences] _setEnabled:YES forExperimentalFeature:feature];
+        else if ([feature.key isEqualToString:@"CrossOriginEmbedderPolicyEnabled"])
+            [[webViewConfiguration preferences] _setEnabled:YES forExperimentalFeature:feature];
+    }
+
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+    auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:navigationDelegate.get()];
+
+    done = false;
+    [webView loadRequest:server.request("/source.html")];
+    Util::run(&done);
+    done = false;
+
+    auto pid1 = [webView _webProcessIdentifier];
+
+    // The next navigation will get converted into a download via decidePolicyForNavigationResponse.
+    shouldConvertToDownload = true;
+
+    done = false;
+    failed = false;
+    [webView loadRequest:server.request("/destination.html")];
+    Util::run(&failed);
+    failed = false;
+    shouldConvertToDownload = false;
+
+    auto pid2 = [webView _webProcessIdentifier];
+    EXPECT_EQ(pid1, pid2);
+
+    // The layer tree should no longer be frozen since the navigation didn't happen.
+    __block bool isFrozen = true;
+    do {
+        Util::sleep(0.1);
+        done = false;
+        [webView _isLayerTreeFrozenForTesting:^(BOOL frozen) {
+            isFrozen = frozen;
+            done = true;
+        }];
+        Util::run(&done);
+    } while (isFrozen);
 }