Bug 1319744 - Ensure that progress events and corresponding LOADING readystatechanges fire as per spec. r=baku a=jcristau
authorThomas Wisniewski <wisniewskit@gmail.com>
Tue, 13 Dec 2016 15:23:01 -0500
changeset 480358 462a50df3d934aaff8483f9aa0f6a231d91f6cae
parent 480357 afe0d94be5473f11a871efad31bb638c17ca3e8f
child 480359 9a1b79873db7526a0c9f33511e224a617af7ca6b
push id44524
push usermartin.thomson@gmail.com
push dateWed, 08 Feb 2017 05:10:11 +0000
reviewersbaku, jcristau
bugs1319744
milestone52.0
Bug 1319744 - Ensure that progress events and corresponding LOADING readystatechanges fire as per spec. r=baku a=jcristau
dom/workers/test/bug1063538_worker.js
dom/workers/test/mochitest.ini
dom/xhr/XMLHttpRequestMainThread.cpp
testing/web-platform/meta/progress-events/tests/submissions/Samsung/firing-events-http-no-content-length.html.ini
testing/web-platform/tests/XMLHttpRequest/resources/xmlhttprequest-event-order.js
--- a/dom/workers/test/bug1063538_worker.js
+++ b/dom/workers/test/bug1063538_worker.js
@@ -10,16 +10,16 @@ var progressFired = false;
 xhr.onloadend = function(e) {
   postMessage({type: 'finish', progressFired: progressFired });
   self.close();
 };
 
 xhr.onprogress = function(e) {
   if (e.loaded > 0) {
     progressFired = true;
+    xhr.abort();
   }
-  xhr.abort();
 };
 
 onmessage = function(e) {
   xhr.open("GET", gJar, true);
   xhr.send();
 }
--- a/dom/workers/test/mochitest.ini
+++ b/dom/workers/test/mochitest.ini
@@ -102,16 +102,17 @@ support-files =
   fileapi_chromeScript.js
   importScripts_3rdParty_worker.js
   worker_bug1278777.js
   worker_setTimeoutWith0.js
   worker_bug1301094.js
   script_createFile.js
   worker_suspended.js
   window_suspended.html
+  !/dom/base/test/file_bug945152.jar
   !/dom/base/test/file_websocket_basic_wsh.py
   !/dom/base/test/file_websocket_hello_wsh.py
   !/dom/base/test/file_websocket_http_resource.txt
   !/dom/base/test/file_websocket_permessage_deflate_disabled_wsh.py
   !/dom/base/test/file_websocket_permessage_deflate_params_wsh.py
   !/dom/base/test/file_websocket_permessage_deflate_rejected_wsh.py
   !/dom/base/test/file_websocket_permessage_deflate_wsh.py
   !/dom/base/test/file_websocket_wsh.py
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -674,32 +674,32 @@ XMLHttpRequestMainThread::CreateResponse
 
 void
 XMLHttpRequestMainThread::CreatePartialBlob(ErrorResult& aRv)
 {
   if (mDOMBlob) {
     // Use progress info to determine whether load is complete, but use
     // mDataAvailable to ensure a slice is created based on the uncompressed
     // data count.
-    if (mLoadTotal == mLoadTransferred) {
+    if (mState == State::done) {
       mResponseBlob = mDOMBlob;
     } else {
       mResponseBlob = mDOMBlob->CreateSlice(0, mDataAvailable,
                                             EmptyString(), aRv);
     }
     return;
   }
 
   // mBlobSet can be null if the request has been canceled
   if (!mBlobSet) {
     return;
   }
 
   nsAutoCString contentType;
-  if (mLoadTotal == mLoadTransferred) {
+  if (mState == State::done) {
     mChannel->GetContentType(contentType);
   }
 
   nsTArray<RefPtr<BlobImpl>> subImpls(mBlobSet->GetBlobImpls());
   RefPtr<BlobImpl> blobImpl =
     MultipartBlobImpl::Create(Move(subImpls),
                               NS_ConvertASCIItoUTF16(contentType),
                               aRv);
@@ -1735,17 +1735,25 @@ XMLHttpRequestMainThread::OnDataAvailabl
     }
 
     ChangeState(State::loading);
     return request->Cancel(NS_OK);
   }
 
   mDataAvailable += totalRead;
 
-  ChangeState(State::loading);
+  // Fire the first progress event/loading state change
+  if (mState != State::loading) {
+    ChangeState(State::loading);
+    if (!mFlagSynchronous) {
+      DispatchProgressEvent(this, ProgressEventType::progress,
+                            mLoadTransferred, mLoadTotal);
+    }
+    mProgressSinceLastProgressEvent = false;
+  }
 
   if (!mFlagSynchronous && !mProgressTimerIsActive) {
     StartProgressEventTimer();
   }
 
   return NS_OK;
 }
 
@@ -2043,46 +2051,36 @@ XMLHttpRequestMainThread::OnStopRequest(
   }
 
   mXMLParserStreamListener = nullptr;
   mContext = nullptr;
 
   bool waitingForBlobCreation = false;
 
   if (NS_SUCCEEDED(status) &&
-      (mResponseType == XMLHttpRequestResponseType::_empty ||
-       mResponseType == XMLHttpRequestResponseType::Text)) {
-    mLoadTotal = mResponseBody.Length();
-  } else if (NS_SUCCEEDED(status) &&
       (mResponseType == XMLHttpRequestResponseType::Blob ||
        mResponseType == XMLHttpRequestResponseType::Moz_blob)) {
     ErrorResult rv;
     if (!mDOMBlob) {
       CreateDOMBlob(request);
     }
     if (mDOMBlob) {
       mResponseBlob = mDOMBlob;
       mDOMBlob = nullptr;
-
-      mLoadTotal = mResponseBlob->GetSize(rv);
-      if (NS_WARN_IF(rv.Failed())) {
-        status = rv.StealNSResult();
-      }
     } else {
       // Smaller files may be written in cache map instead of separate files.
       // Also, no-store response cannot be written in persistent cache.
       nsAutoCString contentType;
       mChannel->GetContentType(contentType);
 
       if (mResponseType == XMLHttpRequestResponseType::Blob) {
         // mBlobStorage can be null if the channel is non-file non-cacheable
         // and if the response length is zero.
         MaybeCreateBlobStorage();
-        mLoadTotal =
-          mBlobStorage->GetBlobWhenReady(GetOwner(), contentType, this);
+        mBlobStorage->GetBlobWhenReady(GetOwner(), contentType, this);
         waitingForBlobCreation = true;
       } else {
         // mBlobSet can be null if the channel is non-file non-cacheable
         // and if the response length is zero.
         if (!mBlobSet) {
           mBlobSet = new BlobSet();
         }
 
@@ -2093,33 +2091,28 @@ XMLHttpRequestMainThread::OnStopRequest(
                                     rv);
         mBlobSet = nullptr;
 
         if (NS_WARN_IF(rv.Failed())) {
           return rv.StealNSResult();
         }
 
         mResponseBlob = Blob::Create(GetOwner(), blobImpl);
-        mLoadTotal = mResponseBlob->GetSize(rv);
-        if (NS_WARN_IF(rv.Failed())) {
-          status = rv.StealNSResult();
-        }
       }
     }
 
     NS_ASSERTION(mResponseBody.IsEmpty(), "mResponseBody should be empty");
     NS_ASSERTION(mResponseText.IsEmpty(), "mResponseText should be empty");
   } else if (NS_SUCCEEDED(status) &&
              ((mResponseType == XMLHttpRequestResponseType::Arraybuffer &&
                !mIsMappedArrayBuffer) ||
               mResponseType == XMLHttpRequestResponseType::Moz_chunked_arraybuffer)) {
     // set the capacity down to the actual length, to realloc back
     // down to the actual size
-    mLoadTotal = mArrayBufferBuilder.length();
-    if (!mArrayBufferBuilder.setCapacity(mLoadTotal)) {
+    if (!mArrayBufferBuilder.setCapacity(mArrayBufferBuilder.length())) {
       // this should never happen!
       status = NS_ERROR_UNEXPECTED;
     }
   }
 
   nsCOMPtr<nsIChannel> channel(do_QueryInterface(request));
   NS_ENSURE_TRUE(channel, NS_ERROR_UNEXPECTED);
 
@@ -2213,22 +2206,16 @@ XMLHttpRequestMainThread::ChangeStateToD
              "ChangeStateToDone() called before async HTML parsing is done.");
 
   mFlagSend = false;
 
   if (mTimeoutTimer) {
     mTimeoutTimer->Cancel();
   }
 
-  if (mLoadTransferred) {
-    mLoadTotal = mLoadTransferred;
-  } else {
-    mLoadTotal = -1;
-  }
-
   // Per spec, fire the last download progress event, if any,
   // before readystatechange=4/done. (Note that 0-sized responses
   // will have not sent a progress event yet, so one must be sent here).
   if (!mFlagSynchronous &&
       (!mLoadTransferred || mProgressSinceLastProgressEvent)) {
     DispatchProgressEvent(this, ProgressEventType::progress,
                           mLoadTransferred, mLoadTotal);
     mProgressSinceLastProgressEvent = false;
@@ -3357,19 +3344,18 @@ XMLHttpRequestMainThread::OnProgress(nsI
     }
     mUploadTransferred = loaded;
     mProgressSinceLastProgressEvent = true;
 
     if (!mFlagSynchronous && !mProgressTimerIsActive) {
       StartProgressEventTimer();
     }
   } else {
-    mLoadTotal = lengthComputable ? aProgressMax : -1;
+    mLoadTotal = aProgressMax;
     mLoadTransferred = aProgress;
-  
     // OnDataAvailable() handles mProgressSinceLastProgressEvent
     // for the download phase.
   }
 
   if (mProgressEventSink) {
     mProgressEventSink->OnProgress(aRequest, aContext, aProgress,
                                    aProgressMax);
   }
@@ -3576,16 +3562,17 @@ XMLHttpRequestMainThread::HandleProgress
   }
 
   if (InUploadPhase()) {
     if (mUpload && !mUploadComplete) {
       DispatchProgressEvent(mUpload, ProgressEventType::progress,
                             mUploadTransferred, mUploadTotal);
     }
   } else {
+    FireReadystatechangeEvent();
     DispatchProgressEvent(this, ProgressEventType::progress,
                           mLoadTransferred, mLoadTotal);
   }
 
   mProgressSinceLastProgressEvent = false;
 
   StartProgressEventTimer();
 }
@@ -3752,22 +3739,16 @@ XMLHttpRequestMainThread::BlobStoreCompl
     return;
   }
 
   MOZ_ASSERT(mState != State::done);
 
   mResponseBlob = aBlob;
   mBlobStorage = nullptr;
 
-  ErrorResult rv;
-  mLoadTotal = mResponseBlob->GetSize(rv);
-  if (NS_WARN_IF(rv.Failed())) {
-    rv.SuppressException();
-  }
-
   ChangeStateToDone();
 }
 
 // nsXMLHttpRequestXPCOMifier implementation
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsXMLHttpRequestXPCOMifier)
   NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
   NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
   NS_INTERFACE_MAP_ENTRY(nsIChannelEventSink)
deleted file mode 100644
--- a/testing/web-platform/meta/progress-events/tests/submissions/Samsung/firing-events-http-no-content-length.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[firing-events-http-no-content-length.html]
-  type: testharness
-  [ProgressEvent: firing events for HTTP with no Content-Length]
-    expected: FAIL
-
--- a/testing/web-platform/tests/XMLHttpRequest/resources/xmlhttprequest-event-order.js
+++ b/testing/web-platform/tests/XMLHttpRequest/resources/xmlhttprequest-event-order.js
@@ -17,63 +17,66 @@
     if ("upload" in xhr) {
       for(var i=0; i<events.length; ++i) {
         xhr.upload.addEventListener(events[i], record_xhr_event);
       }
     }
   }
 
   function getNextEvent(arr) {
-    var eventStr = arr.shift();
+    var event = { str: arr.shift() };
 
     // we can only handle strings, numbers (readystates) and undefined
-    if (eventStr === undefined) {
+    if (event.str === undefined) {
       return event;
     }
-    if (typeof eventStr !== "string") {
-      if (Number.isInteger(eventStr)) {
-        eventStr = "readystatechange(" + eventStr + ")";
+
+    if (typeof event.str !== "string") {
+      if (Number.isInteger(event.str)) {
+        event.state = event.str;
+        event.str = "readystatechange(" + event.str + ")";
       } else {
-        throw "Test error: unexpected event type " + eventStr;
+        throw "Test error: unexpected event type " + event.str;
       }
     }
 
     // parse out the general type, loaded and total values
-    var type = eventStr.type = eventStr.split("(")[0].split(".").pop();
-    eventStr.mayFollowOptionalProgressEvents = type == "progress" ||
-      type == "load" || type == "abort" || type == "error";
-    var loadedAndTotal = eventStr.match(/\((\d)+,(\d)+/);
+    var type = event.type = event.str.split("(")[0].split(".").pop();
+    var loadedAndTotal = event.str.match(/.*\((\d+),(\d+),(true|false)\)/);
     if (loadedAndTotal) {
-      eventStr.loaded = parseInt(loadedAndTotal[0]);
-      eventStr.total = parseInt(loadedAndTotal[1]);
+      event.loaded = parseInt(loadedAndTotal[1]);
+      event.total = parseInt(loadedAndTotal[2]);
+      event.lengthComputable = loadedAndTotal[3] == "true";
     }
 
-    return eventStr;
+    return event;
   }
 
   global.assert_xhr_event_order_matches = function(expected) {
     var recorded = recorded_xhr_events;
     var lastRecordedLoaded = -1;
-
     while(expected.length && recorded.length) {
       var currentExpected = getNextEvent(expected),
           currentRecorded = getNextEvent(recorded);
 
-      // skip to the last progress event if we've hit one
-      while (recorded.length && currentRecorded.type == "progress") {
-        assert_greater(currentRecorded.loaded, lastRecordedLoaded,
-                       "progress event 'loaded' values must only increase");
+      // skip to the last progress event if we've hit one (note the next
+      // event after a progress event should be a LOADING readystatechange,
+      // if there are multiple progress events in a row).
+      while (recorded.length && currentRecorded.type == "progress" &&
+             parseInt(recorded) === 3) {
+        assert_greater_than(currentRecorded.loaded, lastRecordedLoaded,
+                            "progress event 'loaded' values must only increase");
         lastRecordedLoaded = currentRecorded.loaded;
-        currentRecorded = getNextEvent(recorded);
       }
-      if (currentRecorded.type == "loadstart") {
+      if (currentRecorded.type == "loadend") {
+        recordedProgressCount = 0;
         lastRecordedLoaded = -1;
       }
 
-      assert_equals(currentRecorded, currentExpected);
+      assert_equals(currentRecorded.str, currentExpected.str);
     }
     if (recorded.length) {
       throw "\nUnexpected extra events: " + recorded.join(", ");
     }
     if (expected.length) {
       throw "\nExpected more events: " + expected.join(", ");
     }
   }