Backed out 3 changesets (bug 1627533) for causing assertions in RequestContextService.cpp CLOSED TREE
authorNoemi Erli <nerli@mozilla.com>
Thu, 21 May 2020 04:16:21 +0300
changeset 531366 6d55c17a5a856ab243aa4fcaea11368b9890f60b
parent 531365 7aaac87105b7e87c636057fec87311f60c50dd76
child 531367 22862efa8d0460e5dc1ebdf82a3622c08082dc9e
push id37438
push userabutkovits@mozilla.com
push dateThu, 21 May 2020 09:36:57 +0000
treeherdermozilla-central@2d00a1a6495c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1627533
milestone78.0a1
backs out3b35a1852a6011c8094e2d3d538db7093ab6f86c
70fa5e075269720bf83a452698b78abbfeafe5ba
0d10466705340f8a95cb8b64222f611315b5b068
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
Backed out 3 changesets (bug 1627533) for causing assertions in RequestContextService.cpp CLOSED TREE Backed out changeset 3b35a1852a60 (bug 1627533) Backed out changeset 70fa5e075269 (bug 1627533) Backed out changeset 0d1046670534 (bug 1627533)
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
docshell/base/nsDocShell.cpp
netwerk/base/RequestContextService.cpp
netwerk/base/nsLoadGroup.cpp
netwerk/base/nsLoadGroup.h
netwerk/ipc/DocumentLoadListener.cpp
uriloader/base/moz.build
uriloader/base/nsDocLoader.cpp
uriloader/base/nsDocLoader.h
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -24,17 +24,16 @@
 #include "mozilla/dom/StructuredCloneTags.h"
 #include "mozilla/dom/UserActivationIPCUtils.h"
 #include "mozilla/dom/WindowBinding.h"
 #include "mozilla/dom/WindowGlobalChild.h"
 #include "mozilla/dom/WindowGlobalParent.h"
 #include "mozilla/dom/WindowProxyHolder.h"
 #include "mozilla/dom/SyncedContextInlines.h"
 #include "mozilla/net/DocumentLoadListener.h"
-#include "mozilla/net/RequestContextService.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/AsyncEventDispatcher.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/Components.h"
 #include "mozilla/HashTable.h"
 #include "mozilla/Logging.h"
 #include "mozilla/ResultExtensions.h"
 #include "mozilla/Services.h"
@@ -301,26 +300,16 @@ already_AddRefed<BrowsingContext> Browsi
 
   context->mFields.SetWithoutSyncing<IDX_OrientationLock>(
       mozilla::hal::eScreenOrientation_None);
 
   const bool useGlobalHistory =
       inherit ? inherit->GetUseGlobalHistory() : false;
   context->mFields.SetWithoutSyncing<IDX_UseGlobalHistory>(useGlobalHistory);
 
-  nsCOMPtr<nsIRequestContextService> rcsvc =
-      net::RequestContextService::GetOrCreate();
-  if (rcsvc) {
-    nsCOMPtr<nsIRequestContext> requestContext;
-    nsresult rv = rcsvc->NewRequestContext(getter_AddRefs(requestContext));
-    if (NS_SUCCEEDED(rv) && requestContext) {
-      context->mRequestContextId = requestContext->GetID();
-    }
-  }
-
   return context.forget();
 }
 
 already_AddRefed<BrowsingContext> BrowsingContext::CreateIndependent(
     Type aType) {
   RefPtr<BrowsingContext> bc(
       CreateDetached(nullptr, nullptr, EmptyString(), aType));
   bc->mWindowless = bc->IsContent();
@@ -375,17 +364,16 @@ void BrowsingContext::CreateFromIPC(Brow
     context->InitSessionHistory();
   }
 
   // NOTE: Call through the `Set` methods for these values to ensure that any
   // relevant process-local state is also updated.
   context->SetOriginAttributes(aInit.mOriginAttributes);
   context->SetRemoteTabs(aInit.mUseRemoteTabs);
   context->SetRemoteSubframes(aInit.mUseRemoteSubframes);
-  context->mRequestContextId = aInit.mRequestContextId;
   // NOTE: Private browsing ID is set by `SetOriginAttributes`.
 
   Register(context);
 
   context->Attach(/* aFromIPC */ true, aOriginProcess);
 }
 
 BrowsingContext::BrowsingContext(WindowContext* aParentWindow,
@@ -559,22 +547,16 @@ void BrowsingContext::Attach(bool aFromI
 void BrowsingContext::Detach(bool aFromIPC) {
   MOZ_DIAGNOSTIC_ASSERT(mEverAttached);
 
   MOZ_LOG(GetLog(), LogLevel::Debug,
           ("%s: Detaching 0x%08" PRIx64 " from 0x%08" PRIx64,
            XRE_IsParentProcess() ? "Parent" : "Child", Id(),
            GetParent() ? GetParent()->Id() : 0));
 
-  nsCOMPtr<nsIRequestContextService> rcsvc =
-      net::RequestContextService::GetOrCreate();
-  if (rcsvc) {
-    rcsvc->RemoveRequestContext(GetRequestContextId());
-  }
-
   // This will only ever be null if the cycle-collector has unlinked us. Don't
   // try to detach ourselves in that case.
   if (NS_WARN_IF(!mGroup)) {
     return;
   }
 
   if (mParentWindow) {
     mParentWindow->RemoveChildBrowsingContext(this);
@@ -1853,17 +1835,16 @@ BrowsingContext::IPCInitializer Browsing
   IPCInitializer init;
   init.mId = Id();
   init.mParentId = mParentWindow ? mParentWindow->Id() : 0;
   init.mWindowless = mWindowless;
   init.mUseRemoteTabs = mUseRemoteTabs;
   init.mUseRemoteSubframes = mUseRemoteSubframes;
   init.mOriginAttributes = mOriginAttributes;
   init.mHasSessionHistory = mChildSessionHistory != nullptr;
-  init.mRequestContextId = mRequestContextId;
   init.mFields = mFields.Fields();
   return init;
 }
 
 already_AddRefed<WindowContext> BrowsingContext::IPCInitializer::GetParent() {
   RefPtr<WindowContext> parent;
   if (mParentId != 0) {
     parent = WindowContext::GetById(mParentId);
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -254,18 +254,16 @@ class BrowsingContext : public nsILoadCo
   void Embed();
 
   // Get the outer window object for this BrowsingContext if it is in-process
   // and still has a docshell, or null otherwise.
   nsPIDOMWindowOuter* GetDOMWindow() const {
     return mDocShell ? mDocShell->GetWindow() : nullptr;
   }
 
-  uint64_t GetRequestContextId() const { return mRequestContextId; }
-
   // Detach the current BrowsingContext from its parent, in both the
   // child and the parent process.
   void Detach(bool aFromIPC = false);
 
   // Prepare this BrowsingContext to leave the current process.
   void PrepareForProcessChange();
 
   // Triggers a load in the process which currently owns this BrowsingContext.
@@ -561,27 +559,25 @@ class BrowsingContext : public nsILoadCo
 
     uint64_t GetOpenerId() const { return mozilla::Get<IDX_OpenerId>(mFields); }
 
     bool mWindowless = false;
     bool mUseRemoteTabs = false;
     bool mUseRemoteSubframes = false;
     bool mHasSessionHistory = false;
     OriginAttributes mOriginAttributes;
-    uint64_t mRequestContextId = 0;
 
     FieldTuple mFields;
 
     bool operator==(const IPCInitializer& aOther) const {
       return mId == aOther.mId && mParentId == aOther.mParentId &&
              mWindowless == aOther.mWindowless &&
              mUseRemoteTabs == aOther.mUseRemoteTabs &&
              mUseRemoteSubframes == aOther.mUseRemoteSubframes &&
              mOriginAttributes == aOther.mOriginAttributes &&
-             mRequestContextId == aOther.mRequestContextId &&
              mFields == aOther.mFields;
     }
   };
 
   // Create an IPCInitializer object for this BrowsingContext.
   IPCInitializer GetIPCInitializer();
 
   // Create a BrowsingContext object from over IPC.
@@ -803,20 +799,16 @@ class BrowsingContext : public nsILoadCo
   // objectMoved hook and clear it from its finalize hook.
   JS::Heap<JSObject*> mWindowProxy;
   LocationProxy mLocation;
 
   // OriginAttributes for this BrowsingContext. May not be changed after this
   // BrowsingContext is attached.
   OriginAttributes mOriginAttributes;
 
-  // The network request context id, representing the nsIRequestContext
-  // associated with this BrowsingContext, and LoadGroups created for it.
-  uint64_t mRequestContextId = 0;
-
   // Determines if private browsing should be used. May not be changed after
   // this BrowsingContext is attached. This field matches mOriginAttributes in
   // content Browsing Contexts. Currently treated as a binary value: 1 - in
   // private mode, 0 - not private mode.
   uint32_t mPrivateBrowsingId;
 
   // True if Attach() has been called on this BrowsingContext already.
   bool mEverAttached : 1;
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -459,17 +459,17 @@ nsDocShell::~nsDocShell() {
 already_AddRefed<nsDocShell> nsDocShell::Create(
     BrowsingContext* aBrowsingContext, uint64_t aContentWindowID) {
   MOZ_ASSERT(aBrowsingContext, "DocShell without a BrowsingContext!");
 
   nsresult rv;
   RefPtr<nsDocShell> ds = new nsDocShell(aBrowsingContext, aContentWindowID);
 
   // Initialize the underlying nsDocLoader.
-  rv = ds->nsDocLoader::InitWithBrowsingContext(aBrowsingContext);
+  rv = ds->nsDocLoader::Init();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
 
   // Create our ContentListener
   ds->mContentListener = new nsDSURIContentListener(ds);
 
   // If parent intercept is not enabled then we must forward to
--- a/netwerk/base/RequestContextService.cpp
+++ b/netwerk/base/RequestContextService.cpp
@@ -542,16 +542,20 @@ RequestContextService::NewRequestContext
   mTable.Put(rcID, newSC);
   newSC.swap(*rc);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RequestContextService::RemoveRequestContext(const uint64_t rcID) {
+  if (IsNeckoChild() && gNeckoChild) {
+    gNeckoChild->SendRemoveRequestContext(rcID);
+  }
+
   MOZ_ASSERT(NS_IsMainThread());
   mTable.Remove(rcID);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RequestContextService::Observe(nsISupports* subject, const char* topic,
                                const char16_t* data_unicode) {
--- a/netwerk/base/nsLoadGroup.cpp
+++ b/netwerk/base/nsLoadGroup.cpp
@@ -89,33 +89,29 @@ nsLoadGroup::nsLoadGroup()
       mLoadFlags(LOAD_NORMAL),
       mDefaultLoadFlags(0),
       mPriority(PRIORITY_NORMAL),
       mRequests(&sRequestHashOps, sizeof(RequestMapEntry)),
       mStatus(NS_OK),
       mIsCanceling(false),
       mDefaultLoadIsTimed(false),
       mBrowsingContextDiscarded(false),
-      mExternalRequestContext(false),
       mTimedRequests(0),
       mCachedRequests(0) {
   LOG(("LOADGROUP [%p]: Created.\n", this));
 }
 
 nsLoadGroup::~nsLoadGroup() {
   DebugOnly<nsresult> rv = Cancel(NS_BINDING_ABORTED);
   NS_ASSERTION(NS_SUCCEEDED(rv), "Cancel failed");
 
   mDefaultLoadRequest = nullptr;
 
-  if (mRequestContext && !mExternalRequestContext) {
+  if (mRequestContext) {
     mRequestContextService->RemoveRequestContext(mRequestContext->GetID());
-    if (IsNeckoChild() && gNeckoChild) {
-      gNeckoChild->SendRemoveRequestContext(mRequestContext->GetID());
-    }
   }
 
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {
     Unused << os->RemoveObserver(this, "last-pb-context-exited");
   }
 
   LOG(("LOADGROUP [%p]: Destroyed.\n", this));
@@ -974,33 +970,16 @@ nsresult nsLoadGroup::Init() {
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   NS_ENSURE_STATE(os);
 
   Unused << os->AddObserver(this, "last-pb-context-exited", true);
 
   return NS_OK;
 }
 
-nsresult nsLoadGroup::InitWithRequestContextId(
-    const uint64_t& aRequestContextId) {
-  mRequestContextService = RequestContextService::GetOrCreate();
-  if (mRequestContextService) {
-    Unused << mRequestContextService->GetRequestContext(
-        aRequestContextId, getter_AddRefs(mRequestContext));
-  }
-  mExternalRequestContext = true;
-
-  nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
-  NS_ENSURE_STATE(os);
-
-  Unused << os->AddObserver(this, "last-pb-context-exited", true);
-
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsLoadGroup::Observe(nsISupports* aSubject, const char* aTopic,
                      const char16_t* aData) {
   MOZ_ASSERT(!strcmp(aTopic, "last-pb-context-exited"));
 
   OriginAttributes attrs;
   StoragePrincipalHelper::GetRegularPrincipalOriginAttributes(this, attrs);
   if (attrs.mPrivateBrowsingId == 0) {
--- a/netwerk/base/nsLoadGroup.h
+++ b/netwerk/base/nsLoadGroup.h
@@ -51,17 +51,16 @@ class nsLoadGroup : public nsILoadGroup,
   NS_DECL_NSIOBSERVER
 
   ////////////////////////////////////////////////////////////////////////////
   // nsLoadGroup methods:
 
   nsLoadGroup();
 
   nsresult Init();
-  nsresult InitWithRequestContextId(const uint64_t& aRequestContextId);
 
  protected:
   virtual ~nsLoadGroup();
 
   nsresult MergeLoadFlags(nsIRequest* aRequest, nsLoadFlags& flags);
   nsresult MergeDefaultLoadFlags(nsIRequest* aRequest, nsLoadFlags& flags);
 
  private:
@@ -88,17 +87,16 @@ class nsLoadGroup : public nsILoadGroup,
 
   nsWeakPtr mObserver;
   nsWeakPtr mParentLoadGroup;
 
   nsresult mStatus;
   bool mIsCanceling;
   bool mDefaultLoadIsTimed;
   bool mBrowsingContextDiscarded;
-  bool mExternalRequestContext;
 
   /* Telemetry */
   mozilla::TimeStamp mDefaultRequestCreationTime;
   uint32_t mTimedRequests;
   uint32_t mCachedRequests;
 };
 
 }  // namespace net
--- a/netwerk/ipc/DocumentLoadListener.cpp
+++ b/netwerk/ipc/DocumentLoadListener.cpp
@@ -449,21 +449,16 @@ bool DocumentLoadListener::Open(
     httpChannelImpl->SetWarningReporter(this);
   }
 
   nsCOMPtr<nsITimedChannel> timedChannel = do_QueryInterface(mChannel);
   if (timedChannel) {
     timedChannel->SetAsyncOpen(aAsyncOpenTime);
   }
 
-  if (nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(mChannel)) {
-    Unused << httpChannel->SetRequestContextID(
-        browsingContext->GetRequestContextId());
-  }
-
   // nsViewSourceChannel normally replaces the nsIRequest passed to
   // OnStart/StopRequest with itself. We don't need this, and instead
   // we want the original request so that we get different ones for
   // each part of a multipart channel.
   nsCOMPtr<nsIViewSourceChannel> viewSourceChannel;
   if (OtherPid() && (viewSourceChannel = do_QueryInterface(mChannel))) {
     viewSourceChannel->SetReplaceRequest(false);
   }
--- a/uriloader/base/moz.build
+++ b/uriloader/base/moz.build
@@ -25,13 +25,9 @@ EXPORTS += [
     'nsURILoader.h',
 ]
 
 UNIFIED_SOURCES += [
     'nsDocLoader.cpp',
     'nsURILoader.cpp',
 ]
 
-LOCAL_INCLUDES += [
-    '/netwerk/base',
-]
-
 FINAL_LIBRARY = 'xul'
--- a/uriloader/base/nsDocLoader.cpp
+++ b/uriloader/base/nsDocLoader.cpp
@@ -10,17 +10,16 @@
 #include "mozilla/Components.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/Logging.h"
 #include "mozilla/IntegerPrintfMacros.h"
 #include "mozilla/PresShell.h"
 
 #include "nsDocLoader.h"
 #include "nsDocShell.h"
-#include "nsLoadGroup.h"
 #include "nsNetUtil.h"
 #include "nsIHttpChannel.h"
 #include "nsIWebNavigation.h"
 #include "nsIWebProgressListener2.h"
 
 #include "nsString.h"
 
 #include "nsCOMPtr.h"
@@ -135,34 +134,16 @@ nsresult nsDocLoader::Init() {
   if (NS_FAILED(rv)) return rv;
 
   MOZ_LOG(gDocLoaderLog, LogLevel::Debug,
           ("DocLoader:%p: load group %p.\n", this, mLoadGroup.get()));
 
   return NS_OK;
 }
 
-nsresult nsDocLoader::InitWithBrowsingContext(
-    BrowsingContext* aBrowsingContext) {
-  RefPtr<net::nsLoadGroup> loadGroup = new net::nsLoadGroup();
-  nsresult rv = loadGroup->InitWithRequestContextId(
-      aBrowsingContext->GetRequestContextId());
-  if (NS_FAILED(rv)) return rv;
-
-  rv = loadGroup->SetGroupObserver(this);
-  if (NS_FAILED(rv)) return rv;
-
-  mLoadGroup = loadGroup;
-
-  MOZ_LOG(gDocLoaderLog, LogLevel::Debug,
-          ("DocLoader:%p: load group %p.\n", this, mLoadGroup.get()));
-
-  return NS_OK;
-}
-
 nsDocLoader::~nsDocLoader() {
   /*
           |ClearWeakReferences()| here is intended to prevent people holding
      weak references from re-entering this destructor since |QueryReferent()|
      will |AddRef()| me, and the subsequent |Release()| will try to destroy me.
      At this point there should be only weak references remaining (otherwise, we
      wouldn't be getting destroyed).
 
--- a/uriloader/base/nsDocLoader.h
+++ b/uriloader/base/nsDocLoader.h
@@ -26,17 +26,16 @@
 #include "nsCycleCollectionParticipant.h"
 
 #include "mozilla/LinkedList.h"
 #include "mozilla/UniquePtr.h"
 
 namespace mozilla {
 namespace dom {
 class BrowserBridgeChild;
-class BrowsingContext;
 }  // namespace dom
 }  // namespace mozilla
 
 /****************************************************************************
  * nsDocLoader implementation...
  ****************************************************************************/
 
 #define NS_THIS_DOCLOADER_IMPL_CID                   \
@@ -57,18 +56,16 @@ class nsDocLoader : public nsIDocumentLo
  public:
   using BrowserBridgeChild = mozilla::dom::BrowserBridgeChild;
 
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_THIS_DOCLOADER_IMPL_CID)
 
   nsDocLoader();
 
   [[nodiscard]] virtual nsresult Init();
-  [[nodiscard]] nsresult InitWithBrowsingContext(
-      mozilla::dom::BrowsingContext* aBrowsingContext);
 
   static already_AddRefed<nsDocLoader> GetAsDocLoader(nsISupports* aSupports);
   // Needed to deal with ambiguous inheritance from nsISupports...
   static nsISupports* GetAsSupports(nsDocLoader* aDocLoader) {
     return static_cast<nsIDocumentLoader*>(aDocLoader);
   }
 
   // Add aDocLoader as a child to the docloader service.