- Revision
- 241015
- Author
- [email protected]
- Date
- 2019-02-06 01:42:49 -0800 (Wed, 06 Feb 2019)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (241014 => 241015)
--- trunk/LayoutTests/ChangeLog 2019-02-06 09:35:16 UTC (rev 241014)
+++ trunk/LayoutTests/ChangeLog 2019-02-06 09:42:49 UTC (rev 241015)
@@ -1,3 +1,15 @@
+2019-02-05 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Web Inspector: Styles: PropertiesChanged shouldn't fire when old and new text are both empty
Modified: trunk/LayoutTests/TestExpectations (241014 => 241015)
--- trunk/LayoutTests/TestExpectations 2019-02-06 09:35:16 UTC (rev 241014)
+++ trunk/LayoutTests/TestExpectations 2019-02-06 09:42:49 UTC (rev 241015)
@@ -6,6 +6,8 @@
# Platform-specific tests. Skipped here, then re-enabled on the appropriate platform.
#//////////////////////////////////////////////////////////////////////////////////////////
+webgl [ Skip ]
+
compositing/ios [ Skip ]
css3/touch-action [ Skip ]
accessibility/ios-simulator [ Skip ]
Added: trunk/LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation-expected.txt (0 => 241015)
--- trunk/LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation-expected.txt 2019-02-06 09:42:49 UTC (rev 241015)
@@ -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".
+
+
Added: trunk/LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation.html (0 => 241015)
--- trunk/LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation.html (rev 0)
+++ trunk/LayoutTests/fast/loader/navigate-with-new-target-after-back-forward-navigation.html 2019-02-06 09:42:49 UTC (rev 241015)
@@ -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 = '';
+ let step = 0;
+ frame._onload_ = () => {
+ switch (step++) {
+ case 0:
+ setTimeout(() => frame.src = '', 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="" target="unknownTarget">go</a>`);
+}
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (241014 => 241015)
--- trunk/Source/WebCore/ChangeLog 2019-02-06 09:35:16 UTC (rev 241014)
+++ trunk/Source/WebCore/ChangeLog 2019-02-06 09:42:49 UTC (rev 241015)
@@ -1,3 +1,21 @@
+2019-02-05 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
[FreeType] Build fix for Debian stable
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (241014 => 241015)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2019-02-06 09:35:16 UTC (rev 241014)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2019-02-06 09:42:49 UTC (rev 241015)
@@ -1379,6 +1379,7 @@
if (!targetFrame && !effectiveFrameName.isEmpty()) {
action = "" 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);
