Title: [286505] trunk
Revision
286505
Author
[email protected]
Date
2021-12-03 11:39:02 -0800 (Fri, 03 Dec 2021)

Log 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:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (286504 => 286505)


--- trunk/Source/WebKit/ChangeLog	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/ChangeLog	2021-12-03 19:39:02 UTC (rev 286505)
@@ -1,3 +1,42 @@
+2021-12-03  Chris Dumez  <[email protected]>
+
+        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  <[email protected]>
 
         [ Monterey wk2 ] http/tests/media/video-webm-stall.html (layout-test) is a constant text failure

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h (286504 => 286505)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h	2021-12-03 19:39:02 UTC (rev 286505)
@@ -116,6 +116,8 @@
 - (void)_createMediaSessionCoordinatorForTesting:(id <_WKMediaSessionCoordinator>)privateCoordinator completionHandler:(void(^)(BOOL))completionHandler;
 - (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
 
 typedef NS_ENUM(NSInteger, _WKMediaSessionReadyState) {

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm (286504 => 286505)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm	2021-12-03 19:39:02 UTC (rev 286505)
@@ -424,6 +424,13 @@
     });
 }
 
+- (void)_isLayerTreeFrozenForTesting:(void (^)(BOOL frozen))completionHandler
+{
+    _page->isLayerTreeFrozen([completionHandler = makeBlockPtr(completionHandler)](bool isFrozen) {
+        completionHandler(isFrozen);
+    });
+}
+
 - (void)_gpuToWebProcessConnectionCountForTesting:(void(^)(NSUInteger))completionHandler
 {
     RefPtr gpuProcess = _page->process().processPool().gpuProcess();

Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (286504 => 286505)


--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp	2021-12-03 19:39:02 UTC (rev 286505)
@@ -67,6 +67,7 @@
     , m_request(request)
     , 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))
 #endif
@@ -296,6 +297,15 @@
     ASSERT(!m_provisionalLoadURL.isNull());
     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())
         pageMainFrame->didFailProvisionalLoad();

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (286504 => 286505)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2021-12-03 19:39:02 UTC (rev 286505)
@@ -3587,6 +3587,11 @@
     m_inspectorController->didCommitProvisionalPage(oldWebPageID, m_webPageID);
 }
 
+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)
 {
     WEBPAGEPROXY_RELEASE_LOG(Loading, "continueNavigationInNewProcess: newProcessPID=%i, hasSuspendedPage=%i", newProcess->processIdentifier(), !!suspendedPage);
@@ -10800,6 +10805,11 @@
     });
 }
 
+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)
 {
     if (!m_speechRecognitionPermissionManager)

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (286504 => 286505)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.h	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h	2021-12-03 19:39:02 UTC (rev 286505)
@@ -1502,6 +1502,8 @@
     void setOverlayScrollbarStyle(std::optional<WebCore::ScrollbarOverlayStyle>);
     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
     // receives a CommitLayerTree which has an ActivityStateChangeID equal to or greater than the one it sent.
@@ -1770,6 +1772,7 @@
 
     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.
     void didStartProvisionalLoadForFrameShared(Ref<WebProcessProxy>&&, WebCore::FrameIdentifier, FrameInfoData&&, WebCore::ResourceRequest&&, uint64_t navigationID, URL&&, URL&& unreachableURL, const UserData&);

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (286504 => 286505)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-12-03 19:39:02 UTC (rev 286505)
@@ -2787,6 +2787,11 @@
     updateDrawingAreaLayerTreeFreezeState();
 }
 
+void WebPage::isLayerTreeFrozen(CompletionHandler<void(bool)>&& completionHandler)
+{
+    completionHandler(!!m_layerTreeFreezeReasons);
+}
+
 void WebPage::updateDrawingAreaLayerTreeFreezeState()
 {
     if (!m_drawingArea)

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (286504 => 286505)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2021-12-03 19:39:02 UTC (rev 286505)
@@ -859,6 +859,8 @@
     void freezeLayerTree(LayerTreeFreezeReason);
     void unfreezeLayerTree(LayerTreeFreezeReason);
 
+    void isLayerTreeFrozen(CompletionHandler<void(bool)>&&);
+
     void markLayersVolatile(CompletionHandler<void(bool)>&& completionHandler = { });
     void cancelMarkLayersVolatile();
 

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in (286504 => 286505)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in	2021-12-03 19:39:02 UTC (rev 286505)
@@ -402,6 +402,7 @@
 
     FreezeLayerTreeDueToSwipeAnimation()
     UnfreezeLayerTreeDueToSwipeAnimation()
+    IsLayerTreeFrozen() -> (bool isFrozen) Async
 
     # Printing.
     BeginPrinting(WebCore::FrameIdentifier frameID, struct WebKit::PrintInfo printInfo)

Modified: trunk/Tools/ChangeLog (286504 => 286505)


--- trunk/Tools/ChangeLog	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Tools/ChangeLog	2021-12-03 19:39:02 UTC (rev 286505)
@@ -1,3 +1,14 @@
+2021-12-03  Chris Dumez  <[email protected]>
+
+        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  <[email protected]>
 
         Add room for more bytecode in WKContentRuleList file format

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (286504 => 286505)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2021-12-03 19:21:51 UTC (rev 286504)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2021-12-03 19:39:02 UTC (rev 286505)
@@ -7266,6 +7266,64 @@
     [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);
+}
+
 enum class IsSameOrigin : bool { No, Yes };
 enum class DoServerSideRedirect : bool { No, Yes };
 static void runCOOPProcessSwapTest(const char* sourceCOOP, const char* sourceCOEP, const char* destinationCOOP, const char* destinationCOEP, IsSameOrigin isSameOrigin, DoServerSideRedirect doServerSideRedirect, ExpectSwap expectSwap)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to