Attachment 362687 Details for Bug 194924 – Patch

[patch] Patch

bug-194924-20190221194501.patch (text/plain), 6.57 KB, created by Chris Dumez on 2019-02-21 19:45:03 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-02-21 19:45:03 PST

Size: 6.57 KB

patch

obsolete

>Subversion Revision: 241864
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 92c66d4dcfaaa152ea88c3d785a6c4eecb9b0e8e..94a39fd0eec11d69ed5e6243870fbb4d6d5c9942 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-02-21  Chris Dumez  <cdumez@apple.com>
>+
>+        REGRESSION(PSON) Scroll position is sometimes not restored on history navigation
>+        https://bugs.webkit.org/show_bug.cgi?id=194924
>+        <rdar://problem/48216125>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When process-swapping, we would create a new WebPage in the new process, which would
>+        call restoreSessionInternal() to restore the HistoryItems based on the UIProcess'
>+        backforward list. The issue is that this session restoring would send HistoryItem
>+        updates back to the UIProcess. Without PSON, this would be unnecessary but harmless.
>+        With PSON though, this may end up overwriting values set by the previous process,
>+        such as the scroll position.
>+
>+        Address the issue by temporarily disabling the HistoryItem update notifications to
>+        the UIProcess while restoring a session.
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::ignoreHistoryItemUpdates):
>+        (WebKit::WebPage::restoreSessionInternal):
>+
> 2019-02-20  Antti Koivisto  <antti@apple.com>
> 
>         Make programmatic frame scrolling work on iOS
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 085d82a8e64d97ab6219948000ab9fc7a9b78fb0..c3155d3c8fd3fafe3a69390e43676dbf44b95752 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -2702,6 +2702,8 @@ void WebPage::setNeedsFontAttributes(bool needsFontAttributes)
> 
> void WebPage::restoreSessionInternal(const Vector<BackForwardListItemState>& itemStates, WasRestoredByAPIRequest restoredByAPIRequest, WebBackForwardListProxy::OverwriteExistingItem overwrite)
> {
>+    // Since we're merely restoring HistoryItems from the UIProcess, there is no need to send HistoryItem update notifications back to the UIProcess.
>+    SetForScope<void (*)(WebCore::HistoryItem&)> bypassHistoryItemUpdateNotifications(WebCore::notifyHistoryItemChanged, [](auto){});
>     for (const auto& itemState : itemStates) {
>         auto historyItem = toHistoryItem(itemState);
>         historyItem->setWasRestoredFromSession(restoredByAPIRequest == WasRestoredByAPIRequest::Yes);
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 4f32b37f60af1e516528fe5da88f4c1dda354310..b69a36af161d092a1d61720c17967087d146b8ed 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-02-21  Chris Dumez  <cdumez@apple.com>
>+
>+        REGRESSION(PSON) Scroll position is sometimes not restored on history navigation
>+        https://bugs.webkit.org/show_bug.cgi?id=194924
>+        <rdar://problem/48216125>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> 2019-02-21  Rob Buis  <rbuis@igalia.com>
> 
>         Update MIME type parser
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index 37f6773b0c873257db494963ae1cfc8ba1c0e73c..5f160eea50f70ce19e00d42148c5c67968f9e628 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -4794,6 +4794,8 @@ TEST(ProcessSwap, GoBackToSuspendedPageWithMainFrameIDThatIsNotOne)
>     EXPECT_EQ(pid1, pid5);
> }
> 
>+#endif // PLATFORM(MAC)
>+
> static const char* tallPageBytes = R"PSONRESOURCE(
> <!DOCTYPE html>
> <html>
>@@ -4808,11 +4810,30 @@ body {
> </style>
> </head>
> <body>
>+<script>
>+// Pages with dedicated workers do not go into page cache.
>+var myWorker = new Worker('worker.js');
>+</script>
> <a id="testLink" href="pson://www.apple.com/main.html">Test</a>
> </body>
> </html>
> )PSONRESOURCE";
> 
>+static unsigned waitUntilScrollPositionIsRestored(WKWebView *webView)
>+{
>+    unsigned scrollPosition = 0;
>+    do {
>+        [webView evaluateJavaScript:@"window.scrollY" completionHandler: [&] (id result, NSError *error) {
>+            scrollPosition = [result integerValue];
>+            done = true;
>+        }];
>+        TestWebKitAPI::Util::run(&done);
>+        done = false;
>+    } while (!scrollPosition);
>+
>+    return scrollPosition;
>+}
>+
> TEST(ProcessSwap, ScrollPositionRestoration)
> {
>     auto processPoolConfiguration = psonProcessPoolConfiguration();
>@@ -4824,8 +4845,6 @@ TEST(ProcessSwap, ScrollPositionRestoration)
>     [handler addMappingFromURLString:@"pson://www.webkit.org/main.html" toData:tallPageBytes];
>     [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
> 
>-    [[webViewConfiguration preferences] _setUsesPageCache:NO];
>-
>     auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
>     auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
>     [webView setNavigationDelegate:delegate.get()];
>@@ -4841,6 +4860,8 @@ TEST(ProcessSwap, ScrollPositionRestoration)
>     TestWebKitAPI::Util::run(&done);
>     done = false;
> 
>+    TestWebKitAPI::Util::sleep(0.5);
>+
>     [webView evaluateJavaScript:@"testLink.click()" completionHandler: nil];
> 
>     TestWebKitAPI::Util::run(&done);
>@@ -4857,15 +4878,36 @@ TEST(ProcessSwap, ScrollPositionRestoration)
>     TestWebKitAPI::Util::run(&done);
>     done = false;
> 
>+    auto scrollPosition = waitUntilScrollPositionIsRestored(webView.get());
>+    EXPECT_EQ(5000U, scrollPosition);
>+
>+    [webView evaluateJavaScript:@"scroll(0, 4000)" completionHandler: [&] (id result, NSError *error) {
>+        done = true;
>+    }];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    TestWebKitAPI::Util::sleep(0.5);
>+
>+    [webView evaluateJavaScript:@"testLink.click()" completionHandler: nil];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>     [webView evaluateJavaScript:@"window.scrollY" completionHandler: [&] (id result, NSError *error) {
>-        EXPECT_EQ(5000, [result integerValue]);
>+        EXPECT_EQ(0, [result integerValue]);
>         done = true;
>     }];
>     TestWebKitAPI::Util::run(&done);
>     done = false;
>-}
> 
>-#endif // PLATFORM(MAC)
>+    [webView goBack];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    scrollPosition = waitUntilScrollPositionIsRestored(webView.get());
>+    EXPECT_EQ(4000U, scrollPosition);
>+}
> 
> static NSString *blockmeFilter = @"[{\"action\":{\"type\":\"block\"},\"trigger\":{\"url-filter\":\".*blockme.html\"}}]";
>

Actions: View | Formatted Diff | Diff

Attachments on bug 194924: 362671 | 362681 | 362687 | 362689