Bug 1551601 - Do process switch _before_ processing cached redirect r=mayhemer
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 28 May 2019 09:37:06 +0000
changeset 475837 ecceef291b89d6970eaf433dfa520b0930810dcc
parent 475836 267d1abce0581f08bdefc977f24e421c161e603f
child 475838 fa7dcd1f369cf19b7af37d9cfc953fe1d131e1c4
push id86510
push uservalentin.gosu@gmail.com
push dateTue, 28 May 2019 12:44:57 +0000
treeherderautoland@ecceef291b89 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1551601
milestone69.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 1551601 - Do process switch _before_ processing cached redirect r=mayhemer This patch splits ReadFromCache in two - by adding ContinueReadFromCache. ContinueReadFromCache is called asycly when a cross-process-redirect is complete. Sometimes, the channel will be cancelled before the cross-process-redirect is complete, such as in: testing/firefox-ui/tests/functional/safebrowsing/test_notification.py In that case we must make sure to call HandleAsyncAbort if the listener's onStart/StopRequest callbacks haven't been called. Differential Revision: https://phabricator.services.mozilla.com/D31226
netwerk/protocol/http/HttpBaseChannel.h
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.h
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -851,16 +851,20 @@ MOZ_MUST_USE nsresult HttpAsyncAborter<T
   return AsyncCall(&T::HandleAsyncAbort);
 }
 
 // Each subclass needs to define its own version of this (which just calls this
 // base version), else we wind up casting base/derived member function ptrs
 template <class T>
 inline void HttpAsyncAborter<T>::HandleAsyncAbort() {
   MOZ_ASSERT(!mCallOnResume, "How did that happen?");
+  nsresult status = mThis->mStatus;
+  MOZ_LOG(gHttpLog, LogLevel::Debug,
+          ("HttpAsyncAborter::HandleAsyncAbort [this=%p status=%" PRIx32 "]\n",
+           mThis, static_cast<uint32_t>(status)));
 
   if (mThis->mSuspendCount) {
     MOZ_LOG(
         gHttpLog, LogLevel::Debug,
         ("Waiting until resume to do async notification [this=%p]\n", mThis));
     mCallOnResume = [](T* self) {
       self->HandleAsyncAbort();
       return NS_OK;
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -5235,16 +5235,54 @@ nsresult nsHttpChannel::ReadFromCache(bo
     //
     // TODO: This should be done asynchronously so we don't take the cache
     // service lock on the main thread.
     mCacheEntry->MaybeMarkValid();
   }
 
   nsresult rv;
 
+  // notify "http-on-may-change-process" observers
+  gHttpHandler->OnMayChangeProcess(this);
+
+  if (mRedirectContentProcessIdPromise) {
+    PushRedirectAsyncFunc(&nsHttpChannel::ContinueReadFromCache);
+    rv = StartCrossProcessRedirect();
+    if (NS_SUCCEEDED(rv)) {
+      return NS_OK;
+    }
+    PopRedirectAsyncFunc(&nsHttpChannel::ContinueReadFromCache);
+  }
+
+  return ContinueReadFromCache(NS_OK);
+}
+
+nsresult nsHttpChannel::ContinueReadFromCache(nsresult rv) {
+  LOG(("nsHttpChannel::ContinueReadFromCache [this=%p] spec: %s\n", this,
+       mSpec.get()));
+
+  // The channel may have been cancelled in the meantime, either by the
+  // consumer or the channel classifier. In that case, we need to AsyncAbort
+  // to ensure OnStart/StopRequest are called.
+  // This should only be an error code if the cross-process redirect failed,
+  // and OnRedirectVerifyCallback was called with an error code.
+  if (NS_FAILED(rv)) {
+    // rv can only be an error code if this was a failed cross-process redirect
+    // It shouldn't have happened during a revalidation.
+    MOZ_ASSERT(!mDidReval, "Should not be a 304 response");
+    // Also, this should not be possible.
+    MOZ_ASSERT(!mCachedContentIsPartial, "Unexpected partially cached page?");
+
+    MOZ_ASSERT(!mCachePump);
+
+    CloseCacheEntry(false);
+    DoAsyncAbort(NS_FAILED(mStatus) ? mStatus : rv);
+    return rv;
+  }
+
   // Keep the conditions below in sync with the conditions in
   // StartBufferingCachedEntity.
 
   if (WillRedirect(mResponseHead)) {
     // TODO: Bug 759040 - We should call HandleAsyncRedirect directly here,
     // to avoid event dispatching latency.
     MOZ_ASSERT(!mCacheInputStream);
     LOG(("Skipping skip read of cached redirect entity\n"));
--- a/netwerk/protocol/http/nsHttpChannel.h
+++ b/netwerk/protocol/http/nsHttpChannel.h
@@ -404,16 +404,17 @@ class nsHttpChannel final : public HttpB
       nsICacheEntry* entry, bool aNew, nsIApplicationCache* aAppCache,
       nsresult status);
   MOZ_MUST_USE nsresult GenerateCacheKey(uint32_t postID, nsACString& key);
   MOZ_MUST_USE nsresult UpdateExpirationTime();
   MOZ_MUST_USE nsresult CheckPartial(nsICacheEntry* aEntry, int64_t* aSize,
                                      int64_t* aContentLength);
   bool ShouldUpdateOfflineCacheEntry();
   MOZ_MUST_USE nsresult ReadFromCache(bool alreadyMarkedValid);
+  MOZ_MUST_USE nsresult ContinueReadFromCache(nsresult rv);
   void CloseCacheEntry(bool doomOnFailure);
   void CloseOfflineCacheEntry();
   MOZ_MUST_USE nsresult InitCacheEntry();
   void UpdateInhibitPersistentCachingFlag();
   MOZ_MUST_USE nsresult InitOfflineCacheEntry();
   MOZ_MUST_USE nsresult AddCacheEntryHeaders(nsICacheEntry* entry);
   MOZ_MUST_USE nsresult FinalizeCacheEntry();
   MOZ_MUST_USE nsresult InstallCacheListener(int64_t offset = 0);