Attachment 361276 Details for Bug 194329 – Fixes the bug

[patch] Fixes the bug

bug-194329-20190205210601.patch (text/plain), 5.50 KB, created by Ryosuke Niwa on 2019-02-05 21:06:01 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2019-02-05 21:06:01 PST

Size: 5.50 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 241009)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,21 @@
>+2019-02-05  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        REGRESSION (r240909): Release assert in FrameLoader::loadURL when navigating with a non-existent target name
>+        https://bugs.webkit.org/show_bug.cgi?id=194329
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The bug was caused by the code path for when navigating with a specific target frame name that does not exist
>+        never setting the load type of PolicyChecker. As a result, we would use whatever load type used in the previous
>+        navigation, resulting in this release assertion.
>+
>+        Updating the load type here should in theory fix the underlying bug r240909 was meant to catch & fix.
>+
>+        Test: fast/loader/navigate-with-new-target-after-back-forward-navigation.html
>+
>+        * loader/FrameLoader.cpp:
>+        (WebCore::FrameLoader::loadURL):
>+
> 2019-02-05  Alex Christensen  <achristensen@webkit.org>
> 
>         Stop using blobRegistry in NetworkProcess
>Index: Source/WebCore/loader/FrameLoader.cpp
>===================================================================
>--- Source/WebCore/loader/FrameLoader.cpp	(revision 241008)
>+++ Source/WebCore/loader/FrameLoader.cpp	(working copy)
>@@ -1379,6 +1379,7 @@ void FrameLoader::loadURL(FrameLoadReque
> 
>     if (!targetFrame && !effectiveFrameName.isEmpty()) {
>         action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, frameLoadRequest));
>+        policyChecker().setLoadType(newLoadType);
>         RELEASE_ASSERT(!isBackForwardLoadType(policyChecker().loadType()));
>         policyChecker().checkNewWindowPolicy(WTFMove(action), WTFMove(request), WTFMove(formState), effectiveFrameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) mutable {
>             continueLoadAfterNewWindowPolicy(request, formState.get(), frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 241008)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2019-02-05  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        REGRESSION (r240909): Release assert in FrameLoader::loadURL when navigating with a non-existent target name
>+        https://bugs.webkit.org/show_bug.cgi?id=194329
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a regression test.
>+
>+        * fast/loader/navigate-with-new-target-after-back-forward-navigation-expected.txt: Added.
>+        * fast/loader/navigate-with-new-target-after-back-forward-navigation.html: Added.
>+
> 2019-02-05  Nikita Vasilyev  <nvasilyev@apple.com>
> 
>         Web Inspector: Fix modify-css-property-race.html test failures
>Index: LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation-expected.txt
>===================================================================
>--- LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation-expected.txt	(nonexistent)
>+++ LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation-expected.txt	(working copy)
>@@ -0,0 +1,5 @@
>+ALERT: PASS
>+This tests navigating via an anchor element with a non-existent target name, which should create a new window.
>+WebKit should not hit any assertions and alert "PASS".
>+
>+
>Index: LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation.html
>===================================================================
>--- LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation.html	(nonexistent)
>+++ LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation.html	(working copy)
>@@ -0,0 +1,51 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script>
>+
>+if (window.testRunner) {
>+    testRunner.dumpAsText();
>+    testRunner.waitUntilDone();
>+    testRunner.setCanOpenWindows();
>+    testRunner.setCloseRemainingWindowsWhenComplete();
>+    testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
>+}
>+
>+if (location.search == '?third') {
>+    alert('PASS');
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+} else if (self == top) {
>+    document.write(`<p>This tests navigating via an anchor element with a non-existent target name, which should create a new window.<br>
>+WebKit should not hit any assertions and alert "PASS".</p>`);
>+    const frame = document.createElement('iframe');
>+    frame.src = '?first';
>+    let step = 0;
>+    frame.onload = () => {
>+        switch (step++) {
>+        case 0:
>+            setTimeout(() => frame.src = '?second', 0);
>+            break;
>+        case 1:
>+            setTimeout(() => history.back(), 0);
>+            break;
>+        }
>+    }
>+    document.body.appendChild(frame);
>+} else {
>+    if (location.search == '?first') {
>+        if (localStorage.getItem('loaded')) {
>+            localStorage.removeItem('loaded');
>+            window.onload = () => {
>+                setTimeout(() => document.querySelector('a').click(), 0);
>+            }
>+        } else
>+            localStorage.setItem('loaded', 'true');
>+    }
>+    document.write(location.search);
>+    document.write(` <a href="?third" target="unknownTarget">go</a>`);
>+}
>+
>+</script>
>+</body>
>+</html>

Flags:

ggaren: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194329: 361276