Attachment 358557 Details for Bug 193224 – Patch

[patch] Patch

bug-193224-20190107165000.patch (text/plain), 5.91 KB, created by Chris Dumez on 2019-01-07 16:50:01 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-07 16:50:01 PST

Size: 5.91 KB

patch

obsolete

>Subversion Revision: 239693
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 598756fdfa6bf4c6756e7ebb6ebb72f37af4ffd2..37ce363f9a1a3c505e3f9d977326bcfafd5d7ae9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-01-07  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON-r239182): Blank view when navigating back and forth between google.com and stack overflow
>+        https://bugs.webkit.org/show_bug.cgi?id=193224
>+        <rdar://problem/47097726>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Since r239182, pages get suspended in-place when we suspend the old process after a process-swap on navigation.
>+        When we return to a suspended page, we load the current history item again and it normally properly restores
>+        the page from PageCache, even though we load the same history item and the current one and even though the
>+        page is suspended in-place (i.e. we did not navigate away, which is the usual case for page cache).
>+
>+        The issue is that if the page URL contains a fragment, FrameLoader::shouldPerformFragmentNavigation() would
>+        return true because both the source and destination URLs (which are the same) contains a fragment. To address
>+        the issue, update FrameLoader::shouldPerformFragmentNavigation() to return false if the current page is
>+        suspended.
>+
>+        * loader/FrameLoader.cpp:
>+        (WebCore::FrameLoader::shouldPerformFragmentNavigation):
>+
> 2019-01-07  Chris Dumez  <cdumez@apple.com>
> 
>         Prevent cross-site top-level navigations from third-party iframes
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index 51bb9488e59c7df3b633a02059db12f91c3208f1..4c26a6981b464da5916f05eb7359669157ee9ffe 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -3134,6 +3134,7 @@ bool FrameLoader::shouldPerformFragmentNavigation(bool isFormSubmission, const S
>     return (!isFormSubmission || equalLettersIgnoringASCIICase(httpMethod, "get"))
>         && !isReload(loadType)
>         && loadType != FrameLoadType::Same
>+        && m_frame.document()->pageCacheState() != Document::InPageCache
>         && !shouldReload(m_frame.document()->url(), url)
>         // We don't want to just scroll if a link from within a
>         // frameset is trying to reload the frameset into _top.
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index d5fb0a31b6994b44f76d69820541babf494cb739..b2933d12485fe65d8dec8c38a62615fe5d1f567f 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-01-07  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON-r239182): Blank view when navigating back and forth between google.com and stack overflow
>+        https://bugs.webkit.org/show_bug.cgi?id=193224
>+        <rdar://problem/47097726>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
>+
> 2019-01-07  Eric Carlson  <eric.carlson@apple.com>
> 
>         A MediaTime timescale must never be zero
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>index 96133a9877799018a5b59410641a1fd268520fef..a954546405a58ba81e065f1d3186485a47a7e6bd 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm
>@@ -685,6 +685,63 @@ TEST(ProcessSwap, Back)
>         EXPECT_EQ(5u, seenPIDs.size());
> }
> 
>+static const char* pageWithFragmentTestBytes = R"PSONRESOURCE(
>+<div id="foo">TEST</div>
>+)PSONRESOURCE";
>+
>+TEST(ProcessSwap, HistoryNavigationToFragmentURL)
>+{
>+    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.apple.com/main.html#foo" toData:pageWithFragmentTestBytes];
>+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"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];
>+
>+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html#foo"]];
>+    [webView loadRequest:request];
>+
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    auto applePID = [webView _webProcessIdentifier];
>+
>+    [webView goBack];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
>+
>+    [webView goForward];
>+    TestWebKitAPI::Util::run(&done);
>+    done = false;
>+
>+    EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
>+
>+    bool finishedRunningScript = false;
>+    [webView evaluateJavaScript:@"document.getElementById('foo').innerText" completionHandler: [&] (id result, NSError *error) {
>+        NSString *innerText = (NSString *)result;
>+        EXPECT_WK_STREQ(@"TEST", innerText);
>+        finishedRunningScript = true;
>+    }];
>+    TestWebKitAPI::Util::run(&finishedRunningScript);
>+}
>+
> TEST(ProcessSwap, SuspendedPageDiesAfterBackForwardListItemIsGone)
> {
>     auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);

Actions: View | Formatted Diff | Diff

Attachments on bug 193224: 358557