Bug 748117 - Make sure http channel is pending in OnStartRequest & OnDataAvailable r=jduell

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -70,6 +70,7 @@ HttpBaseChannel::HttpBaseChannel()
   , mContentDispositionHint(UINT32_MAX)
   , mHttpHandler(gHttpHandler)
   , mRedirectCount(0)
+  , mForcePending(false)
 {
   LOG(("Creating HttpBaseChannel @%x\n", this));
 
@@ -187,7 +188,7 @@ NS_IMETHODIMP
 HttpBaseChannel::IsPending(bool *aIsPending)
 {
   NS_ENSURE_ARG_POINTER(aIsPending);
-  *aIsPending = mIsPending;
+  *aIsPending = mIsPending || mForcePending;
   return NS_OK;
 }
 
@@ -1616,6 +1617,13 @@ HttpBaseChannel::AddRedirect(nsIPrincipal *aRedirect)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+HttpBaseChannel::ForcePending(bool aForcePending)
+{
+  mForcePending = aForcePending;
+  return NS_OK;
+}
+
 //-----------------------------------------------------------------------------
 // HttpBaseChannel::nsISupportsPriority
 //-----------------------------------------------------------------------------

netwerk/protocol/http/HttpBaseChannel.h

@@ -169,6 +169,7 @@ public:
   NS_IMETHOD GetResponseTimeoutEnabled(bool *aEnable);
   NS_IMETHOD SetResponseTimeoutEnabled(bool aEnable);
   NS_IMETHOD AddRedirect(nsIPrincipal *aRedirect);
+  NS_IMETHOD ForcePending(bool aForcePending);
 
   inline void CleanRedirectCacheChainIfNecessary()
   {
@@ -369,7 +370,9 @@ protected:
   // so that the timing can still be queried from OnStopRequest
   TimingStruct                      mTransactionTimings;
 
-  nsCOMPtr<nsIPrincipal> mPrincipal;
+  nsCOMPtr<nsIPrincipal>            mPrincipal;
+
+  bool                              mForcePending;
 };
 
 // Share some code while working around C++'s absurd inability to handle casting
@@ -412,7 +415,6 @@ nsresult HttpAsyncAborter<T>::AsyncAbort(nsresult status)
          ("HttpAsyncAborter::AsyncAbort [this=%p status=%x]\n", mThis, status));
 
   mThis->mStatus = status;
-  mThis->mIsPending = false;
 
   // if this fails?  Callers ignore our return value anyway....
   return AsyncCall(&T::HandleAsyncAbort);

netwerk/protocol/http/HttpChannelChild.cpp

@@ -711,7 +711,7 @@ HttpChannelChild::FailedAsyncOpen(const nsresult& status)
   LOG(("HttpChannelChild::FailedAsyncOpen [this=%p status=%x]\n", this, status));
 
   mStatus = status;
-  mIsPending = false;
+
   // We're already being called from IPDL, therefore already "async"
   HandleAsyncAbort();
 }

netwerk/protocol/http/nsHttpChannel.cpp

@@ -226,7 +226,6 @@ nsHttpChannel::nsHttpChannel()
     , mIsPartialRequest(0)
     , mHasAutoRedirectVetoNotifier(0)
     , mDidReval(false)
-    , mForcePending(false)
 {
     LOG(("Creating nsHttpChannel [this=%p]\n", this));
     mChannelCreationTime = PR_Now();
@@ -6205,22 +6204,4 @@ nsHttpChannel::GetPerformance()
     return docPerformance;
 }
 
-void
-nsHttpChannel::ForcePending(bool aForcePending)
-{
-    // Set true here so IsPending will return true.
-    // Required for callback diversion from child back to parent. In such cases
-    // OnStopRequest can be called in the parent before callbacks are diverted
-    // back from the child to the listener in the parent.
-    mForcePending = aForcePending;
-}
-
-NS_IMETHODIMP
-nsHttpChannel::IsPending(bool *aIsPending)
-{
-    NS_ENSURE_ARG_POINTER(aIsPending);
-    *aIsPending = mIsPending || mForcePending;
-    return NS_OK;
-}
-
 } } // namespace mozilla::net

netwerk/protocol/http/nsHttpChannel.h

@@ -103,7 +103,6 @@ public:
     NS_IMETHOD Cancel(nsresult status);
     NS_IMETHOD Suspend();
     NS_IMETHOD Resume();
-    NS_IMETHOD IsPending(bool *aIsPending);
     // nsIChannel
     NS_IMETHOD GetSecurityInfo(nsISupports **aSecurityInfo);
     NS_IMETHOD AsyncOpen(nsIStreamListener *listener, nsISupports *aContext);
@@ -189,8 +188,6 @@ public: /* internal necko use only */
       uint32_t mKeep : 2;
     };
 
-    void ForcePending(bool aForcePending);
-
 private:
     typedef nsresult (nsHttpChannel::*nsContinueRedirectionFunc)(nsresult result);
 
@@ -422,9 +419,6 @@ protected:
 
 private: // cache telemetry
     bool mDidReval;
-
-private:
-    bool mForcePending;
 };
 
 } } // namespace mozilla::net

netwerk/protocol/http/nsIHttpChannelInternal.idl

@@ -199,4 +199,11 @@ interface nsIHttpChannelInternal : nsISupports
      * write to nsIRedirectHistory.redirects.
      */
     void addRedirect(in nsIPrincipal aPrincipal);
+
+  /**
+   * ForcePending(true) overrides the normal behavior for the 
+   * channel's IsPending(), forcing 'true' to be returned. A call to
+   * ForcePending(false) reverts IsPending() back to normal behavior.
+   */
+    void ForcePending(in boolean aForcePending);
 };

netwerk/streamconv/converters/nsUnknownDecoder.cpp

@@ -17,6 +17,7 @@
 
 #include "nsIViewSourceChannel.h"
 #include "nsIHttpChannel.h"
+#include "nsIHttpChannelInternal.h"
 #include "nsNetCID.h"
 #include "nsNetUtil.h"
 
@@ -200,11 +201,24 @@ nsUnknownDecoder::OnStopRequest(nsIRequest* request, nsISupports *aCtxt,
   if (mContentType.IsEmpty()) {
     DetermineContentType(request);
 
+    // Make sure channel listeners see channel as pending while we call 
+    // OnStartRequest/OnDataAvailable, even though the underlying channel 
+    // has already hit OnStopRequest.
+    nsCOMPtr<nsIHttpChannelInternal> httpChannel = do_QueryInterface(request);
+    if (httpChannel) {
+      httpChannel->ForcePending(true);
+    }
+
     rv = FireListenerNotifications(request, aCtxt);
 
     if (NS_FAILED(rv)) {
       aStatus = rv;
     }
+
+    // now we need to set pending state to false before calling OnStopRequest
+    if (httpChannel) {
+      httpChannel->ForcePending(false);
+    }
   }
 
   rv = mNextListener->OnStopRequest(request, aCtxt, aStatus);

netwerk/test/unit/head_channels.js

@@ -85,6 +85,8 @@ ChannelListener.prototype = {
         if (!(this._flags & (CL_EXPECT_FAILURE | CL_ALLOW_UNKNOWN_CL)))
           do_throw("Could not get contentLength");
       }
+      if (!request.isPending())
+        do_throw("request reports itself as not pending from onStartRequest!");
       if (this._contentLen == -1 && !(this._flags & (CL_EXPECT_FAILURE | CL_ALLOW_UNKNOWN_CL)))
         do_throw("Content length is unknown in onStartRequest!");
 

netwerk/test/unit/test_reply_without_content_type.js

@@ -0,0 +1,57 @@
+//
+// tests HTTP replies that lack content-type (where we try to sniff content-type).
+//
+
+// Note: sets Cc and Ci variables
+
+Cu.import("resource://testing-common/httpd.js");
+
+var httpserver = new HttpServer();
+var testpath = "/simple";
+var httpbody = "<html><body>omg hai</body></html>";
+var buffer = "";
+
+var dbg=0
+if (dbg) { print("============== START =========="); }
+
+function run_test() {
+  setup_test();
+  do_test_pending();
+}
+
+function setup_test() {
+  if (dbg) { print("============== setup_test: in"); }
+  httpserver.registerPathHandler(testpath, serverHandler);
+  httpserver.start(-1);
+  var channel = setupChannel(testpath);
+  // ChannelListener defined in head_channels.js
+  channel.asyncOpen(new ChannelListener(checkRequest, channel), null);
+  if (dbg) { print("============== setup_test: out"); }
+}
+
+function setupChannel(path) {
+  var ios = Cc["@mozilla.org/network/io-service;1"].
+                       getService(Ci.nsIIOService);
+  var chan = ios.newChannel("http://localhost:" +
+                            httpserver.identity.primaryPort + path, "", null);
+  chan.QueryInterface(Ci.nsIHttpChannel);
+  chan.requestMethod = "GET";
+  return chan;
+}
+
+function serverHandler(metadata, response) {
+  if (dbg) { print("============== serverHandler: in"); }
+//  no content type set
+//  response.setHeader("Content-Type", "text/plain", false);
+  response.bodyOutputStream.write(httpbody, httpbody.length);
+  if (dbg) { print("============== serverHandler: out"); }
+}
+
+function checkRequest(request, data, context) {
+  if (dbg) { print("============== checkRequest: in"); }
+  do_check_eq(data, httpbody);
+  do_check_eq(request.QueryInterface(Ci.nsIChannel).contentType,"text/html");
+  httpserver.stop(do_test_finished);
+  if (dbg) { print("============== checkRequest: out"); }
+}
+

netwerk/test/unit/xpcshell.ini

@@ -333,3 +333,4 @@ skip-if = os == "android"
 run-if = os == "win"
 [test_udp_multicast.js]
 [test_redirect_history.js]
+[test_reply_without_content_type.js]

netwerk/test/unit_ipc/test_reply_without_content_type_wrap.js

@@ -0,0 +1,7 @@
+//
+// Run test script in content process instead of chrome (xpcshell's default)
+//
+
+function run_test() {
+  run_test_in_child("../unit/test_reply_without_content_type.js");
+}

netwerk/test/unit_ipc/xpcshell.ini

@@ -33,3 +33,4 @@ skip-if = true
 [test_xmlhttprequest_wrap.js]
 [test_XHR_redirects.js]
 [test_redirect_history_wrap.js]
+[test_reply_without_content_type_wrap.js]