Bug 1338241 - Part 2: Simplify the in large allocation process logic, r=smaug

MozReview-Commit-ID: 6aF2wlLgy42

browser/modules/E10SUtils.jsm

@@ -188,7 +188,7 @@ this.E10SUtils = {
     // If we are in a Large-Allocation process, and it wouldn't be content visible
     // to change processes, we want to load into a new process so that we can throw
     // this one out.
-    if (aDocShell.inLargeAllocProcess &&
+    if (Services.appinfo.remoteType == LARGE_ALLOCATION_REMOTE_TYPE &&
         !aDocShell.awaitingLargeAlloc &&
         aDocShell.isOnlyToplevelInTabGroup) {
       return false;

docshell/base/nsDocShell.cpp

@@ -14911,11 +14911,3 @@ nsDocShell::GetAwaitingLargeAlloc(bool* aResult)
   *aResult = static_cast<TabChild*>(tabChild.get())->IsAwaitingLargeAlloc();
   return NS_OK;
 }
-
-NS_IMETHODIMP
-nsDocShell::GetInLargeAllocProcess(bool* aResult)
-{
-  MOZ_ASSERT(aResult);
-  *aResult = TabChild::InLargeAllocProcess();
-  return NS_OK;
-}

docshell/base/nsIDocShell.idl

@@ -1126,12 +1126,6 @@ interface nsIDocShell : nsIDocShellTreeItem
    */
   [infallible] readonly attribute boolean isOnlyToplevelInTabGroup;
 
-  /**
-   * Returns `true` if this docshell is loaded within a Large-Allocation
-   * process.
-   */
-  [infallible] readonly attribute boolean inLargeAllocProcess;
-
   /**
    * Returns `true` if this docshell was created due to a Large-Allocation
    * header, and has not seen the initiating load yet.

dom/base/nsContentUtils.cpp

@@ -9788,8 +9788,9 @@ nsContentUtils::AttemptLargeAllocationLoad(nsIHttpChannel* aChannel)
   TabChild* tabChild = TabChild::GetFrom(outer->AsOuter());
   NS_ENSURE_TRUE(tabChild, false);
 
-  if (tabChild->TakeAwaitingLargeAlloc())  {
+  if (tabChild->IsAwaitingLargeAlloc())  {
     NS_WARNING("In a Large-Allocation TabChild, ignoring Large-Allocation header!");
+    tabChild->StopAwaitingLargeAlloc();
     outer->SetLargeAllocStatus(LargeAllocStatus::SUCCESS);
     return false;
   }

dom/ipc/ContentParent.cpp

@@ -672,21 +672,24 @@ ContentParent::RandomSelect(const nsTArray<ContentParent*>& aContentParents,
 /*static*/ already_AddRefed<ContentParent>
 ContentParent::GetNewOrUsedBrowserProcess(const nsAString& aRemoteType,
                                           ProcessPriority aPriority,
-                                          ContentParent* aOpener,
-                                          bool* aNew)
+                                          ContentParent* aOpener)
 {
-  if (aNew) {
-    *aNew = false;
-  }
-
   nsTArray<ContentParent*>& contentParents = GetOrCreatePool(aRemoteType);
 
   uint32_t maxContentParents = GetMaxProcessCount(aRemoteType);
 
   RefPtr<ContentParent> p;
-  if (contentParents.Length() >= uint32_t(maxContentParents) &&
-      (p = RandomSelect(contentParents, aOpener, maxContentParents))) {
-    return p.forget();
+  if (contentParents.Length() >= maxContentParents) {
+    // We never want to re-use Large-Allocation processes.
+    if (aRemoteType.EqualsLiteral(LARGE_ALLOCATION_REMOTE_TYPE)) {
+      return GetNewOrUsedBrowserProcess(NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE),
+                                        aPriority,
+                                        aOpener);
+    }
+
+    if ((p = RandomSelect(contentParents, aOpener, maxContentParents))) {
+      return p.forget();
+    }
   }
 
   // Try to take the preallocated process only for the default process type.
@@ -697,10 +700,6 @@ ContentParent::GetNewOrUsedBrowserProcess(const nsAString& aRemoteType,
   } else {
     p = new ContentParent(aOpener, aRemoteType);
 
-    if (aNew) {
-      *aNew = true;
-    }
-
     if (!p->LaunchSubprocess(aPriority)) {
       return nullptr;
     }
@@ -1004,7 +1003,6 @@ ContentParent::CreateBrowser(const TabContext& aContext,
     remoteType.AssignLiteral(DEFAULT_REMOTE_TYPE);
   }
 
-  bool newProcess = false;
   RefPtr<nsIContentParent> constructorSender;
   if (isInContentProcess) {
     MOZ_ASSERT(aContext.IsMozBrowserElement());
@@ -1015,8 +1013,7 @@ ContentParent::CreateBrowser(const TabContext& aContext,
       constructorSender = aOpenerContentParent;
     } else {
       constructorSender =
-        GetNewOrUsedBrowserProcess(remoteType, initialPriority, nullptr,
-                                   &newProcess);
+        GetNewOrUsedBrowserProcess(remoteType, initialPriority, nullptr);
       if (!constructorSender) {
         return nullptr;
       }
@@ -1065,9 +1062,8 @@ ContentParent::CreateBrowser(const TabContext& aContext,
 
     if (remoteType.EqualsLiteral(LARGE_ALLOCATION_REMOTE_TYPE)) {
       // Tell the TabChild object that it was created due to a Large-Allocation
-      // request, and whether or not that Large-Allocation request succeeded at
-      // creating a new content process.
-      Unused << browser->SendSetIsLargeAllocation(true, newProcess);
+      // request.
+      Unused << browser->SendAwaitLargeAlloc();
     }
 
     if (browser) {

dom/ipc/ContentParent.h

@@ -173,8 +173,7 @@ public:
   GetNewOrUsedBrowserProcess(const nsAString& aRemoteType = NS_LITERAL_STRING(NO_REMOTE_TYPE),
                              hal::ProcessPriority aPriority =
                              hal::ProcessPriority::PROCESS_PRIORITY_FOREGROUND,
-                             ContentParent* aOpener = nullptr,
-                             bool* anew = nullptr);
+                             ContentParent* aOpener = nullptr);
 
   /**
    * Get or create a content process for the given TabContext.  aFrameElement

dom/ipc/PBrowser.ipdl

@@ -896,10 +896,10 @@ child:
     async NotifyPartialSHistoryDeactive();
 
     /**
-     * Tell the child that it is a fresh process created for a Large-Allocation
-     * load.
+     * Tell the TabChild that it should expect a Large-Allocation load to occur.
+     * Loads which occur until this flag is cleared will not leave the process.
      */
-    async SetIsLargeAllocation(bool aIsLA, bool aNewProcess);
+    async AwaitLargeAlloc();
 
 /*
  * FIXME: write protocol!

dom/ipc/TabChild.cpp

@@ -158,7 +158,6 @@ static const char BEFORE_FIRST_PAINT[] = "before-first-paint";
 
 typedef nsDataHashtable<nsUint64HashKey, TabChild*> TabChildMap;
 static TabChildMap* sTabChildren;
-bool TabChild::sInLargeAllocProcess = false;
 
 TabChildBase::TabChildBase()
   : mTabChildGlobal(nullptr)
@@ -3121,10 +3120,9 @@ TabChild::RecvThemeChanged(nsTArray<LookAndFeelInt>&& aLookAndFeelIntCache)
 }
 
 mozilla::ipc::IPCResult
-TabChild::RecvSetIsLargeAllocation(const bool& aIsLA, const bool& aNewProcess)
+TabChild::RecvAwaitLargeAlloc()
 {
-  mAwaitingLA = aIsLA;
-  sInLargeAllocProcess = aIsLA && aNewProcess;
+  mAwaitingLA = true;
   return IPC_OK();
 }
 
@@ -3135,7 +3133,7 @@ TabChild::IsAwaitingLargeAlloc()
 }
 
 bool
-TabChild::TakeAwaitingLargeAlloc()
+TabChild::StopAwaitingLargeAlloc()
 {
   bool awaiting = mAwaitingLA;
   mAwaitingLA = false;

dom/ipc/TabChild.h

@@ -664,17 +664,9 @@ public:
 
   // These methods return `true` if this TabChild is currently awaiting a
   // Large-Allocation header.
-  bool TakeAwaitingLargeAlloc();
+  bool StopAwaitingLargeAlloc();
   bool IsAwaitingLargeAlloc();
 
-  // Returns `true` if this this process was created to load a docshell in a
-  // "Fresh Process". This value is initialized to `false`, and is set to `true`
-  // in RecvSetFreshProcess.
-  static bool InLargeAllocProcess()
-  {
-    return sInLargeAllocProcess;
-  }
-
   already_AddRefed<nsISHistory> GetRelatedSHistory();
 
   mozilla::dom::TabGroup* TabGroup();
@@ -716,8 +708,7 @@ protected:
 
   virtual mozilla::ipc::IPCResult RecvNotifyPartialSHistoryDeactive() override;
 
-  virtual mozilla::ipc::IPCResult RecvSetIsLargeAllocation(const bool& aIsLA,
-                                                           const bool& aNewProcess) override;
+  virtual mozilla::ipc::IPCResult RecvAwaitLargeAlloc() override;
 
 private:
   void HandleDoubleTap(const CSSPoint& aPoint, const Modifiers& aModifiers,
@@ -829,8 +820,6 @@ private:
   uintptr_t mNativeWindowHandle;
 #endif // defined(XP_WIN)
 
-  static bool sInLargeAllocProcess;
-
   DISALLOW_EVIL_CONSTRUCTORS(TabChild);
 };
 

dom/tests/browser/helper_largeAllocation.js

@@ -48,12 +48,7 @@ function getPID(aBrowser) {
 
 function getInLAProc(aBrowser) {
   return ContentTask.spawn(aBrowser, null, () => {
-    try {
-      return docShell.inLargeAllocProcess;
-    } catch (e) {
-      // This must be a non-remote browser, which means it is not fresh
-      return false;
-    }
+    return Services.appinfo.remoteType == "webLargeAllocation";
   });
 }