Bug 579846 - nsIHttpChannel::SetResponseHeader should work after the stream has ended, r=bzbarsky, a=betaN+
authorHonza Bambas <honzab.moz@firemni.cz>
Sun, 16 Jan 2011 17:58:49 +0100
changeset 60666 2d7561bc0cb013ee27ae62b0aa05a43055edc4d2
parent 60665 b93fc0ad9fe05823fd16b4d9a74141d6ee0acbb0
child 60667 0d08db0bf6765a50360c9636fae6af2be6f1c324
child 60671 f80d15a6292680e3110da650355fa54ed50cf720
push idunknown
push userunknown
push dateunknown
reviewersbzbarsky, betaN
bugs579846
milestone2.0b10pre
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 579846 - nsIHttpChannel::SetResponseHeader should work after the stream has ended, r=bzbarsky, a=betaN+
netwerk/base/public/nsICacheInfoChannel.idl
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
parser/html/nsHtml5StreamParser.cpp
parser/html/nsHtml5StreamParser.h
parser/htmlparser/tests/mochitest/Makefile.in
parser/htmlparser/tests/mochitest/sub_bug579846.sjs
parser/htmlparser/tests/mochitest/test_bug579846.html
--- a/netwerk/base/public/nsICacheInfoChannel.idl
+++ b/netwerk/base/public/nsICacheInfoChannel.idl
@@ -54,8 +54,20 @@ interface nsICacheInfoChannel : nsISuppo
 
   /**
    * TRUE if this channel's data is being loaded from the cache.  This value
    * is undefined before the channel fires its OnStartRequest notification
    * and after the channel fires its OnStopRequest notification.
    */
   boolean isFromCache();
 };
+
+[scriptable, uuid(746064fc-8783-4d19-9c5d-6361ed807b39)]
+interface nsICacheInfoChannel_MOZILLA_2_0_BRANCH : nsISupports
+{
+  /**
+   * Return an object that while not released prevents the channel from
+   * releasing the cache entry after all work on it has been done.  Used by
+   * asynchronous consumers that needs to work with the cache entry long after
+   * onStopRequest has been called.
+   */
+  readonly attribute nsISupports cacheEntryClosePreventer;
+};
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -116,30 +116,33 @@ nsHttpChannel::nsHttpChannel()
     : mLogicalOffset(0)
     , mCacheAccess(0)
     , mPostID(0)
     , mRequestTime(0)
     , mOnCacheEntryAvailableCallback(nsnull)
     , mAsyncCacheOpen(PR_FALSE)
     , mPendingAsyncCallOnResume(nsnull)
     , mSuspendCount(0)
+    , mCacheEntryClosePreventionCount(0)
     , mCachedContentIsValid(PR_FALSE)
     , mCachedContentIsPartial(PR_FALSE)
     , mTransactionReplaced(PR_FALSE)
     , mAuthRetryPending(PR_FALSE)
     , mResuming(PR_FALSE)
     , mInitedCacheEntry(PR_FALSE)
     , mCacheForOfflineUse(PR_FALSE)
     , mCachingOpportunistically(PR_FALSE)
     , mFallbackChannel(PR_FALSE)
     , mTracingEnabled(PR_TRUE)
     , mCustomConditionalRequest(PR_FALSE)
     , mFallingBack(PR_FALSE)
     , mWaitingForRedirectCallback(PR_FALSE)
     , mRequestTimeInitialized(PR_FALSE)
+    , mDeferredCacheEntryClose(PR_FALSE)
+    , mDoomCacheEntryOnClose(PR_FALSE)
 {
     LOG(("Creating nsHttpChannel [this=%p]\n", this));
 }
 
 nsHttpChannel::~nsHttpChannel()
 {
     LOG(("Destroying nsHttpChannel [this=%p]\n", this));
 
@@ -2823,31 +2826,53 @@ nsHttpChannel::ReadFromCache()
         mCachePump->Suspend();
 
     return NS_OK;
 }
 
 void
 nsHttpChannel::CloseCacheEntry(PRBool doomOnFailure)
 {
-    if (!mCacheEntry)
+    if (!mCacheEntry || mDeferredCacheEntryClose)
         return;
 
     LOG(("nsHttpChannel::CloseCacheEntry [this=%p] mStatus=%x mCacheAccess=%x",
          this, mStatus, mCacheAccess));
 
+    mDoomCacheEntryOnClose = doomOnFailure;
+
+    if (mCacheEntryClosePreventionCount) {
+        LOG(("  close is on hold"));
+        mDeferredCacheEntryClose = PR_TRUE;
+        return;
+    }
+
+    CloseCacheEntryInternal();
+}
+
+void
+nsHttpChannel::CloseCacheEntryInternal()
+{
+    LOG(("nsHttpChannel::CloseCacheEntryInternal [this=%p] mStatus=%x mCacheAccess=%x",
+         this, mStatus, mCacheAccess));
+
+    // perform any final cache operations before we close the cache entry.
+    if ((mCacheAccess & nsICache::ACCESS_WRITE) && mRequestTimeInitialized) {
+        FinalizeCacheEntry();
+    }
+    
     // If we have begun to create or replace a cache entry, and that cache
     // entry is not complete and not resumable, then it needs to be doomed.
     // Otherwise, CheckCache will make the mistake of thinking that the
     // partial cache entry is complete.
 
     PRBool doom = PR_FALSE;
     if (mInitedCacheEntry) {
         NS_ASSERTION(mResponseHead, "oops");
-        if (NS_FAILED(mStatus) && doomOnFailure &&
+        if (NS_FAILED(mStatus) && mDoomCacheEntryOnClose &&
             (mCacheAccess & nsICache::ACCESS_WRITE) &&
             !mResponseHead->IsResumable())
             doom = PR_TRUE;
     }
     else if (mCacheAccess == nsICache::ACCESS_WRITE)
         doom = PR_TRUE;
 
     if (doom) {
@@ -3485,16 +3510,17 @@ NS_IMPL_ADDREF_INHERITED(nsHttpChannel, 
 NS_IMPL_RELEASE_INHERITED(nsHttpChannel, HttpBaseChannel)
 
 NS_INTERFACE_MAP_BEGIN(nsHttpChannel)
     NS_INTERFACE_MAP_ENTRY(nsIRequest)
     NS_INTERFACE_MAP_ENTRY(nsIChannel)
     NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
     NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
     NS_INTERFACE_MAP_ENTRY(nsIHttpChannel)
+    NS_INTERFACE_MAP_ENTRY(nsICacheInfoChannel_MOZILLA_2_0_BRANCH)
     NS_INTERFACE_MAP_ENTRY(nsICacheInfoChannel)
     NS_INTERFACE_MAP_ENTRY(nsICachingChannel)
     NS_INTERFACE_MAP_ENTRY(nsIUploadChannel)
     NS_INTERFACE_MAP_ENTRY(nsIUploadChannel2)
     NS_INTERFACE_MAP_ENTRY(nsICacheListener)
     NS_INTERFACE_MAP_ENTRY(nsIHttpChannelInternal)
     NS_INTERFACE_MAP_ENTRY(nsIResumableChannel)
     NS_INTERFACE_MAP_ENTRY(nsITransportEventSink)
@@ -4014,22 +4040,16 @@ nsHttpChannel::OnStopRequest(nsIRequest 
         // if this transaction has been replaced, then bail.
         if (mTransactionReplaced)
             return NS_OK;
     }
 
     mIsPending = PR_FALSE;
     mStatus = status;
 
-    // perform any final cache operations before we close the cache entry.
-    if (mCacheEntry && (mCacheAccess & nsICache::ACCESS_WRITE) &&
-        mRequestTimeInitialized){
-        FinalizeCacheEntry();
-    }
-    
     if (mListener) {
         LOG(("  calling OnStopRequest\n"));
         mListener->OnStopRequest(this, mListenerContext, status);
         mListener = 0;
         mListenerContext = 0;
     }
 
     if (mCacheEntry)
@@ -4203,16 +4223,60 @@ nsHttpChannel::SetCacheTokenCachedCharse
     if (!mCacheEntry)
         return NS_ERROR_NOT_AVAILABLE;
 
     return mCacheEntry->SetMetaDataElement("charset",
                                            PromiseFlatCString(aCharset).get());
 }
 
 //-----------------------------------------------------------------------------
+// nsHttpChannel::nsIHttpChannelInternal_GECKO_2_0
+//-----------------------------------------------------------------------------
+
+class HttpChannelCacheEntryClosePreventer : public nsISupports
+{
+public:
+    NS_DECL_ISUPPORTS
+
+    HttpChannelCacheEntryClosePreventer(nsHttpChannel* channel) 
+    : mChannel(channel)
+    {
+        ++mChannel->mCacheEntryClosePreventionCount;
+        LOG(("mCacheEntryClosePreventionCount increased to %d, [this=%x]",
+             mChannel->mCacheEntryClosePreventionCount,
+             mChannel.get()));
+    }
+
+private:
+    ~HttpChannelCacheEntryClosePreventer()
+    {
+        --mChannel->mCacheEntryClosePreventionCount;
+        LOG(("mCacheEntryClosePreventionCount decreased to %d, [this=%x]",
+             mChannel->mCacheEntryClosePreventionCount,
+             mChannel.get()));
+
+        if (!mChannel->mCacheEntryClosePreventionCount && 
+            mChannel->mDeferredCacheEntryClose) {
+            mChannel->CloseCacheEntryInternal();
+        }
+    }
+
+    nsRefPtr<nsHttpChannel> mChannel;
+};
+
+NS_IMPL_ISUPPORTS0(HttpChannelCacheEntryClosePreventer)
+
+NS_IMETHODIMP
+nsHttpChannel::GetCacheEntryClosePreventer(nsISupports** _retval)
+{
+    NS_ADDREF(*_retval = new HttpChannelCacheEntryClosePreventer(this));
+    return NS_OK;
+}
+
+//-----------------------------------------------------------------------------
 // nsHttpChannel::nsICachingChannel
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsHttpChannel::GetCacheToken(nsISupports **token)
 {
     NS_ENSURE_ARG_POINTER(token);
     if (!mCacheEntry)
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -46,54 +46,58 @@
 #include "HttpBaseChannel.h"
 
 #include "nsHttpTransaction.h"
 #include "nsInputStreamPump.h"
 #include "nsThreadUtils.h"
 #include "nsTArray.h"
 
 #include "nsIHttpEventSink.h"
+#include "nsICacheInfoChannel.h"
 #include "nsICachingChannel.h"
 #include "nsICacheEntryDescriptor.h"
 #include "nsICacheListener.h"
 #include "nsIApplicationCacheChannel.h"
 #include "nsIPrompt.h"
 #include "nsIResumableChannel.h"
 #include "nsIProtocolProxyCallback.h"
 #include "nsICancelable.h"
 #include "nsIHttpAuthenticableChannel.h"
 #include "nsITraceableChannel.h"
 #include "nsIHttpChannelAuthProvider.h"
 #include "nsIAsyncVerifyRedirectCallback.h"
 #include "nsICryptoHash.h"
 
 class nsAHttpConnection;
 class AutoRedirectVetoNotifier;
+class HttpChannelCacheEntryClosePreventer;
 
 using namespace mozilla::net;
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel
 //-----------------------------------------------------------------------------
 
 class nsHttpChannel : public HttpBaseChannel
                     , public nsIStreamListener
+                    , public nsICacheInfoChannel_MOZILLA_2_0_BRANCH
                     , public nsICachingChannel
                     , public nsICacheListener
                     , public nsITransportEventSink
                     , public nsIProtocolProxyCallback
                     , public nsIHttpAuthenticableChannel
                     , public nsITraceableChannel
                     , public nsIApplicationCacheChannel
                     , public nsIAsyncVerifyRedirectCallback
 {
 public:
     NS_DECL_ISUPPORTS_INHERITED
     NS_DECL_NSIREQUESTOBSERVER
     NS_DECL_NSISTREAMLISTENER
+    NS_DECL_NSICACHEINFOCHANNEL_MOZILLA_2_0_BRANCH
     NS_DECL_NSICACHEINFOCHANNEL
     NS_DECL_NSICACHINGCHANNEL
     NS_DECL_NSICACHELISTENER
     NS_DECL_NSITRANSPORTEVENTSINK
     NS_DECL_NSIPROTOCOLPROXYCALLBACK
     NS_DECL_NSIPROXIEDCHANNEL
     NS_DECL_NSITRACEABLECHANNEL
     NS_DECL_NSIAPPLICATIONCACHECONTAINER
@@ -224,16 +228,17 @@ private:
                                          PRBool aSync);
     nsresult OpenOfflineCacheEntryForWriting();
     nsresult GenerateCacheKey(PRUint32 postID, nsACString &key);
     nsresult UpdateExpirationTime();
     nsresult CheckCache();
     nsresult ShouldUpdateOfflineCacheEntry(PRBool *shouldCacheForOfflineUse);
     nsresult ReadFromCache();
     void     CloseCacheEntry(PRBool doomOnFailure);
+    void     CloseCacheEntryInternal();
     void     CloseOfflineCacheEntry();
     nsresult InitCacheEntry();
     nsresult InitOfflineCacheEntry();
     nsresult AddCacheEntryHeaders(nsICacheEntryDescriptor *entry);
     nsresult StoreAuthorizationMetaData(nsICacheEntryDescriptor *entry);
     nsresult FinalizeCacheEntry();
     nsresult InstallCacheListener(PRUint32 offset = 0);
     nsresult InstallOfflineCacheListener();
@@ -317,16 +322,20 @@ private:
     // cache entry.
     nsCString                         mFallbackKey;
 
     friend class AutoRedirectVetoNotifier;
     nsCOMPtr<nsIURI>                  mRedirectURI;
     nsCOMPtr<nsIChannel>              mRedirectChannel;
     PRUint32                          mRedirectType;
 
+    // Close prevention count, keeps the number of existing prevention objects,
+    // positive value prevents the cache entry from release in OnStopRequest.
+    PRUint32                          mCacheEntryClosePreventionCount;
+
     // state flags
     PRUint32                          mCachedContentIsValid     : 1;
     PRUint32                          mCachedContentIsPartial   : 1;
     PRUint32                          mTransactionReplaced      : 1;
     PRUint32                          mAuthRetryPending         : 1;
     PRUint32                          mResuming                 : 1;
     PRUint32                          mInitedCacheEntry         : 1;
     PRUint32                          mCacheForOfflineUse       : 1;
@@ -340,20 +349,27 @@ private:
     // True if consumer added its own If-None-Match or If-Modified-Since
     // headers. In such a case we must not override them in the cache code
     // and also we want to pass possible 304 code response through.
     PRUint32                          mCustomConditionalRequest : 1;
     PRUint32                          mFallingBack              : 1;
     PRUint32                          mWaitingForRedirectCallback : 1;
     // True if mRequestTime has been set. In such a case it is safe to update
     // the cache entry's expiration time. Otherwise, it is not(see bug 567360).
-    PRUint32                          mRequestTimeInitialized : 1;
+    PRUint32                          mRequestTimeInitialized   : 1;
+    // True if CloseCacheEntry was called while cache entry close prevention
+    // count was positive.
+    PRUint32                          mDeferredCacheEntryClose  : 1;
+    // True if CloseCacheEntry was called with doomOnFailure set to TRUE.
+    PRUint32                          mDoomCacheEntryOnClose    : 1;
 
     nsTArray<nsContinueRedirectionFunc> mRedirectFuncStack;
 
     nsCOMPtr<nsICryptoHash>        mHasher;
 
     nsresult WaitForRedirectCallback();
     void PushRedirectAsyncFunc(nsContinueRedirectionFunc func);
     void PopRedirectAsyncFunc(nsContinueRedirectionFunc func);
+
+    friend class HttpChannelCacheEntryClosePreventer;
 };
 
 #endif // nsHttpChannel_h__
--- a/parser/html/nsHtml5StreamParser.cpp
+++ b/parser/html/nsHtml5StreamParser.cpp
@@ -36,16 +36,17 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsHtml5StreamParser.h"
 #include "nsICharsetConverterManager.h"
 #include "nsICharsetAlias.h"
+#include "nsICacheInfoChannel.h"
 #include "nsServiceManagerUtils.h"
 #include "nsEncoderDecoderUtils.h"
 #include "nsContentUtils.h"
 #include "nsHtml5Tokenizer.h"
 #include "nsIHttpChannel.h"
 #include "nsHtml5Parser.h"
 #include "nsHtml5TreeBuilder.h"
 #include "nsHtml5AtomTable.h"
@@ -215,16 +216,17 @@ nsHtml5StreamParser::nsHtml5StreamParser
 }
 
 nsHtml5StreamParser::~nsHtml5StreamParser()
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   mTokenizer->end();
   NS_ASSERTION(!mFlushTimer, "Flush timer was not dropped before dtor!");
 #ifdef DEBUG
+  mCacheEntryClosePreventer = nsnull;
   mRequest = nsnull;
   mObserver = nsnull;
   mUnicodeDecoder = nsnull;
   mSniffingBuffer = nsnull;
   mMetaScanner = nsnull;
   mFirstBuffer = nsnull;
   mExecutor = nsnull;
   mTreeBuilder = nsnull;
@@ -553,16 +555,27 @@ nsHtml5StreamParser::OnStartRequest(nsIR
   NS_PRECONDITION(!mExecutor->HasStarted(), 
                   "Got OnStartRequest at the wrong stage in the executor life cycle.");
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   if (mObserver) {
     mObserver->OnStartRequest(aRequest, aContext);
   }
   mRequest = aRequest;
 
+  // We must keep the cache entry hold lock to prevent the channel from
+  // dropping the cache entry after OnStopRequest.  We may need to modify
+  // the cache entry asynchronously, after OnStopRequest.
+  // See bug 579846.
+  nsCOMPtr<nsICacheInfoChannel_MOZILLA_2_0_BRANCH> cacheInfoChannel =
+      do_QueryInterface(aRequest);
+  if (cacheInfoChannel) {
+    cacheInfoChannel->
+      GetCacheEntryClosePreventer(getter_AddRefs(mCacheEntryClosePreventer));
+  }
+
   mStreamState = STREAM_BEING_READ;
 
   PRBool scriptingEnabled = mExecutor->IsScriptEnabled();
   mOwner->StartTokenizer(scriptingEnabled);
   mTreeBuilder->setScriptingEnabled(scriptingEnabled);
   mTokenizer->start();
   mExecutor->Start();
   mExecutor->StartReadingFromStage();
--- a/parser/html/nsHtml5StreamParser.h
+++ b/parser/html/nsHtml5StreamParser.h
@@ -326,16 +326,17 @@ class nsHtml5StreamParser : public nsISt
     /**
      * Parser thread entry point for (maybe) flushing the ops and posting
      * a flush runnable back on the main thread.
      */
     void TimerFlush();
 
     nsCOMPtr<nsIRequest>          mRequest;
     nsCOMPtr<nsIRequestObserver>  mObserver;
+    nsCOMPtr<nsISupports>         mCacheEntryClosePreventer;
 
     /**
      * The Unicode decoder
      */
     nsCOMPtr<nsIUnicodeDecoder>   mUnicodeDecoder;
 
     /**
      * The buffer for sniffing the character encoding
--- a/parser/htmlparser/tests/mochitest/Makefile.in
+++ b/parser/htmlparser/tests/mochitest/Makefile.in
@@ -79,16 +79,18 @@ include $(topsrcdir)/config/rules.mk
 		file_bug594730-4.html \
 		file_bug594730-5.html \
 		file_bug594730-6.html \
 		file_bug594730-7.html \
 		file_bug594730-8.html \
 		file_bug594730-9.html \
 		test_bug599584.html \
 		iframe_bug599584.html \
+		test_bug579846.html \
+		sub_bug579846.sjs \
 		$(NULL)
 
 # Disabled test due to orange on Linux
 #		test_bug568470.html \
 #		file_bug568470.sjs \
 #		file_bug568470-script.sjs \
 
 # Disable test due to frequent orange on Mac
new file mode 100644
--- /dev/null
+++ b/parser/htmlparser/tests/mochitest/sub_bug579846.sjs
@@ -0,0 +1,17 @@
+function handleRequest(request, response)
+{
+  var date = new Date();
+  var now = date.getTime();
+  date.setTime(date.getTime() + 5 * 60 * 1000);
+  var exp = (date).toGMTString();
+
+  response.setHeader("Content-Type", "text/html", false);
+  response.setHeader("Expires", exp, false);
+
+  response.write(
+    "<html>\n" +
+    "<head><meta http-equiv='Cache-control' content='no-cache'></head>\n" +
+    "<body onload='parent.testFrameOnLoad(document.body.innerHTML)'>" + now + "</body>" +
+    "</html>"
+  );
+}
new file mode 100644
--- /dev/null
+++ b/parser/htmlparser/tests/mochitest/test_bug579846.html
@@ -0,0 +1,47 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=579846
+-->
+<head>
+  <title>Test for Bug 579846</title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <script type="text/javascript">
+    SimpleTest.waitForExplicitFinish();
+
+    var nextAction;
+    function testFrameOnLoad(content)
+    {
+      nextAction(content);
+    }
+
+    nextAction = function(acontent)
+    {
+      nextAction = function(bcontent)
+      {
+        ok(parseInt(acontent) < parseInt(bcontent), "Content has changed");
+        SimpleTest.finish();
+      }
+
+      // Give a chance to update
+      window.setTimeout(function() {
+        var testFrame = document.getElementById('_test_iframe2');
+        testFrame.contentWindow.location.href = 'sub_bug579846.sjs';
+      }, 100);
+    }
+
+    window.onload = function()
+    {
+      var testFrame = document.getElementById('_test_iframe1');
+      testFrame.contentWindow.location.href = 'sub_bug579846.sjs';
+    }
+  </script>
+</head>
+<body>
+  <iframe src="" id="_test_iframe1"></iframe>
+  <iframe src="" id="_test_iframe2"></iframe>
+</body>
+</html>
+