Changeset 241015 in webkit

Timestamp:

Feb 6, 2019 1:42:49 AM (5 years ago)

Author:

rniwa@webkit.org

Message:

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 Geoffrey Garen.

Source/WebCore:

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

LayoutTests:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation-expected.txt (added)
• LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/FrameLoader.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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 Geoffrey Garen.
+
+        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>
 

trunk/LayoutTests/TestExpectations

@@ -6 +6 @@
 # Platform-specific tests. Skipped here, then re-enabled on the appropriate platform.
 #//////////////////////////////////////////////////////////////////////////////////////////
+
+webgl [ Skip ]
 
 compositing/ios [ Skip ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 Geoffrey Garen.
+
+        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  Claudio Saavedra  <csaavedra@igalia.com>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -1380 +1380 @@
     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 {