Bug 1497995 - Make URI a constructor argument of nsDocShellLoadState; r=bzbarsky
authorKyle Machulis <kyle@nonpolynomial.com>
Fri, 21 Dec 2018 20:17:43 +0000
changeset 508861 63e0af38669e2984ebd98f83499ab7aea383ff92
parent 508860 2ddf61b1db11219e203fe17fe5aaa84424fed362
child 508862 70996d85e8beb2a4053f0938da90b55354e2ef5a
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1497995, 1515433
milestone66.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 1497995 - Make URI a constructor argument of nsDocShellLoadState; r=bzbarsky We'll always need a URI for DocShellLoadState, and it should only change is special circumstances. Construct the object with it, and then follow up in Bug 1515433 for more cleanup. Differential Revision: https://phabricator.services.mozilla.com/D13490
docshell/base/nsDocShell.cpp
docshell/base/nsDocShellLoadState.cpp
docshell/base/nsDocShellLoadState.h
docshell/shistory/nsSHistory.cpp
dom/base/Location.cpp
dom/base/nsFrameLoader.cpp
dom/clients/manager/ClientNavigateOpChild.cpp
toolkit/components/windowwatcher/nsWindowWatcher.cpp
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3969,30 +3969,29 @@ nsDocShell::LoadURIWithOptions(const nsA
   bool forceAllowDataURI = aLoadFlags & LOAD_FLAGS_FORCE_ALLOW_DATA_URI;
 
   // Don't pass certain flags that aren't needed and end up confusing
   // ConvertLoadTypeToDocShellInfoLoadType.  We do need to ensure that they are
   // passed to LoadURI though, since it uses them.
   uint32_t extraFlags = (aLoadFlags & EXTRA_LOAD_FLAGS);
   aLoadFlags &= ~EXTRA_LOAD_FLAGS;
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(uri);
 
   /*
    * If the user "Disables Protection on This Page", we have to make sure to
    * remember the users decision when opening links in child tabs [Bug 906190]
    */
   if (aLoadFlags & LOAD_FLAGS_ALLOW_MIXED_CONTENT) {
     loadState->SetLoadType(
         MAKE_LOAD_TYPE(LOAD_NORMAL_ALLOW_MIXED_CONTENT, aLoadFlags));
   } else {
     loadState->SetLoadType(MAKE_LOAD_TYPE(LOAD_NORMAL, aLoadFlags));
   }
 
-  loadState->SetURI(uri);
   loadState->SetLoadFlags(extraFlags);
   loadState->SetFirstParty(true);
   loadState->SetPostDataStream(postStream);
   loadState->SetReferrer(aReferringURI);
   loadState->SetReferrerPolicy((mozilla::net::ReferrerPolicy)aReferrerPolicy);
   loadState->SetHeadersStream(aHeaderStream);
   loadState->SetBaseURI(aBaseURI);
   loadState->SetTriggeringPrincipal(aTriggeringPrincipal);
@@ -4568,18 +4567,17 @@ nsresult nsDocShell::LoadErrorPage(nsIUR
 
   if (mLSHE) {
     // Abandon mLSHE's BFCache entry and create a new one.  This way, if
     // we go back or forward to another SHEntry with the same doc
     // identifier, the error page won't persist.
     mLSHE->AbandonBFCacheEntry();
   }
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
-  loadState->SetURI(aErrorURI);
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aErrorURI);
   loadState->SetTriggeringPrincipal(nsContentUtils::GetSystemPrincipal());
   loadState->SetLoadType(LOAD_ERROR_PAGE);
   loadState->SetFirstParty(true);
   loadState->SetSourceDocShell(this);
 
   return InternalLoad(loadState, nullptr, nullptr);
 }
 
@@ -4666,18 +4664,17 @@ nsDocShell::Reload(uint32_t aReloadFlags
     nsCOMPtr<nsIURI> currentURI = mCurrentURI;
     nsCOMPtr<nsIURI> referrerURI = mReferrerURI;
     uint32_t referrerPolicy = mReferrerPolicy;
 
     // Reload always rewrites result principal URI.
     Maybe<nsCOMPtr<nsIURI>> emplacedResultPrincipalURI;
     emplacedResultPrincipalURI.emplace(std::move(resultPrincipalURI));
 
-    RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
-    loadState->SetURI(currentURI);
+    RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(currentURI);
     loadState->SetOriginalURI(originalURI);
     loadState->SetMaybeResultPrincipalURI(emplacedResultPrincipalURI);
     loadState->SetLoadReplace(loadReplace);
     loadState->SetReferrer(referrerURI);
     loadState->SetReferrerPolicy((mozilla::net::ReferrerPolicy)referrerPolicy);
     loadState->SetTriggeringPrincipal(triggeringPrincipal);
     loadState->SetPrincipalToInherit(triggeringPrincipal);
     loadState->SetLoadFlags(flags);
@@ -5869,17 +5866,17 @@ nsresult nsDocShell::ForceRefreshURIFrom
   return ForceRefreshURI(aURI, aPrincipal, aDelay, aMetaRefresh);
 }
 
 NS_IMETHODIMP
 nsDocShell::ForceRefreshURI(nsIURI* aURI, nsIPrincipal* aPrincipal,
                             int32_t aDelay, bool aMetaRefresh) {
   NS_ENSURE_ARG(aURI);
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aURI);
 
   /* We do need to pass in a referrer, but we don't want it to
    * be sent to the server.
    */
   loadState->SetSendReferrer(false);
 
   /* for most refreshes the current URI is an appropriate
    * internal referrer
@@ -5924,17 +5921,16 @@ nsDocShell::ForceRefreshURI(nsIURI* aURI
     GetReferringURI(getter_AddRefs(internalReferrer));
     if (internalReferrer) {
       loadState->SetReferrer(internalReferrer);
     }
   } else {
     loadState->SetLoadType(LOAD_REFRESH);
   }
 
-  loadState->SetURI(aURI);
   loadState->SetLoadFlags(
       nsIWebNavigation::LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL);
   loadState->SetFirstParty(true);
 
   /*
    * LoadURI(...) will cancel all refresh timers... This causes the
    * Timer and its refreshData instance to be released...
    */
@@ -8894,17 +8890,18 @@ nsresult nsDocShell::InternalLoad(nsDocS
         // type would be set to LOAD_NORMAL_REPLACE; otherwise it should be
         // LOAD_LINK.
         MOZ_ASSERT(aLoadState->LoadType() == LOAD_LINK ||
                    aLoadState->LoadType() == LOAD_NORMAL_REPLACE);
         MOZ_ASSERT(!aLoadState->SHEntry());
         MOZ_ASSERT(
             aLoadState->FirstParty());  // Windowwatcher will assume this.
 
-        RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
+        RefPtr<nsDocShellLoadState> loadState =
+            new nsDocShellLoadState(aLoadState->URI());
 
         // Set up our loadinfo so it will do the load as much like we would have
         // as possible.
         loadState->SetReferrer(aLoadState->Referrer());
         loadState->SetReferrerPolicy(
             (mozilla::net::ReferrerPolicy)aLoadState->ReferrerPolicy());
         loadState->SetSendReferrer(!(
             aLoadState->HasLoadFlags(INTERNAL_LOAD_FLAGS_DONT_SEND_REFERRER)));
@@ -11632,18 +11629,17 @@ nsresult nsDocShell::LoadHistoryEntry(ns
 
   // Passing nullptr as aSourceDocShell gives the same behaviour as before
   // aSourceDocShell was introduced. According to spec we should be passing
   // the source browsing context that was used when the history entry was
   // first created. bug 947716 has been created to address this issue.
   Maybe<nsCOMPtr<nsIURI>> emplacedResultPrincipalURI;
   emplacedResultPrincipalURI.emplace(std::move(resultPrincipalURI));
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
-  loadState->SetURI(uri);
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(uri);
   loadState->SetOriginalURI(originalURI);
   loadState->SetMaybeResultPrincipalURI(emplacedResultPrincipalURI);
   loadState->SetLoadReplace(loadReplace);
   loadState->SetReferrer(referrerURI);
   loadState->SetReferrerPolicy((mozilla::net::ReferrerPolicy)referrerPolicy);
   loadState->SetTriggeringPrincipal(triggeringPrincipal);
   loadState->SetPrincipalToInherit(principalToInherit);
   loadState->SetLoadFlags(flags);
@@ -12750,18 +12746,17 @@ nsDocShell::OnLinkClickSync(nsIContent* 
   bool inOnLoadHandler = false;
   GetIsExecutingOnLoadHandler(&inOnLoadHandler);
   uint32_t loadType = inOnLoadHandler ? LOAD_NORMAL_REPLACE : LOAD_LINK;
 
   if (aIsUserTriggered) {
     flags |= INTERNAL_LOAD_FLAGS_IS_USER_TRIGGERED;
   }
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
-  loadState->SetURI(aURI);
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aURI);
   loadState->SetReferrer(referer);
   loadState->SetReferrerPolicy((mozilla::net::ReferrerPolicy)refererPolicy);
   loadState->SetTriggeringPrincipal(triggeringPrincipal);
   loadState->SetPrincipalToInherit(aContent->NodePrincipal());
   loadState->SetLoadFlags(flags);
   loadState->SetTarget(aTargetSpec);
   loadState->SetTypeHint(NS_ConvertUTF16toUTF8(typeHint));
   loadState->SetFileName(aFileName);
--- a/docshell/base/nsDocShellLoadState.cpp
+++ b/docshell/base/nsDocShellLoadState.cpp
@@ -8,34 +8,37 @@
 #include "nsIDocShell.h"
 #include "nsIDocShellTreeItem.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsIWebNavigation.h"
 
 #include "mozilla/OriginAttributes.h"
 #include "mozilla/NullPrincipal.h"
 
-nsDocShellLoadState::nsDocShellLoadState()
-    : mResultPrincipalURIIsSome(false),
+nsDocShellLoadState::nsDocShellLoadState(nsIURI* aURI)
+    : mURI(aURI),
+      mResultPrincipalURIIsSome(false),
       mKeepResultPrincipalURIIfSet(false),
       mLoadReplace(false),
       mInheritPrincipal(false),
       mPrincipalIsExplicit(false),
       mForceAllowDataURI(false),
       mOriginalFrameSrc(false),
       mSendReferrer(true),
       mReferrerPolicy(mozilla::net::RP_Unset),
       mLoadType(LOAD_NORMAL),
       mTarget(),
       mSrcdocData(VoidString()),
       mLoadFlags(0),
       mFirstParty(false),
       mTypeHint(VoidCString()),
       mFileName(VoidString()),
-      mIsFromProcessingFrameAttributes(false) {}
+      mIsFromProcessingFrameAttributes(false) {
+  MOZ_ASSERT(aURI, "Cannot create a LoadState with a null URI!");
+}
 
 nsDocShellLoadState::~nsDocShellLoadState() {}
 
 nsIURI* nsDocShellLoadState::Referrer() const { return mReferrer; }
 
 void nsDocShellLoadState::SetReferrer(nsIURI* aReferrer) {
   mReferrer = aReferrer;
 }
--- a/docshell/base/nsDocShellLoadState.h
+++ b/docshell/base/nsDocShellLoadState.h
@@ -22,17 +22,17 @@ class OriginAttibutes;
 /**
  * nsDocShellLoadState contains setup information used in a nsIDocShell::loadURI
  * call.
  */
 class nsDocShellLoadState final {
  public:
   NS_INLINE_DECL_REFCOUNTING(nsDocShellLoadState);
 
-  nsDocShellLoadState();
+  explicit nsDocShellLoadState(nsIURI* aURI);
 
   // Getters and Setters
 
   nsIURI* Referrer() const;
 
   void SetReferrer(nsIURI* aReferrer);
 
   nsIURI* URI() const;
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -1463,34 +1463,33 @@ nsresult nsSHistory::LoadDifferingEntrie
   }
   return result;
 }
 
 nsresult nsSHistory::InitiateLoad(nsISHEntry* aFrameEntry,
                                   nsIDocShell* aFrameDS, long aLoadType) {
   NS_ENSURE_STATE(aFrameDS && aFrameEntry);
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
+  nsCOMPtr<nsIURI> newURI = aFrameEntry->GetURI();
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(newURI);
 
   /* Set the loadType in the SHEntry too to  what was passed on.
    * This will be passed on to child subframes later in nsDocShell,
    * so that proper loadType is maintained through out a frameset
    */
   aFrameEntry->SetLoadType(aLoadType);
 
   loadState->SetLoadType(aLoadType);
   loadState->SetSHEntry(aFrameEntry);
 
   nsCOMPtr<nsIURI> originalURI = aFrameEntry->GetOriginalURI();
   loadState->SetOriginalURI(originalURI);
 
   loadState->SetLoadReplace(aFrameEntry->GetLoadReplace());
 
-  nsCOMPtr<nsIURI> newURI = aFrameEntry->GetURI();
-  loadState->SetURI(newURI);
   loadState->SetLoadFlags(nsIWebNavigation::LOAD_FLAGS_NONE);
   nsCOMPtr<nsIPrincipal> triggeringPrincipal =
       aFrameEntry->GetTriggeringPrincipal();
   loadState->SetTriggeringPrincipal(triggeringPrincipal);
   loadState->SetFirstParty(false);
 
   // Time to initiate a document load
   return aFrameDS->LoadURI(loadState);
--- a/dom/base/Location.cpp
+++ b/dom/base/Location.cpp
@@ -141,17 +141,17 @@ already_AddRefed<nsDocShellLoadState> Lo
       }
     }
   } else {
     // No document; just use our subject principal as the triggering principal.
     triggeringPrincipal = &aSubjectPrincipal;
   }
 
   // Create load info
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aURI);
 
   loadState->SetTriggeringPrincipal(triggeringPrincipal);
 
   if (sourceURI) {
     loadState->SetReferrer(sourceURI);
     loadState->SetReferrerPolicy(referrerPolicy);
   }
 
@@ -216,17 +216,16 @@ void Location::SetURI(nsIURI* aURI, nsIP
 
     // Get the incumbent script's browsing context to set as source.
     nsCOMPtr<nsPIDOMWindowInner> sourceWindow =
         do_QueryInterface(mozilla::dom::GetIncumbentGlobal());
     if (sourceWindow) {
       loadState->SetSourceDocShell(sourceWindow->GetDocShell());
     }
 
-    loadState->SetURI(aURI);
     loadState->SetLoadFlags(nsIWebNavigation::LOAD_FLAGS_NONE);
     loadState->SetFirstParty(true);
 
     nsresult rv = docShell->LoadURI(loadState);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       aRv.Throw(rv);
     }
   }
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -371,17 +371,17 @@ nsresult nsFrameLoader::ReallyStartLoadi
   }
   NS_ASSERTION(mDocShell,
                "MaybeCreateDocShell succeeded with a null mDocShell");
 
   // Just to be safe, recheck uri.
   rv = CheckURILoad(mURIToLoad, mTriggeringPrincipal);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(mURIToLoad);
 
   loadState->SetOriginalFrameSrc(mLoadingOriginalSrc);
   mLoadingOriginalSrc = false;
 
   // If this frame is sandboxed with respect to origin we will set it up with
   // a null principal later in nsDocShell::DoURILoad.
   // We do it there to correctly sandbox content that was loaded into
   // the frame via other methods than the src attribute.
@@ -451,17 +451,16 @@ nsresult nsFrameLoader::ReallyStartLoadi
             nsIWebNavigation::LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL;
   }
 
   loadState->SetIsFromProcessingFrameAttributes();
 
   // Kick off the load...
   bool tmpState = mNeedsAsyncDestroy;
   mNeedsAsyncDestroy = true;
-  loadState->SetURI(mURIToLoad);
   loadState->SetLoadFlags(flags);
   loadState->SetFirstParty(false);
   rv = mDocShell->LoadURI(loadState);
   mNeedsAsyncDestroy = tmpState;
   mURIToLoad = nullptr;
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
--- a/dom/clients/manager/ClientNavigateOpChild.cpp
+++ b/dom/clients/manager/ClientNavigateOpChild.cpp
@@ -213,23 +213,22 @@ RefPtr<ClientOpPromise> ClientNavigateOp
 
   nsCOMPtr<nsIDocShell> docShell = window->GetDocShell();
   nsCOMPtr<nsIWebProgress> webProgress = do_GetInterface(docShell);
   if (!docShell || !webProgress) {
     return ClientOpPromise::CreateAndReject(NS_ERROR_DOM_INVALID_STATE_ERR,
                                             __func__);
   }
 
-  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState();
+  RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(url);
 
   loadState->SetTriggeringPrincipal(principal);
   loadState->SetReferrerPolicy(doc->GetReferrerPolicy());
   loadState->SetLoadType(LOAD_STOP_CONTENT);
   loadState->SetSourceDocShell(docShell);
-  loadState->SetURI(url);
   loadState->SetLoadFlags(nsIWebNavigation::LOAD_FLAGS_NONE);
   loadState->SetFirstParty(true);
   rv = docShell->LoadURI(loadState);
   if (NS_FAILED(rv)) {
     return ClientOpPromise::CreateAndReject(rv, __func__);
   }
 
   RefPtr<ClientOpPromise::Private> promise =
--- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp
+++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp
@@ -1046,18 +1046,22 @@ nsresult nsWindowWatcher::OpenWindowInte
     nsCOMPtr<nsILoadContext> childContext = do_QueryInterface(newDocShellItem);
     if (childContext) {
       childContext->SetPrivateBrowsing(isPrivateBrowsingWindow);
       childContext->SetRemoteTabs(isRemoteWindow);
     }
   }
 
   RefPtr<nsDocShellLoadState> loadState = aLoadState;
-  if (uriToLoad && aNavigate && !loadState) {
-    loadState = new nsDocShellLoadState();
+  if (uriToLoad && loadState) {
+    // If a URI was passed to this function, open that, not what was passed in
+    // the original LoadState. See Bug 1515433.
+    loadState->SetURI(uriToLoad);
+  } else if (uriToLoad && aNavigate && !loadState) {
+    loadState = new nsDocShellLoadState(uriToLoad);
 
     if (subjectPrincipal) {
       loadState->SetTriggeringPrincipal(subjectPrincipal);
     }
 #ifndef ANDROID
     MOZ_ASSERT(subjectPrincipal,
                "nsWindowWatcher: triggeringPrincipal required");
 #endif
@@ -1117,17 +1121,16 @@ nsresult nsWindowWatcher::OpenWindowInte
 
       obsSvc->NotifyObservers(static_cast<nsIPropertyBag2*>(props),
                               "webNavigation-createdNavigationTarget-from-js",
                               nullptr);
     }
   }
 
   if (uriToLoad && aNavigate) {
-    loadState->SetURI(uriToLoad);
     loadState->SetLoadFlags(
         windowIsNew
             ? static_cast<uint32_t>(nsIWebNavigation::LOAD_FLAGS_FIRST_LOAD)
             : static_cast<uint32_t>(nsIWebNavigation::LOAD_FLAGS_NONE));
     loadState->SetFirstParty(true);
     // Should this pay attention to errors returned by LoadURI?
     newDocShell->LoadURI(loadState);
   }