LayoutTests:

        Reviewed by Maciej

        Added test to cover bug http://bugs.webkit.org/show_bug.cgi?id=11450
        -Confirm there is no assert when nuking a frame with an active XMLHttpRequest load
        -Confirm the correct state-change notifications for the request are dispatched

        * http/tests/xmlhttprequest/frame-load-cancelled-abort-expected.txt: Added.
        * http/tests/xmlhttprequest/frame-load-cancelled-abort.html: Added.
        * http/tests/xmlhttprequest/resources/endlessxml.php: Added.
        * http/tests/xmlhttprequest/resources/frame-load-to-cancel.html: Added.

WebCore:

        Reviewed by Maciej

        If the request has already been aborted, bail out of didFinishLoading()
        immediately.  This prevents state change notifications that aren't
        needed.

        * xml/xmlhttprequest.cpp:
        (WebCore::XMLHttpRequest::didFinishLoading):

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 1d9e6096fda26cda7af79abe852fdbf5e7755458..2d9a41793b3d2eda240c5ec3a94e741a7d7c73f9 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2006-11-08  Brady Eidson <beidson@apple.com>
+
+        Reviewed by Maciej
+
+        Added test to cover bug http://bugs.webkit.org/show_bug.cgi?id=11450
+        -Confirm there is no assert when nuking a frame with an active XMLHttpRequest load
+        -Confirm the correct state-change notifications for the request are dispatched
+
+        * http/tests/xmlhttprequest/frame-load-cancelled-abort-expected.txt: Added.
+        * http/tests/xmlhttprequest/frame-load-cancelled-abort.html: Added.
+        * http/tests/xmlhttprequest/resources/endlessxml.php: Added.
+        * http/tests/xmlhttprequest/resources/frame-load-to-cancel.html: Added.
+
 2006-11-08  Darin Adler  <darin@apple.com>
 
         Reviewed by Geoff.

diff --git a/LayoutTests/http/tests/xmlhttprequest/frame-load-cancelled-abort-expected.txt b/LayoutTests/http/tests/xmlhttprequest/frame-load-cancelled-abort-expected.txt
new file mode 100644 (file)
index 0000000..ddda2fa
--- /dev/null
+++ b/LayoutTests/http/tests/xmlhttprequest/frame-load-cancelled-abort-expected.txt
@@ -0,0 +1,50 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 53x18
+          text run at (0,0) width 53: "Test for "
+        RenderInline {A} at (0,0) size 68x18 [color=#0000EE]
+          RenderText {#text} at (53,0) size 68x18
+            text run at (53,0) width 68: "bug 11450"
+        RenderText {#text} at (121,0) size 757x36
+          text run at (121,0) width 636: " REGRESSION: XMLHttpRequest::didFinishLoading() should immediately return if the request has"
+          text run at (0,18) width 131: "already been aborted"
+      RenderBlock {P} at (0,52) size 784x36
+        RenderText {#text} at (0,0) size 601x18
+          text run at (0,0) width 601: "If a frame has an active XMLHttpRequest that is still receiving data, and that frame is destroyed"
+        RenderBR {BR} at (601,14) size 0x0
+        RenderText {#text} at (0,18) size 659x18
+          text run at (0,18) width 659: "(or its load is otherwise stopped) the XMLHttpRequest gets an abort call then a didFinishedLoading call."
+      RenderBlock {P} at (0,104) size 784x18
+        RenderText {#text} at (0,0) size 738x18
+          text run at (0,0) width 621: "That didFinishedLoading() call should immediately exit and not update the object's state or do any "
+          text run at (621,0) width 117: "other notifications."
+      RenderBlock {UL} at (0,138) size 784x90
+        RenderListItem {LI} at (40,0) size 744x18
+          RenderListMarker at (-17,0) size 7x18
+          RenderText {#text} at (0,0) size 175x18
+            text run at (0,0) width 175: "Loading subframe to cancel"
+        RenderListItem {LI} at (40,18) size 744x18
+          RenderListMarker at (-17,0) size 7x18
+          RenderText {#text} at (0,0) size 92x18
+            text run at (0,0) width 92: "Ready State: 1"
+        RenderListItem {LI} at (40,36) size 744x18
+          RenderListMarker at (-17,0) size 7x18
+          RenderText {#text} at (0,0) size 558x18
+            text run at (0,0) width 182: "Body of subframe is loaded. "
+            text run at (182,0) width 261: "XMLHttpRequest should be in progress. "
+            text run at (443,0) width 115: "Nuking the iframe"
+        RenderListItem {LI} at (40,54) size 744x18
+          RenderListMarker at (-17,0) size 7x18
+          RenderText {#text} at (0,0) size 103x18
+            text run at (0,0) width 103: "Iframe unloaded"
+        RenderListItem {LI} at (40,72) size 744x18
+          RenderListMarker at (-17,0) size 7x18
+          RenderText {#text} at (0,0) size 100x18
+            text run at (0,0) width 100: "Iframe removed"
+      RenderBlock {DIV} at (0,244) size 784x18
+        RenderText {#text} at (0,0) size 88x18
+          text run at (0,0) width 88: "Test complete"

diff --git a/LayoutTests/http/tests/xmlhttprequest/frame-load-cancelled-abort.html b/LayoutTests/http/tests/xmlhttprequest/frame-load-cancelled-abort.html
new file mode 100644 (file)
index 0000000..63c35bd
--- /dev/null
+++ b/LayoutTests/http/tests/xmlhttprequest/frame-load-cancelled-abort.html
@@ -0,0 +1,54 @@
+<html>
+<head>
+<title>Test XMLHttpRequest frame load abort causes unexpected notification activity</title>
+<meta http-equiv="content-type" content="text/html;charset=utf-8">
+</head>
+<body onload="startTest()">
+<p>Test for <a href="http://bugzilla.opendarwin.org/show_bug.cgi?id=11450">bug 11450</a>
+REGRESSION: XMLHttpRequest::didFinishLoading() should immediately return if the request has already been aborted</p>
+<p>If a frame has an active XMLHttpRequest that is still receiving data, and that frame is destroyed<br>
+(or its load is otherwise stopped) the XMLHttpRequest gets an abort call then a didFinishedLoading call.</p>
+<p>That didFinishedLoading() call should immediately exit and not update the object's state or do any 
+other notifications.</p>
+
+<script>
+    var console_messages = document.createElement("ul");
+    document.body.appendChild(console_messages);
+    
+    var framediv;
+    
+    function startTest()
+    {
+        if (window.layoutTestController)
+            layoutTestController.waitUntilDone();
+        log("Loading subframe to cancel");
+        framediv = document.getElementById('framediv');
+        framediv.innerHTML = '<iframe src="resources/frame-load-to-cancel.html" width=0 height=0 border=0></iframe>';
+    }
+    
+    function subframeLoaded()
+    {
+        log("Body of subframe is loaded.  XMLHttpRequest should be in progress.  Nuking the iframe");
+        framediv.innerHTML = 'Test complete';
+        log("Iframe removed");
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+    
+    function dumpRequestStatus(req) {
+       try { log ("Ready State: " + req.readyState); } catch (ex) { log("Exception getting status: " + ex.message); }
+    }
+    
+    function log(message)
+    {
+        var item = document.createElement("li");
+        item.appendChild(document.createTextNode(message));
+        console_messages.appendChild(item);
+    }
+    
+</script>
+<div id="framediv" name="framediv" style="visibility:hidden">
+iFrame will go here
+</div>
+</body>
+</html>

diff --git a/LayoutTests/http/tests/xmlhttprequest/resources/endlessxml.php b/LayoutTests/http/tests/xmlhttprequest/resources/endlessxml.php
new file mode 100644 (file)
index 0000000..e84de7f
--- /dev/null
+++ b/LayoutTests/http/tests/xmlhttprequest/resources/endlessxml.php
@@ -0,0 +1,3 @@
+<?php
+sleep(10);
+?>
\ No newline at end of file

diff --git a/LayoutTests/http/tests/xmlhttprequest/resources/frame-load-to-cancel.html b/LayoutTests/http/tests/xmlhttprequest/resources/frame-load-to-cancel.html
new file mode 100644 (file)
index 0000000..4e65b6b
--- /dev/null
+++ b/LayoutTests/http/tests/xmlhttprequest/resources/frame-load-to-cancel.html
@@ -0,0 +1,36 @@
+    <html>
+<head>
+<script>
+var rq;
+
+function loadXMLDoc()
+{
+    url = 'endlessxml.php';
+
+    rq = false;
+    
+    try {
+      if (window.XMLHttpRequest) {
+        rq = new XMLHttpRequest();
+        rq.overrideMimeType('text/xml');
+        rq.onreadystatechange = readyStateChanged
+        rq.open('GET', url, true);
+        rq.send();
+      }
+    } catch (ex) {
+        rq = false;
+        alert(ex);
+    }
+}
+
+function readyStateChanged()
+{
+    window.parent.dumpRequestStatus(rq);
+}
+
+</script>
+</head>
+<body onload="loadXMLDoc(); window.parent.subframeLoaded();" onunload="window.parent.log('Iframe unloaded');">
+<p>This should have an XMLHttpRequest going on inside it</p>
+</body>
+</html>
\ No newline at end of file

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 5f08a61d91015ec059afc73ca5bc8e8fefeb7fb9..25dcfe811e8746add2e1ae99845ed4d1c7d565f4 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,14 @@
+2006-11-08  Brady Eidson <beidson@apple.com>
+
+        Reviewed by Maciej
+
+        If the request has already been aborted, bail out of didFinishLoading()
+        immediately.  This prevents state change notifications that aren't
+        needed.
+
+        * xml/xmlhttprequest.cpp:
+        (WebCore::XMLHttpRequest::didFinishLoading):
+
 2006-11-08  Darin Adler  <darin@apple.com>
 
         - another attempt to fix Qt build

diff --git a/WebCore/xml/xmlhttprequest.cpp b/WebCore/xml/xmlhttprequest.cpp
index f9a14abf706f2e5826eb4334990dcdcafa5416e5..2876305e08ec95ed8971bc60b800dbff0e1e8089 100644 (file)
--- a/WebCore/xml/xmlhttprequest.cpp
+++ b/WebCore/xml/xmlhttprequest.cpp
@@ -485,7 +485,10 @@ void XMLHttpRequest::processSyncLoadResults(const Vector<char>& data, const Reso
 
 void XMLHttpRequest::didFinishLoading(ResourceHandle* loader)
 {
-    ASSERT(m_aborted || loader == m_loader);
+    if (m_aborted)
+        return;
+        
+    ASSERT(loader == m_loader);
 
     if (m_state < Sent)
         changeState(Sent);