Assertion failure in WebCore::FrameLoader::stopLoading() running fast/events tests
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Oct 2015 20:04:24 +0000 (20:04 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Oct 2015 20:04:24 +0000 (20:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150624
Source/WebCore:

Reviewed by Darin Adler.

After r191652, a form's target attribute can no longer refer to a frame's id,
only its name. This is because the frame's id no longer sets the Window name
when the frame's name attribute is missing. This caused a change in behavior
for the fast/events/form-iframe-target-before-load-crash*.html tests, which
exposed a pre-existing bug.

This patch updates the fast/events/form-iframe-target-before-load-crash*.html
tests so they keep testing the same thing as before r191652. It also adds a
variant to keep covering the newly exposed bug.

The issue was that the frame was no longer navigated when submitting the form
(due to the form's target not matching the frame name). Therefore, when
removing the iframe from the document, its navigation has not started yet and
DocumentLoadTiming::navigationStart() is not initialized yet when
FrameLoader::stopLoading() is called and we hit an assertion. This patch
replaces the assertion with an if check as we now know it can happen and we
have test coverage for it.

Test: fast/events/form-iframe-target-before-load-crash.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::stopLoading):

LayoutTests:

<rdar://problem/23294110>

Reviewed by Darin Adler.

* fast/events/form-iframe-target-before-load-crash2.html:
Set the frame name so that the test still tests the same thing as it was
originally testing before r191652. This is needed because the frame id
no longer sets the window name and therefore no longer matches the form's
target.

* fast/events/form-iframe-target-before-load-crash3-expected.txt: Added.
* fast/events/form-iframe-target-before-load-crash3.html: Added.
This is a version on fast/events/form-iframe-target-before-load-crash.html
with the frame name set so that it tests the same thing it was originally
testing before r191652. form-iframe-target-before-load-crash.html is kept
as is (without frame name) as it exposed a new crash.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@191688 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/form-iframe-target-before-load-crash2.html
LayoutTests/fast/events/form-iframe-target-before-load-crash3-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/form-iframe-target-before-load-crash3.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp

index 3962cc61b8110013a4ae6c6bb6ee72024669ead2..be683485fa12554a8fcef49d7c5e2ec46b9e76d9 100644 (file)
@@ -1,3 +1,24 @@
+2015-10-28  Chris Dumez  <cdumez@apple.com>
+
+        Assertion failure in WebCore::FrameLoader::stopLoading() running fast/events tests
+        https://bugs.webkit.org/show_bug.cgi?id=150624
+        <rdar://problem/23294110>
+
+        Reviewed by Darin Adler.
+
+        * fast/events/form-iframe-target-before-load-crash2.html:
+        Set the frame name so that the test still tests the same thing as it was
+        originally testing before r191652. This is needed because the frame id
+        no longer sets the window name and therefore no longer matches the form's
+        target.
+
+        * fast/events/form-iframe-target-before-load-crash3-expected.txt: Added.
+        * fast/events/form-iframe-target-before-load-crash3.html: Added.
+        This is a version on fast/events/form-iframe-target-before-load-crash.html
+        with the frame name set so that it tests the same thing it was originally
+        testing before r191652. form-iframe-target-before-load-crash.html is kept
+        as is (without frame name) as it exposed a new crash.
+
 2015-10-28  Mark Lam  <mark.lam@apple.com>
 
         Update FTL to support UntypedUse operands for op_sub.
index dae0316e394365cbcaf317104292f93c03ecc922..516d39338644d4daee82777fb096daa85fe7346f 100644 (file)
@@ -31,7 +31,7 @@
                 }\r
             }, true);\r
        </script>\r
-       <iframe id="test" src="about:blank"></iframe>\r
+       <iframe id="test" name="test" src="about:blank"></iframe>\r
    </body>\r
 </html>\r
 \r
diff --git a/LayoutTests/fast/events/form-iframe-target-before-load-crash3-expected.txt b/LayoutTests/fast/events/form-iframe-target-before-load-crash3-expected.txt
new file mode 100644 (file)
index 0000000..69cfc5a
--- /dev/null
@@ -0,0 +1,2 @@
+PASS
+
diff --git a/LayoutTests/fast/events/form-iframe-target-before-load-crash3.html b/LayoutTests/fast/events/form-iframe-target-before-load-crash3.html
new file mode 100644 (file)
index 0000000..0d8c694
--- /dev/null
@@ -0,0 +1,37 @@
+<html>\r
+    <script src="../../resources/js-test-pre.js"></script>\r
+    <body onload="runTest()">\r
+        <div id="console"></div>\r
+        <form id="form1" style="display:none" method="post" target="test" action="http://anything.com"></form>\r
+        <script>\r
+            if (window.testRunner)\r
+            {\r
+                testRunner.dumpAsText();\r
+                testRunner.waitUntilDone();\r
+            }\r
+        \r
+            function runTest()\r
+            {\r
+                document.getElementById('form1').submit();\r
+                \r
+                if (window.testRunner)\r
+                    testRunner.notifyDone();\r
+                document.getElementById('console').innerHTML = 'PASS';\r
+            }\r
+\r
+            count = 0;\r
+            document.addEventListener("beforeload", function(event) {\r
+                event.preventDefault();\r
+                count = count + 1;\r
+                if (count == 2)\r
+                {\r
+                    document.body.removeChild(document.getElementById('test'));\r
+                    gc();\r
+                    document.body.offsetTop;\r
+                }\r
+            }, true);\r
+       </script>\r
+       <iframe id="test" name="test" src="about:blank"></iframe>\r
+   </body>\r
+</html>\r
+\r
index 76a924826f3934654ca211c2d848c52bb0d2d442..a52332e69d937f15add57611813b69820e76c20e 100644 (file)
@@ -1,3 +1,33 @@
+2015-10-28  Chris Dumez  <cdumez@apple.com>
+
+        Assertion failure in WebCore::FrameLoader::stopLoading() running fast/events tests
+        https://bugs.webkit.org/show_bug.cgi?id=150624
+
+        Reviewed by Darin Adler.
+
+        After r191652, a form's target attribute can no longer refer to a frame's id,
+        only its name. This is because the frame's id no longer sets the Window name
+        when the frame's name attribute is missing. This caused a change in behavior
+        for the fast/events/form-iframe-target-before-load-crash*.html tests, which
+        exposed a pre-existing bug.
+
+        This patch updates the fast/events/form-iframe-target-before-load-crash*.html
+        tests so they keep testing the same thing as before r191652. It also adds a
+        variant to keep covering the newly exposed bug.
+
+        The issue was that the frame was no longer navigated when submitting the form
+        (due to the form's target not matching the frame name). Therefore, when
+        removing the iframe from the document, its navigation has not started yet and
+        DocumentLoadTiming::navigationStart() is not initialized yet when
+        FrameLoader::stopLoading() is called and we hit an assertion. This patch
+        replaces the assertion with an if check as we now know it can happen and we
+        have test coverage for it.
+
+        Test: fast/events/form-iframe-target-before-load-crash.html
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::stopLoading):
+
 2015-10-28  Brian Burg  <bburg@apple.com>
 
         Builtins generator should emit ENABLE(FEATURE) guards based on @conditional annotation
index 20b07cb16ceff37e51caa9fb40525e66376140f3..63a4772e44eb32dd97839542da516b42eb1d2e6f 100644 (file)
@@ -440,9 +440,8 @@ void FrameLoader::stopLoading(UnloadEventPolicy unloadEventPolicy)
                         // time into freed memory.
                         RefPtr<DocumentLoader> documentLoader = m_provisionalDocumentLoader;
                         m_pageDismissalEventBeingDispatched = PageDismissalType::Unload;
-                        if (documentLoader && !documentLoader->timing().unloadEventStart() && !documentLoader->timing().unloadEventEnd()) {
-                            DocumentLoadTiming& timing = documentLoader->timing();
-                            ASSERT(timing.navigationStart());
+                        if (documentLoader && documentLoader->timing().navigationStart() && !documentLoader->timing().unloadEventStart() && !documentLoader->timing().unloadEventEnd()) {
+                            auto& timing = documentLoader->timing();
                             timing.markUnloadEventStart();
                             m_frame.document()->domWindow()->dispatchEvent(unloadEvent, m_frame.document());
                             timing.markUnloadEventEnd();