Attachment 360757 Details for Bug 194105 – Patch

[patch] Patch

bug-194105-20190131124719.patch (text/plain), 9.96 KB, created by Chris Dumez on 2019-01-31 12:47:20 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-31 12:47:20 PST

Size: 9.96 KB

patch

obsolete

>Subversion Revision: 240793
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index dd09e1bcd77c13904f0edef65253f119395e0a05..93a117a31af4bfe4393069035dfc16e8b4abd79b 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-01-31  Chris Dumez  <cdumez@apple.com>
>+
>+        Page zoom level is lost after a process swap or a crash
>+        https://bugs.webkit.org/show_bug.cgi?id=194105
>+        <rdar://problem/47610781>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Previously, when the client would call setPageAndTextZoomFactors() on the WebPageProxy,
>+        we would update the WebPageProxy's corresponding data members and send an IPC to the
>+        WebProcess to apply the zoom factors.
>+
>+        The issue is that on process crash or process-swap, we never communicate those zoom factors
>+        to the new WebProcess. Even if the client were to call setPageAndTextZoomFactors() with
>+        the same factors again, it would be a no-op since the WebPageProxy's data members already
>+        reflect the expected values.
>+
>+        To address the issue, pass both the page zoom and the text zoom factors to the WebProcess
>+        via WebPageCreationParameters. This way, there is no need to send an extra IPC and we're
>+        sure the WebPageProxy's factors are properly applied to the WebPage on WebContent process
>+        side upon creation (whether after a crash or a process swap).
>+
>+        * Shared/WebPageCreationParameters.cpp:
>+        (WebKit::WebPageCreationParameters::encode const):
>+        (WebKit::WebPageCreationParameters::decode):
>+        * Shared/WebPageCreationParameters.h:
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::creationParameters):
>+        * WebProcess/WebPage/WebPage.cpp:
>+
> 2019-01-30  Simon Fraser  <simon.fraser@apple.com>
> 
>         [Mac] Implement basic hit testing in the scrolling tree
>diff --git a/Source/WebKit/Shared/WebPageCreationParameters.cpp b/Source/WebKit/Shared/WebPageCreationParameters.cpp
>index af75d99771dea4f19f62030d933d8fac6d5dbb38..b164cad7ce6d5fbeb6fdd310abf1add26c724998 100644
>--- a/Source/WebKit/Shared/WebPageCreationParameters.cpp
>+++ b/Source/WebKit/Shared/WebPageCreationParameters.cpp
>@@ -60,6 +60,8 @@ void WebPageCreationParameters::encode(IPC::Encoder& encoder) const
>     encoder << canRunModal;
>     encoder << deviceScaleFactor;
>     encoder << viewScaleFactor;
>+    encoder << textZoomFactor;
>+    encoder << pageZoomFactor;
>     encoder << topContentInset;
>     encoder << mediaVolume;
>     encoder << muted;
>@@ -196,6 +198,10 @@ Optional<WebPageCreationParameters> WebPageCreationParameters::decode(IPC::Decod
>         return WTF::nullopt;
>     if (!decoder.decode(parameters.viewScaleFactor))
>         return WTF::nullopt;
>+    if (!decoder.decode(parameters.textZoomFactor))
>+        return WTF::nullopt;
>+    if (!decoder.decode(parameters.pageZoomFactor))
>+        return WTF::nullopt;
>     if (!decoder.decode(parameters.topContentInset))
>         return WTF::nullopt;
>     if (!decoder.decode(parameters.mediaVolume))
>diff --git a/Source/WebKit/Shared/WebPageCreationParameters.h b/Source/WebKit/Shared/WebPageCreationParameters.h
>index fb327e40820dba7223bd02013d2c126273374ece..d1b0303d72fe0d95e2c8c9fe0f162ed48d71f193 100644
>--- a/Source/WebKit/Shared/WebPageCreationParameters.h
>+++ b/Source/WebKit/Shared/WebPageCreationParameters.h
>@@ -106,6 +106,9 @@ struct WebPageCreationParameters {
>     float deviceScaleFactor;
>     float viewScaleFactor;
> 
>+    double textZoomFactor { 1 };
>+    double pageZoomFactor { 1 };
>+
>     float topContentInset;
>     
>     float mediaVolume;
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index f8a299508792811026c6bf24967a9563fcca8ee2..89c3b9d226ea72ed33cf9632fdba2a0fa2d055c4 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -6786,6 +6786,8 @@ WebPageCreationParameters WebPageProxy::creationParameters(WebProcessProxy& proc
>     parameters.canRunModal = m_canRunModal;
>     parameters.deviceScaleFactor = deviceScaleFactor();
>     parameters.viewScaleFactor = m_viewScaleFactor;
>+    parameters.textZoomFactor = m_textZoomFactor;
>+    parameters.pageZoomFactor = m_pageZoomFactor;
>     parameters.topContentInset = m_topContentInset;
>     parameters.mediaVolume = m_mediaVolume;
>     parameters.muted = m_mutedState;
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 23a9780a8afe98180bf3703f65cc4e03f3319bff..d9114af784b527d9cba6b15c3cf855bdc5f37894 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -484,6 +484,7 @@ WebPage::WebPage(uint64_t pageID, WebPageCreationParameters&& parameters)
>     m_drawingArea->updatePreferences(parameters.store);
> 
>     setBackgroundExtendsBeyondPage(parameters.backgroundExtendsBeyondPage);
>+    setPageAndTextZoomFactors(parameters.pageZoomFactor, parameters.textZoomFactor);
> 
> #if ENABLE(GEOLOCATION)
>     WebCore::provideGeolocationTo(m_page.get(), *new WebGeolocationClient(*this));
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 16c9d467874be85a7ee0c7c3ad6cbc4ba36166d3..a0e007e24f1efd86408e5af817a0679a29d3877b 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-01-31  Chris Dumez  <cdumez@apple.com>
>+
>+        Page zoom level is lost after a process swap or a crash
>+        https://bugs.webkit.org/show_bug.cgi?id=194105
>+        <rdar://problem/47610781>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+        (-[PSONNavigationDelegate webView:didCommitNavigation:]):
>+
> 2019-01-31  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Margin before/after/start/end initial value is 0 and not auto.
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index 7ee9eebb6ec4289e07e539c5194c26eff35dcbf4..bdbca5ee501af89d54478b58f8176c7ab77f98b2 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -97,6 +97,7 @@ @end
> 
> @interface PSONNavigationDelegate : NSObject <WKNavigationDelegate> {
>     @public void (^decidePolicyForNavigationAction)(WKNavigationAction *, void (^)(WKNavigationActionPolicy));
>+    @public void (^didCommitNavigationHandler)();
> }
> @end
> 
>@@ -125,6 +126,12 @@ - (void)webView:(WKWebView *)webView didStartProvisionalNavigation:(null_unspeci
>     didStartProvisionalLoad = true;
> }
> 
>+- (void)webView:(WKWebView *)webView didCommitNavigation:(null_unspecified WKNavigation *)navigation
>+{
>+    if (didCommitNavigationHandler)
>+        didCommitNavigationHandler();
>+}
>+
> - (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
> {
>     ++numberOfDecidePolicyCalls;
>@@ -2491,6 +2498,83 @@ TEST(ProcessSwap, MainFramesOnly)
>     EXPECT_EQ(1u, seenPIDs.size());
> }
> 
>+#if PLATFORM(MAC)
>+
>+static const char* sendBodyClientWidth = R"PSONRESOURCE(
>+<body>
>+TEST
>+<script>
>+onload = () => {
>+    document.body.offsetTop;    // force layout
>+    setTimeout(() => {
>+        window.webkit.messageHandlers.pson.postMessage("" + document.body.clientWidth);
>+    });
>+}
>+</script>
>+</body>
>+)PSONRESOURCE";
>+
>+TEST(ProcessSwap, PageZoomLevelAfterSwap)
>+{
>+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
>+    [processPoolConfiguration setProcessSwapsOnNavigation: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:sendBodyClientWidth];
>+    [handler addMappingFromURLString:@"pson://www.apple.com/main.html" toData:sendBodyClientWidth];
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
>+
>+    auto 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()];
>+
>+    delegate->didCommitNavigationHandler = ^ {
>+        [webView _setPageZoomFactor:2.0];
>+        delegate->didCommitNavigationHandler = nil;
>+    };
>+
>+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
>+    [webView loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&receivedMessage);
>+    receivedMessage = false;
>+
>+    EXPECT_WK_STREQ(@"400", receivedMessages.get()[0]);
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
>+    [webView loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&receivedMessage);
>+    receivedMessage = false;
>+
>+    EXPECT_WK_STREQ(@"400", receivedMessages.get()[1]);
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    // Kill the WebProcess, the page should reload automatically and the page zoom level should be maintained.
>+    kill([webView _webProcessIdentifier], 9);
>+
>+    TestWebKitAPI::Util::run(&receivedMessage);
>+    receivedMessage = false;
>+
>+    EXPECT_WK_STREQ(@"400", receivedMessages.get()[2]);
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+}
>+
>+#endif // PLATFORM(MAC)
>+
> TEST(ProcessSwap, DoSameSiteNavigationAfterCrossSiteProvisionalLoadStarted)
> {
>     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);

Actions: View | Formatted Diff | Diff

Attachments on bug 194105: 360757