Bug 1444143 part 2. Remove nsIFrameLoader::GetDocShell. r=mystor

The change from "docShell" to "mDocShell" for the SetName call in the
OwnerIsMozBrowserFrame case in nsFrameLoader::MaybeCreateDocShell is a
drive-by correctness fix for a bug the rename of "docShell" to "parentDocShell"
caught: setting the name of our _parent_ docshell based on the name attr of our
owner makes no sense.

MozReview-Commit-ID: DwnWt8jTokV

dom/base/nsFrameLoader.cpp

@@ -621,25 +621,11 @@ nsFrameLoader::CheckURILoad(nsIURI* aURI, nsIPrincipal* aTriggeringPrincipal)
   return CheckForRecursiveLoad(aURI);
 }
 
-already_AddRefed<nsIDocShell>
+nsIDocShell*
 nsFrameLoader::GetDocShell(ErrorResult& aRv)
 {
-  nsCOMPtr<nsIDocShell> docShell;
-  nsresult rv = GetDocShell(getter_AddRefs(docShell));
-  if (NS_FAILED(rv)) {
-    aRv.Throw(rv);
-  }
-  return docShell.forget();
-}
-
-NS_IMETHODIMP
-nsFrameLoader::GetDocShell(nsIDocShell **aDocShell)
-{
-  *aDocShell = nullptr;
-  nsresult rv = NS_OK;
-
   if (IsRemoteFrame()) {
-    return rv;
+    return nullptr;
   }
 
   // If we have an owner, make sure we have a docshell and return
@@ -648,16 +634,14 @@ nsFrameLoader::GetDocShell(nsIDocShell **aDocShell)
   if (mOwnerContent) {
     nsresult rv = MaybeCreateDocShell();
     if (NS_FAILED(rv)) {
-      return rv;
+      aRv.Throw(rv);
+      return nullptr;
     }
     NS_ASSERTION(mDocShell,
                  "MaybeCreateDocShell succeeded, but null mDocShell");
   }
 
-  *aDocShell = mDocShell;
-  NS_IF_ADDREF(*aDocShell);
-
-  return rv;
+  return mDocShell;
 }
 
 static void
@@ -2068,8 +2052,8 @@ nsFrameLoader::MaybeCreateDocShell()
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  nsCOMPtr<nsIDocShell> docShell = doc->GetDocShell();
-  nsCOMPtr<nsIWebNavigation> parentAsWebNav = do_QueryInterface(docShell);
+  nsCOMPtr<nsIDocShell> parentDocShell = doc->GetDocShell();
+  nsCOMPtr<nsIWebNavigation> parentAsWebNav = do_QueryInterface(parentDocShell);
   NS_ENSURE_STATE(parentAsWebNav);
 
   // Create the docshell...
@@ -2106,15 +2090,16 @@ nsFrameLoader::MaybeCreateDocShell()
   // Note: This logic duplicates a lot of logic in
   // nsSubDocumentFrame::AttributeChanged.  We should fix that.
 
-  const int32_t parentType = docShell->ItemType();
+  const int32_t parentType = parentDocShell->ItemType();
 
   // XXXbz why is this in content code, exactly?  We should handle
   // this some other way.....  Not sure how yet.
   nsCOMPtr<nsIDocShellTreeOwner> parentTreeOwner;
-  docShell->GetTreeOwner(getter_AddRefs(parentTreeOwner));
+  parentDocShell->GetTreeOwner(getter_AddRefs(parentTreeOwner));
   NS_ENSURE_STATE(parentTreeOwner);
   mIsTopLevelContent =
-    AddTreeItemToTreeOwner(mDocShell, parentTreeOwner, parentType, docShell);
+    AddTreeItemToTreeOwner(mDocShell, parentTreeOwner, parentType,
+                           parentDocShell);
 
   if (mIsTopLevelContent) {
     mDocShell->SetCreatedDynamically(false);
@@ -2135,7 +2120,7 @@ nsFrameLoader::MaybeCreateDocShell()
     // Our parent shell is a content shell. Get the chrome event
     // handler from it and use that for our shell as well.
 
-    docShell->GetChromeEventHandler(getter_AddRefs(chromeEventHandler));
+    parentDocShell->GetChromeEventHandler(getter_AddRefs(chromeEventHandler));
   }
 
   mDocShell->SetChromeEventHandler(chromeEventHandler);
@@ -2183,8 +2168,8 @@ nsFrameLoader::MaybeCreateDocShell()
   }
 
   OriginAttributes attrs;
-  if (docShell->ItemType() == mDocShell->ItemType()) {
-    attrs = nsDocShell::Cast(docShell)->GetOriginAttributes();
+  if (parentDocShell->ItemType() == mDocShell->ItemType()) {
+    attrs = nsDocShell::Cast(parentDocShell)->GetOriginAttributes();
   }
 
   // Inherit origin attributes from parent document if
@@ -2240,7 +2225,7 @@ nsFrameLoader::MaybeCreateDocShell()
   }
 
   bool isPrivate = false;
-  nsCOMPtr<nsILoadContext> parentContext = do_QueryInterface(docShell);
+  nsCOMPtr<nsILoadContext> parentContext = do_QueryInterface(parentDocShell);
   NS_ENSURE_STATE(parentContext);
 
   rv = parentContext->GetUsePrivateBrowsing(&isPrivate);
@@ -2253,7 +2238,7 @@ nsFrameLoader::MaybeCreateDocShell()
     // For inproc frames, set the docshell properties.
     nsAutoString name;
     if (mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::name, name)) {
-      docShell->SetName(name);
+      mDocShell->SetName(name);
     }
     mDocShell->SetFullscreenAllowed(
       mOwnerContent->HasAttr(kNameSpaceID_None, nsGkAtoms::allowfullscreen) ||
@@ -2482,9 +2467,8 @@ nsFrameLoader::UpdatePositionAndSize(nsSubDocumentFrame *aIFrame)
 void
 nsFrameLoader::UpdateBaseWindowPositionAndSize(nsSubDocumentFrame *aIFrame)
 {
-  nsCOMPtr<nsIDocShell> docShell;
-  GetDocShell(getter_AddRefs(docShell));
-  nsCOMPtr<nsIBaseWindow> baseWindow(do_QueryInterface(docShell));
+  nsCOMPtr<nsIBaseWindow> baseWindow =
+    do_QueryInterface(GetDocShell(IgnoreErrors()));
 
   // resize the sub document
   if (baseWindow) {
@@ -2858,8 +2842,7 @@ nsFrameLoader::CreateStaticClone(nsIFrameLoader* aDest)
   dest->mDocShell->GetContentViewer(getter_AddRefs(viewer));
   NS_ENSURE_STATE(viewer);
 
-  nsCOMPtr<nsIDocShell> origDocShell;
-  GetDocShell(getter_AddRefs(origDocShell));
+  nsIDocShell* origDocShell = GetDocShell(IgnoreErrors());
   NS_ENSURE_STATE(origDocShell);
 
   nsCOMPtr<nsIDocument> doc = origDocShell->GetDocument();
@@ -3271,9 +3254,7 @@ nsFrameLoader::LoadContext()
       (mRemoteBrowser || TryRemoteBrowser())) {
     loadContext = mRemoteBrowser->GetLoadContext();
   } else {
-    nsCOMPtr<nsIDocShell> docShell;
-    GetDocShell(getter_AddRefs(docShell));
-    loadContext = do_GetInterface(docShell);
+    loadContext = do_GetInterface(GetDocShell(IgnoreErrors()));
   }
   return loadContext.forget();
 }

dom/base/nsFrameLoader.h

@@ -101,7 +101,7 @@ public:
 
   // WebIDL methods
 
-  already_AddRefed<nsIDocShell> GetDocShell(mozilla::ErrorResult& aRv);
+  nsIDocShell* GetDocShell(mozilla::ErrorResult& aRv);
 
   already_AddRefed<nsITabParent> GetTabParent();
 

dom/base/nsIFrameLoader.idl

@@ -20,11 +20,6 @@ interface nsIWebProgressListener;
 [builtinclass, uuid(1645af04-1bc7-4363-8f2c-eb9679220ab1)]
 interface nsIFrameLoader : nsISupports
 {
-  /**
-   * Get the docshell from the frame loader.
-   */
-  readonly attribute nsIDocShell docShell;
-
   /**
    * Get this frame loader's TabParent, if it has a remote frame.  Otherwise,
    * returns null.

dom/base/nsObjectLoadingContent.cpp

@@ -582,8 +582,9 @@ nsObjectLoadingContent::SetupDocShell(nsIURI* aRecursionCheckURI)
   if (aRecursionCheckURI) {
     nsresult rv = mFrameLoader->CheckForRecursiveLoad(aRecursionCheckURI);
     if (NS_SUCCEEDED(rv)) {
-      rv = mFrameLoader->GetDocShell(getter_AddRefs(docShell));
-      if (NS_FAILED(rv)) {
+      IgnoredErrorResult result;
+      docShell = mFrameLoader->GetDocShell(result);
+      if (result.Failed()) {
         NS_NOTREACHED("Could not get DocShell from mFrameLoader?");
       }
     } else {

dom/browser-element/BrowserElementParent.cpp

@@ -284,8 +284,7 @@ BrowserElementParent::OpenWindowInProcess(nsPIDOMWindowOuter* aOpenerWindow,
   RefPtr<nsFrameLoader> frameLoader = popupFrameElement->GetFrameLoader();
   NS_ENSURE_TRUE(frameLoader, BrowserElementParent::OPEN_WINDOW_IGNORED);
 
-  nsCOMPtr<nsIDocShell> docshell;
-  frameLoader->GetDocShell(getter_AddRefs(docshell));
+  nsCOMPtr<nsIDocShell> docshell = frameLoader->GetDocShell(IgnoreErrors());
   NS_ENSURE_TRUE(docshell, BrowserElementParent::OPEN_WINDOW_IGNORED);
 
   nsCOMPtr<nsPIDOMWindowOuter> window = docshell->GetWindow();

dom/events/EventStateManager.cpp

@@ -4258,8 +4258,7 @@ EventStateManager::NotifyMouseOut(WidgetMouseEvent* aMouseEvent,
     // tell the subdocument that we're moving out of it
     nsSubDocumentFrame* subdocFrame = do_QueryFrame(wrapper->mLastOverFrame.GetFrame());
     if (subdocFrame) {
-      nsCOMPtr<nsIDocShell> docshell;
-      subdocFrame->GetDocShell(getter_AddRefs(docshell));
+      nsIDocShell* docshell = subdocFrame->GetDocShell();
       if (docshell) {
         RefPtr<nsPresContext> presContext;
         docshell->GetPresContext(getter_AddRefs(presContext));

dom/html/nsGenericHTMLFrameElement.cpp

@@ -127,8 +127,7 @@ nsGenericHTMLFrameElement::GetContentWindow()
     return nullptr;
   }
 
-  nsCOMPtr<nsIDocShell> doc_shell;
-  mFrameLoader->GetDocShell(getter_AddRefs(doc_shell));
+  nsCOMPtr<nsIDocShell> doc_shell = mFrameLoader->GetDocShell(IgnoreErrors());
   if (!doc_shell) {
     return nullptr;
   }

dom/presentation/PresentationSessionInfo.cpp

@@ -1585,7 +1585,7 @@ PresentationPresentingInfo::ResolvedCallback(JSContext* aCx,
     return;
   }
 
-  nsCOMPtr<nsIFrameLoader> frameLoader = owner->GetFrameLoader();
+  RefPtr<nsFrameLoader> frameLoader = owner->GetFrameLoader();
   if (NS_WARN_IF(!frameLoader)) {
     ReplyError(NS_ERROR_DOM_OPERATION_ERR);
     return;
@@ -1600,9 +1600,9 @@ PresentationPresentingInfo::ResolvedCallback(JSContext* aCx,
     Unused << NS_WARN_IF(!static_cast<ContentParent*>(mContentParent.get())->SendNotifyPresentationReceiverLaunched(tabParent, mSessionId));
   } else {
     // In-process frame
-    nsCOMPtr<nsIDocShell> docShell;
-    rv = frameLoader->GetDocShell(getter_AddRefs(docShell));
-    if (NS_WARN_IF(NS_FAILED(rv))) {
+    IgnoredErrorResult error;
+    nsCOMPtr<nsIDocShell> docShell = frameLoader->GetDocShell(error);
+    if (NS_WARN_IF(error.Failed())) {
       ReplyError(NS_ERROR_DOM_OPERATION_ERR);
       return;
     }

layout/generic/nsSubDocumentFrame.cpp

@@ -248,8 +248,7 @@ nsSubDocumentFrame::GetSubdocumentPresShellForPainting(uint32_t aFlags)
       // If we don't have a frame we use this roundabout way to get the pres shell.
       if (!mFrameLoader)
         return nullptr;
-      nsCOMPtr<nsIDocShell> docShell;
-      mFrameLoader->GetDocShell(getter_AddRefs(docShell));
+      nsIDocShell* docShell = mFrameLoader->GetDocShell(IgnoreErrors());
       if (!docShell)
         return nullptr;
       presShell = docShell->GetPresShell();
@@ -1066,13 +1065,14 @@ nsSubDocumentFrame::FrameLoader()
 
 // XXX this should be called ObtainDocShell or something like that,
 // to indicate that it could have side effects
-nsresult
-nsSubDocumentFrame::GetDocShell(nsIDocShell **aDocShell)
+nsIDocShell*
+nsSubDocumentFrame::GetDocShell()
 {
-  *aDocShell = nullptr;
-
-  NS_ENSURE_STATE(FrameLoader());
-  return mFrameLoader->GetDocShell(aDocShell);
+  // How can FrameLoader() return null???
+  if (NS_WARN_IF(!FrameLoader())) {
+    return nullptr;
+  }
+  return mFrameLoader->GetDocShell(IgnoreErrors());
 }
 
 static void
@@ -1297,8 +1297,7 @@ nsSubDocumentFrame::ObtainIntrinsicSizeFrame()
     // Try to get an nsIFrame for our sub-document's document element
     nsIFrame* subDocRoot = nullptr;
 
-    nsCOMPtr<nsIDocShell> docShell;
-    GetDocShell(getter_AddRefs(docShell));
+    nsIDocShell* docShell = GetDocShell();
     if (docShell) {
       nsCOMPtr<nsIPresShell> presShell = docShell->GetPresShell();
       if (presShell) {

layout/generic/nsSubDocumentFrame.h

@@ -94,7 +94,7 @@ public:
   mozilla::a11y::AccType AccessibleType() override;
 #endif
 
-  nsresult GetDocShell(nsIDocShell **aDocShell);
+  nsIDocShell* GetDocShell();
   nsresult BeginSwapDocShells(nsIFrame* aOther);
   void EndSwapDocShells(nsIFrame* aOther);
   nsView* EnsureInnerView();

layout/xul/ContainerBoxObject.cpp

@@ -37,9 +37,7 @@ ContainerBoxObject::GetDocShell()
   if (subDocFrame) {
     // Ok, the frame for mContent is an nsSubDocumentFrame, it knows how
     // to reach the docshell, so ask it...
-    nsCOMPtr<nsIDocShell> ret;
-    subDocFrame->GetDocShell(getter_AddRefs(ret));
-    return ret.forget();
+    return do_AddRef(subDocFrame->GetDocShell());
   }
 
   if (!mContent) {

netwerk/base/LoadInfo.cpp

@@ -167,11 +167,11 @@ LoadInfo::LoadInfo(nsIPrincipal* aLoadingPrincipal,
     // any other non-document-loading element.
     nsCOMPtr<nsIFrameLoaderOwner> frameLoaderOwner =
       do_QueryInterface(aLoadingContext);
-    nsCOMPtr<nsIFrameLoader> fl = frameLoaderOwner ?
+    RefPtr<nsFrameLoader> fl = frameLoaderOwner ?
       frameLoaderOwner->GetFrameLoader() : nullptr;
     if (fl) {
-      nsCOMPtr<nsIDocShell> docShell;
-      if (NS_SUCCEEDED(fl->GetDocShell(getter_AddRefs(docShell))) && docShell) {
+      nsCOMPtr<nsIDocShell> docShell = fl->GetDocShell(IgnoreErrors());
+      if (docShell) {
         nsCOMPtr<nsPIDOMWindowOuter> outerWindow = do_GetInterface(docShell);
         if (outerWindow) {
           mFrameOuterWindowID = outerWindow->WindowID();