Bug 1007020 - Also send progress information when connection is LOAD_BACKGROUND. r=bz
authorDragana Damjanovic <dd.mozilla@gmail.com>
Fri, 01 Aug 2014 02:10:00 -0400
changeset 219328 572492b2e400a6655e442cf0804a2aa32c9e4e5c
parent 219327 b50533a4472c1e98223c3544f88c4cf4198ae984
child 219329 4deedde7e4e075cd89f5bf6f4232b1cca1eb186f
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1007020
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1007020 - Also send progress information when connection is LOAD_BACKGROUND. r=bz
content/base/test/chrome/test_bug800386.xul
modules/libjar/nsJARChannel.cpp
netwerk/base/public/nsIRequest.idl
netwerk/base/src/nsBaseChannel.cpp
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelParent.cpp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/wyciwyg/WyciwygChannelChild.cpp
netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
--- a/content/base/test/chrome/test_bug800386.xul
+++ b/content/base/test/chrome/test_bug800386.xul
@@ -21,27 +21,29 @@ https://bugzilla.mozilla.org/show_bug.cg
   Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 
   SimpleTest.waitForExplicitFinish();
 
   var triedForwarding = false;
   var forwardFailed = false;
 
   var xhr = new XMLHttpRequest;
+  var xhr2 = new XMLHttpRequest;
+
   var eventSink = xhr.getInterface(Components.interfaces.nsIProgressEventSink);
   isnot(eventSink, null, "Should get event sink directly!");
 
   // Now jump through some hoops to get us a getInterface call from C++
 
   var requestor = {
     getInterface: function(aIID) {
       if (aIID.equals(Components.interfaces.nsIProgressEventSink)) {
         triedForwarding = true;
         try {
-          return xhr.getInterface(aIID);
+          return xhr2.getInterface(aIID);
         } catch (e) {
           forwardFailed = true;
         }
       }
       throw Components.results.NS_ERROR_NO_INTERFACE;
     },
 
     QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsISupports,
--- a/modules/libjar/nsJARChannel.cpp
+++ b/modules/libjar/nsJARChannel.cpp
@@ -465,20 +465,16 @@ nsJARChannel::NotifyError(nsresult aErro
 }
 
 void
 nsJARChannel::FireOnProgress(uint64_t aProgress)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mProgressSink);
 
-  if (mLoadFlags & LOAD_BACKGROUND) {
-    return;
-  }
-
   mProgressSink->OnProgress(this, nullptr, aProgress,
                             uint64_t(mContentLength));
 }
 
 //-----------------------------------------------------------------------------
 // nsIRequest
 //-----------------------------------------------------------------------------
 
--- a/netwerk/base/public/nsIRequest.idl
+++ b/netwerk/base/public/nsIRequest.idl
@@ -112,21 +112,22 @@ interface nsIRequest : nsISupports
      * Listed below are the various load flags which may be or'd together.
      */
 
     /**
      * No special load flags:
      */
     const unsigned long LOAD_NORMAL = 0;
 
-    /** 
-     * Don't deliver status notifications to the nsIProgressEventSink, or keep 
-     * this load from completing the nsILoadGroup it may belong to.
+    /**
+     * Do not deliver status notifications to the nsIProgressEventSink and
+     * do not block the loadgroup from completing (should this load belong to one).
+     * Note: Progress notifications will still be delivered.
      */
-    const unsigned long LOAD_BACKGROUND = 1 << 0; 
+    const unsigned long LOAD_BACKGROUND = 1 << 0;
 
     /**************************************************************************
      * The following flags control the flow of data into the cache.
      */
 
     /**
      *  This flag prevents loading of the request with an HTTP pipeline.
      *  Generally this is because the resource is expected to take a
--- a/netwerk/base/src/nsBaseChannel.cpp
+++ b/netwerk/base/src/nsBaseChannel.cpp
@@ -636,37 +636,44 @@ nsBaseChannel::AsyncOpen(nsIStreamListen
 // nsBaseChannel::nsITransportEventSink
 
 NS_IMETHODIMP
 nsBaseChannel::OnTransportStatus(nsITransport *transport, nsresult status,
                                  uint64_t progress, uint64_t progressMax)
 {
   // In some cases, we may wish to suppress transport-layer status events.
 
-  if (!mPump || NS_FAILED(mStatus) || HasLoadFlag(LOAD_BACKGROUND))
+  if (!mPump || NS_FAILED(mStatus)) {
     return NS_OK;
+  }
 
   SUSPEND_PUMP_FOR_SCOPE();
 
   // Lazily fetch mProgressSink
   if (!mProgressSink) {
-    if (mQueriedProgressSink)
+    if (mQueriedProgressSink) {
       return NS_OK;
+    }
     GetCallback(mProgressSink);
     mQueriedProgressSink = true;
-    if (!mProgressSink)
+    if (!mProgressSink) {
       return NS_OK;
+    }
   }
 
-  nsAutoString statusArg;
-  if (GetStatusArg(status, statusArg))
-    mProgressSink->OnStatus(this, mListenerContext, status, statusArg.get());
+  if (!HasLoadFlag(LOAD_BACKGROUND)) {
+    nsAutoString statusArg;
+    if (GetStatusArg(status, statusArg)) {
+      mProgressSink->OnStatus(this, mListenerContext, status, statusArg.get());
+    }
+  }
 
-  if (progress)
+  if (progress) {
     mProgressSink->OnProgress(this, mListenerContext, progress, progressMax);
+  }
 
   return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsBaseChannel::nsIInterfaceRequestor
 
 NS_IMETHODIMP
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -430,25 +430,28 @@ HttpChannelChild::OnTransportAndData(con
     SendDivertOnDataAvailable(data, offset, count);
     return;
   }
 
   if (mCanceled)
     return;
 
   // cache the progress sink so we don't have to query for it each time.
-  if (!mProgressSink)
+  if (!mProgressSink) {
     GetCallback(mProgressSink);
+  }
 
   // Hold queue lock throughout all three calls, else we might process a later
   // necko msg in between them.
   AutoEventEnqueuer ensureSerialDispatch(mEventQ);
 
-  // block status/progress after Cancel or OnStopRequest has been called,
+  // Block status/progress after Cancel or OnStopRequest has been called,
   // or if channel has LOAD_BACKGROUND set.
+  // Note: Progress events will be received directly in RecvOnProgress if
+  // LOAD_BACKGROUND is set.
   // - JDUELL: may not need mStatus/mIsPending checks, given this is always called
   //   during OnDataAvailable, and we've already checked mCanceled.  Code
   //   dupe'd from nsHttpChannel
   if (mProgressSink && NS_SUCCEEDED(mStatus) && mIsPending &&
       !(mLoadFlags & LOAD_BACKGROUND))
   {
     // OnStatus
     //
@@ -599,25 +602,24 @@ HttpChannelChild::OnProgress(const uint6
 {
   LOG(("HttpChannelChild::OnProgress [this=%p progress=%llu/%llu]\n",
        this, progress, progressMax));
 
   if (mCanceled)
     return;
 
   // cache the progress sink so we don't have to query for it each time.
-  if (!mProgressSink)
+  if (!mProgressSink) {
     GetCallback(mProgressSink);
+  }
 
   AutoEventEnqueuer ensureSerialDispatch(mEventQ);
 
-  // block socket status event after Cancel or OnStopRequest has been called,
-  // or if channel has LOAD_BACKGROUND set
-  if (mProgressSink && NS_SUCCEEDED(mStatus) && mIsPending &&
-      !(mLoadFlags & LOAD_BACKGROUND))
+  // Block socket status event after Cancel or OnStopRequest has been called.
+  if (mProgressSink && NS_SUCCEEDED(mStatus) && mIsPending)
   {
     if (progress > 0) {
       MOZ_ASSERT(progress <= progressMax, "unexpected progress values");
       mProgressSink->OnProgress(this, nullptr, progress, progressMax);
     }
   }
 }
 
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -700,19 +700,19 @@ HttpChannelParent::OnProgress(nsIRequest
   // OnStatus has always just set mStoredStatus. If it indicates this precedes
   // OnDataAvailable, store and ODA will send to child.
   if (mStoredStatus == NS_NET_STATUS_RECEIVING_FROM ||
       mStoredStatus == NS_NET_STATUS_READING)
   {
     mStoredProgress = aProgress;
     mStoredProgressMax = aProgressMax;
   } else {
-    // Send to child now.  The only case I've observed that this handles (i.e.
-    // non-ODA status with progress > 0) is data upload progress notification
-    // (status == NS_NET_STATUS_SENDING_TO)
+    // Send OnProgress events to the child for data upload progress notifications
+    // (i.e. status == NS_NET_STATUS_SENDING_TO) or if the channel has
+    // LOAD_BACKGROUND set.
     if (mIPCClosed || !SendOnProgress(aProgress, aProgressMax))
       return NS_ERROR_UNEXPECTED;
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -5422,41 +5422,40 @@ nsHttpChannel::OnTransportStatus(nsITran
             do_QueryInterface(trans);
         if (socketTransport) {
             socketTransport->GetSelfAddr(&mSelfAddr);
             socketTransport->GetPeerAddr(&mPeerAddr);
         }
     }
 
     // block socket status event after Cancel or OnStopRequest has been called.
-    if (mProgressSink && NS_SUCCEEDED(mStatus) && mIsPending && !(mLoadFlags & LOAD_BACKGROUND)) {
-        LOG(("sending status notification [this=%p status=%x progress=%llu/%llu]\n",
+    if (mProgressSink && NS_SUCCEEDED(mStatus) && mIsPending) {
+        LOG(("sending progress%s notification [this=%p status=%x"
+             " progress=%llu/%llu]\n",
+            (mLoadFlags & LOAD_BACKGROUND)? "" : " and status",
             this, status, progress, progressMax));
 
-        nsAutoCString host;
-        mURI->GetHost(host);
-        mProgressSink->OnStatus(this, nullptr, status,
-                                NS_ConvertUTF8toUTF16(host).get());
+        if (!(mLoadFlags & LOAD_BACKGROUND)) {
+            nsAutoCString host;
+            mURI->GetHost(host);
+            mProgressSink->OnStatus(this, nullptr, status,
+                                    NS_ConvertUTF8toUTF16(host).get());
+        }
 
         if (progress > 0) {
             MOZ_ASSERT(progress <= progressMax, "unexpected progress values");
             // Try to get mProgressSink if it was nulled out during OnStatus.
             if (!mProgressSink) {
                 GetCallback(mProgressSink);
             }
             if (mProgressSink) {
                 mProgressSink->OnProgress(this, nullptr, progress, progressMax);
             }
         }
     }
-#ifdef DEBUG
-    else
-        LOG(("skipping status notification [this=%p sink=%p pending=%u background=%x]\n",
-            this, mProgressSink.get(), mIsPending, (mLoadFlags & LOAD_BACKGROUND)));
-#endif
 
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsICacheInfoChannel
 //-----------------------------------------------------------------------------
 
--- a/netwerk/protocol/wyciwyg/WyciwygChannelChild.cpp
+++ b/netwerk/protocol/wyciwyg/WyciwygChannelChild.cpp
@@ -208,19 +208,20 @@ WyciwygChannelChild::OnDataAvailable(con
 
   AutoEventEnqueuer ensureSerialDispatch(mEventQ);
   
   rv = mListener->OnDataAvailable(this, mListenerContext,
                                   stringStream, offset, data.Length());
   if (NS_FAILED(rv))
     Cancel(rv);
 
-  if (mProgressSink && NS_SUCCEEDED(rv) && !(mLoadFlags & LOAD_BACKGROUND))
+  if (mProgressSink && NS_SUCCEEDED(rv)) {
     mProgressSink->OnProgress(this, nullptr, offset + data.Length(),
                               uint64_t(mContentLength));
+  }
 }
 
 class WyciwygStopRequestEvent : public ChannelEvent
 {
 public:
   WyciwygStopRequestEvent(WyciwygChannelChild* child,
                           const nsresult& statusCode)
   : mChild(child), mStatusCode(statusCode) {}
--- a/netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
+++ b/netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
@@ -694,19 +694,20 @@ nsWyciwygChannel::OnDataAvailable(nsIReq
   LOG(("nsWyciwygChannel::OnDataAvailable [this=%p request=%x offset=%llu count=%u]\n",
       this, request, offset, count));
 
   nsresult rv;
   
   rv = mListener->OnDataAvailable(this, mListenerContext, input, offset, count);
 
   // XXX handle 64-bit stuff for real
-  if (mProgressSink && NS_SUCCEEDED(rv) && !(mLoadFlags & LOAD_BACKGROUND))
+  if (mProgressSink && NS_SUCCEEDED(rv)) {
     mProgressSink->OnProgress(this, nullptr, offset + count,
                               uint64_t(mContentLength));
+  }
 
   return rv; // let the pump cancel on failure
 }
 
 //////////////////////////////////////////////////////////////////////////////
 // nsIRequestObserver
 //////////////////////////////////////////////////////////////////////////////