Changeset 63680 in webkit


Ignore:
Timestamp:
Jul 19, 2010 11:46:08 AM (14 years ago)
Author:
ap@apple.com
Message:

Reviewed by Darin Adler.

​https://bugs.webkit.org/show_bug.cgi?id=40996
Progress event should not be fired during synchronous XMLHttpRequest

​https://bugs.webkit.org/show_bug.cgi?id=17502
Assertion failure when trying to restart a sync XMLHttpRequest as an async one from onreadystatechange

Tests: http/tests/xmlhttprequest/xmlhttprequest-sync-no-progress-events.html

http/tests/xmlhttprequest/xmlhttprequest-sync-vs-async-assertion-failure.html

  • xml/XMLHttpRequest.cpp: (WebCore::XMLHttpRequest::callReadyStateChangeListener): We now only dispatch readystatechange event for synchronous requests in states UNSENT, OPENED and DONE. I'm not sure what exactly the spec draft says about readystatechange for sync requests, but this seems to be the most logical and backwards compatible behavior. (WebCore::XMLHttpRequest::didReceiveData): Don't dispatch progress events for sync requests. Note that we already don't dispatch upload progress events for those.
Location:
trunk
Files:
5 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r63679 r63680  
     12010-07-19  Alexey Proskuryakov  <ap@apple.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=40996
     6        Progress event should not be fired during synchronous XMLHttpRequest
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=17502
     9        Assertion failure when trying to restart a sync XMLHttpRequest as an async one from onreadystatechange
     10
     11        * fast/xmlhttprequest/xmlhttprequest-recursive-sync-event-expected.txt: There is one less
     12        event dispatched now.
     13
     14        * http/tests/xmlhttprequest/infoOnProgressEvent-expected.txt:
     15        * http/tests/xmlhttprequest/infoOnProgressEvent.html:
     16        * http/tests/xmlhttprequest/xmlhttprequest-no-content-length-onProgress.html:
     17        Changed to use async requests, since sync ones no longer cause progress event dispatch.
     18
     19        * http/tests/xmlhttprequest/resources/access-control-basic-post-fail-non-simple.cgi: Added.
     20        A script that only allows simple cross-origin requests.
     21
     22        * http/tests/xmlhttprequest/xmlhttprequest-addEventListener-onProgress-expected.txt:
     23        * http/tests/xmlhttprequest/xmlhttprequest-addEventListener-onProgress.html:
     24        Removed synchronous case (incorrectly called asynchronous in test comments).
     25
     26        * http/tests/xmlhttprequest/xmlhttprequest-sync-no-progress-events-expected.txt: Added.
     27        * http/tests/xmlhttprequest/xmlhttprequest-sync-no-progress-events.html: Added.
     28        Added a test that logs progress and readystatechange events.
     29
     30        * http/tests/xmlhttprequest/xmlhttprequest-sync-vs-async-assertion-failure-expected.txt: Added.
     31        * http/tests/xmlhttprequest/xmlhttprequest-sync-vs-async-assertion-failure.html: Added.
     32        Aded a test for assertion failure. The problem no longer occurs since we don't fire events
     33        in intermediate states.
     34
    1352010-07-19  Csaba Osztrogonác  <ossy@webkit.org>
    236

  • trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-recursive-sync-event-expected.txt

    r60392 r63680  
    1 CONSOLE MESSAGE: line 0:
    21CONSOLE MESSAGE: line 0:
    32CONSOLE MESSAGE: line 0:

  • trunk/LayoutTests/http/tests/xmlhttprequest/infoOnProgressEvent-expected.txt

    r48701 r63680  
    11Test case for bug 13596: Implement .onprogress handler on XMLHttpRequest objects to support progressive download content length information
    22
    3 You should see type, bubble, cancelable, eventPhase, target and current target for synchronous and asynchronous cases
     3You should see type, bubble, cancelable, eventPhase, target and current target.
    44
    5 Synchronous case:
    65Type: progress
    76Bubble: false
    … …  
    1110Current target: [object XMLHttpRequest]
    1211
    13 Asynchronous case:
    14 Type: progress
    15 Bubble: false
    16 Cancelable: true
    17 EventPhase: 2
    18 Target: [object XMLHttpRequest]
    19 Current target: [object XMLHttpRequest]
    20 

  • trunk/LayoutTests/http/tests/xmlhttprequest/infoOnProgressEvent.html

    r32318 r63680  
    33<body>
    44<p> Test case for bug 13596: Implement .onprogress handler on XMLHttpRequest objects to support progressive download content length information </p>
    5 <p> You should see type, bubble, cancelable, eventPhase, target and current target for synchronous and asynchronous cases </p>
     5<p> You should see type, bubble, cancelable, eventPhase, target and current target.</p>
    66<script type="text/javascript">
    77function log (msg)
    … …  
    2424    log("Current target: " + e.currentTarget);
    2525    e.currentTarget.onprogress = null;
    26     if (shouldNotify && window.layoutTestController)
     26    if (window.layoutTestController)
    2727        layoutTestController.notifyDone();
    2828}
    … …  
    3939}
    4040
    41 log("Synchronous case:");
    42 
    43 // Test synchronous
    4441var req = new XMLHttpRequest();
    4542req.onprogress = onProgress;
    46 req.open("GET", "resources/1251.html", false);
     43req.open("GET", "resources/1251.html", true);
    4744req.send(null);
    48 
    49 insertNewLine();
    50 log("Asynchronous case:");
    51 
    52 // Test asynchronous
    53 var req2 = new XMLHttpRequest();
    54 req2.onprogress = onProgress;
    55 req2.open("GET", "resources/1251.html", true);
    56 shouldNotify = true;
    57 req2.send(null);
    5845</script>
    5946</body>

  • trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-addEventListener-onProgress-expected.txt

    r34320 r63680  
    33This test verify that addEventListener("progress", XXX, XXX) works as expected.
    44
    5 You should see PASSED 4 times.
     5You should see PASSED 2 times.
    66
    77PASSED (1)
    88PASSED (2)
    9 PASSED (3)
    10 PASSED (4)
    119

  • trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-addEventListener-onProgress.html

    r34320 r63680  
    66<p> Test case for Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=18655">18655</a>: [XHR] OnProgress needs more test case </p>
    77<p> This test verify that addEventListener("progress", XXX, XXX) works as expected. </p>
    8 <p> You should see PASSED 4 times. </p>
     8<p> You should see PASSED 2 times. </p>
    99<script type="text/javascript">
    1010var count = 1;
    … …  
    1818function onProgress(e) {
    1919    log("PASSED (" + count + ")");
    20     if (++count > 4 && window.layoutTestController)
     20    if (++count > 2 && window.layoutTestController)
    2121        layoutTestController.notifyDone();
    2222}
    … …  
    2626    layoutTestController.dumpAsText();
    2727}
    28 
    29 // Asynchronous case
    30 
    31 // Test for capture phase
    32 var req = new XMLHttpRequest();
    33 req.addEventListener("progress", onProgress, true);
    34 req.open("GET", "resources/1251.html", false);
    35 req.send(null);
    36 
    37 // Test for bubble phase
    38 var req2 = new XMLHttpRequest();
    39 req2.addEventListener("progress", onProgress, false);
    40 req2.open("GET", "resources/1251.html", false);
    41 req2.send(null);
    42 
    43 // Synchronous case
    4428
    4529// Test for capture phase

  • trunk/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-no-content-length-onProgress.html

    r34320 r63680  
    1111<p id="shouldNotBeCalled"> PASSED </p>
    1212<script type="text/javascript">
     13if (window.layoutTestController) {
     14    layoutTestController.dumpAsText();
     15    layoutTestController.waitUntilDone();
     16}
     17
     18var count = 0;
     19function checkDone() {
     20    if (++count == 2 && window.layoutTestController)
     21        layoutTestController.notifyDone();
     22}
     23
    1324function onProgressPassed(e) {
    1425    document.getElementById("shouldBeCalled").innerHTML = "PASSED";
    … …  
    1930}
    2031
    21 if (window.layoutTestController) {
    22     layoutTestController.dumpAsText();
    23     layoutTestController.waitUntilDone();
    24 }
    25 
    2632var req = new XMLHttpRequest();
    2733req.onprogress = onProgressPassed;
     34req.onload = checkDone;
    2835// Test that onProgress is called on a normal file
    29 req.open("GET", "resource/1251.html", false);
     36req.open("GET", "resource/1251.html", true);
    3037req.send(null);
    3138
    3239// If content length is not given, it should not be called
    33 req.onprogress = onProgressFailed;
    34 req.open("GET", "resources/noContentLength.asis", false);
    35 req.send(null);
    36 
    37 if (window.layoutTestController)
    38     layoutTestController.notifyDone();
     40var req2 = new XMLHttpRequest();
     41req2.onprogress = onProgressFailed;
     42req2.onload = checkDone;
     43req2.open("GET", "resources/noContentLength.asis", true);
     44req2.send(null);
    3945
    4046</script>

  • trunk/WebCore/ChangeLog

    r63672 r63680  
     12010-07-19  Alexey Proskuryakov  <ap@apple.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=40996
     6        Progress event should not be fired during synchronous XMLHttpRequest
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=17502
     9        Assertion failure when trying to restart a sync XMLHttpRequest as an async one from onreadystatechange
     10
     11        Tests: http/tests/xmlhttprequest/xmlhttprequest-sync-no-progress-events.html
     12               http/tests/xmlhttprequest/xmlhttprequest-sync-vs-async-assertion-failure.html
     13
     14        * xml/XMLHttpRequest.cpp:
     15        (WebCore::XMLHttpRequest::callReadyStateChangeListener): We now only dispatch readystatechange
     16        event for synchronous requests in states UNSENT, OPENED and DONE. I'm not sure what exactly
     17        the spec draft says about readystatechange for sync requests, but this seems to be the most
     18        logical and backwards compatible behavior.
     19        (WebCore::XMLHttpRequest::didReceiveData): Don't dispatch progress events for sync requests.
     20        Note that we already don't dispatch upload progress events for those.
     21
    1222010-07-19  Dan Bernstein  <mitz@apple.com>
    223

  • trunk/WebCore/xml/XMLHttpRequest.cpp

    r62828 r63680  
    284284#endif
    285285
    286     m_progressEventThrottle.dispatchEvent(XMLHttpRequestProgressEvent::create(eventNames().readystatechangeEvent), m_state == DONE ? FlushProgressEvent : DoNotFlushProgressEvent);
     286    if (m_async || (m_state <= OPENED || m_state == DONE))
     287        m_progressEventThrottle.dispatchEvent(XMLHttpRequestProgressEvent::create(eventNames().readystatechangeEvent), m_state == DONE ? FlushProgressEvent : DoNotFlushProgressEvent);
    287288
    288289#if ENABLE(INSPECTOR)
    … …  
    979980        m_receivedLength += len;
    980981
    981         bool lengthComputable = expectedLength && m_receivedLength <= expectedLength;
    982         m_progressEventThrottle.dispatchProgressEvent(lengthComputable, static_cast<unsigned>(m_receivedLength), static_cast<unsigned>(expectedLength));
     982        if (m_async) {
     983            bool lengthComputable = expectedLength && m_receivedLength <= expectedLength;
     984            m_progressEventThrottle.dispatchProgressEvent(lengthComputable, static_cast<unsigned>(m_receivedLength), static_cast<unsigned>(expectedLength));
     985        }
    983986
    984987        if (m_state != LOADING)
Note: See TracChangeset for help on using the changeset viewer.