Bug 1814686 - Part 2: Make nsDocShellLoadState not-null, and simplify…

… handling of nsDOMNavigationTiming, r=necko-reviewers,valentin,smaug

These types were already non-nullable, with the serializer implementation not
supporting nullptr values. This patch converts the uses to be explicitly
non-nullable, and adds the relevant `WrapNotNull` changes.

Differential Revision: https://phabricator.services.mozilla.com/D168890

docshell/base/BrowsingContext.cpp

@@ -1946,7 +1946,7 @@ nsresult BrowsingContext::LoadURI(nsDocShellLoadState* aLoadState,
       if (!wgc->CanNavigate(this)) {
         return NS_ERROR_DOM_PROP_ACCESS_DENIED;
       }
-      wgc->SendLoadURI(this, aLoadState, aSetNavigating);
+      wgc->SendLoadURI(this, mozilla::WrapNotNull(aLoadState), aSetNavigating);
     }
   } else if (XRE_IsParentProcess()) {
     if (Canonical()->LoadInParent(aLoadState, aSetNavigating)) {
@@ -1969,7 +1969,7 @@ nsresult BrowsingContext::LoadURI(nsDocShellLoadState* aLoadState,
       // load. Normally we'd expect a PDocumentChannel actor to have been
       // created to claim the load identifier by that time. If not, then it
       // won't be coming, so make sure we clean up and deregister.
-      cp->SendLoadURI(this, aLoadState, aSetNavigating)
+      cp->SendLoadURI(this, mozilla::WrapNotNull(aLoadState), aSetNavigating)
           ->Then(GetMainThreadSerialEventTarget(), __func__,
                  [loadIdentifier](
                      const PContentParent::LoadURIPromise::ResolveOrRejectValue&
@@ -2039,7 +2039,7 @@ nsresult BrowsingContext::InternalLoad(nsDocShellLoadState* aLoadState) {
 
     MOZ_ALWAYS_SUCCEEDS(
         SetCurrentLoadIdentifier(Some(aLoadState->GetLoadIdentifier())));
-    Unused << cp->SendInternalLoad(aLoadState);
+    Unused << cp->SendInternalLoad(mozilla::WrapNotNull(aLoadState));
   } else {
     MOZ_DIAGNOSTIC_ASSERT(sourceBC);
     MOZ_DIAGNOSTIC_ASSERT(sourceBC->Group() == Group());
@@ -2056,7 +2056,7 @@ nsresult BrowsingContext::InternalLoad(nsDocShellLoadState* aLoadState) {
 
     MOZ_ALWAYS_SUCCEEDS(
         SetCurrentLoadIdentifier(Some(aLoadState->GetLoadIdentifier())));
-    wgc->SendInternalLoad(aLoadState);
+    wgc->SendInternalLoad(mozilla::WrapNotNull(aLoadState));
   }
 
   return NS_OK;

docshell/base/CanonicalBrowsingContext.cpp

@@ -1020,7 +1020,7 @@ already_AddRefed<nsDocShellLoadState> CanonicalBrowsingContext::CreateLoadInfo(
 
 void CanonicalBrowsingContext::NotifyOnHistoryReload(
     bool aForceReload, bool& aCanReload,
-    Maybe<RefPtr<nsDocShellLoadState>>& aLoadState,
+    Maybe<NotNull<RefPtr<nsDocShellLoadState>>>& aLoadState,
     Maybe<bool>& aReloadActiveEntry) {
   MOZ_DIAGNOSTIC_ASSERT(!aLoadState);
 
@@ -1034,15 +1034,16 @@ void CanonicalBrowsingContext::NotifyOnHistoryReload(
   }
 
   if (mActiveEntry) {
-    aLoadState.emplace(CreateLoadInfo(mActiveEntry));
+    aLoadState.emplace(WrapMovingNotNull(RefPtr{CreateLoadInfo(mActiveEntry)}));
     aReloadActiveEntry.emplace(true);
     if (aForceReload) {
       shistory->RemoveFrameEntries(mActiveEntry);
     }
   } else if (!mLoadingEntries.IsEmpty()) {
     const LoadingSessionHistoryEntry& loadingEntry =
         mLoadingEntries.LastElement();
-    aLoadState.emplace(CreateLoadInfo(loadingEntry.mEntry));
+    aLoadState.emplace(
+        WrapMovingNotNull(RefPtr{CreateLoadInfo(loadingEntry.mEntry)}));
     aReloadActiveEntry.emplace(false);
     if (aForceReload) {
       SessionHistoryEntry::LoadingEntry* entry =
@@ -2486,7 +2487,8 @@ void CanonicalBrowsingContext::RequestRestoreTabContent(
 
     if (data->CanRestoreInto(aWindow->GetDocumentURI())) {
       if (!aWindow->IsInProcess()) {
-        aWindow->SendRestoreTabContent(data, onTabRestoreComplete,
+        aWindow->SendRestoreTabContent(WrapNotNull(data.get()),
+                                       onTabRestoreComplete,
                                        onTabRestoreComplete);
         return;
       }

docshell/base/CanonicalBrowsingContext.h

@@ -157,9 +157,10 @@ class CanonicalBrowsingContext final : public BrowsingContext {
   // aReloadActiveEntry will be true if we have an active entry. If aCanReload
   // is true and aLoadState and aReloadActiveEntry are not set then we should
   // attempt to reload based on the current document in the docshell.
-  void NotifyOnHistoryReload(bool aForceReload, bool& aCanReload,
-                             Maybe<RefPtr<nsDocShellLoadState>>& aLoadState,
-                             Maybe<bool>& aReloadActiveEntry);
+  void NotifyOnHistoryReload(
+      bool aForceReload, bool& aCanReload,
+      Maybe<NotNull<RefPtr<nsDocShellLoadState>>>& aLoadState,
+      Maybe<bool>& aReloadActiveEntry);
 
   // See BrowsingContext::SetActiveSessionHistoryEntry.
   void SetActiveSessionHistoryEntry(const Maybe<nsPoint>& aPreviousScrollPos,

docshell/base/nsDocShell.cpp

@@ -4088,8 +4088,8 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
           mBrowsingContext, forceReload,
           [docShell, doc, loadType, browsingContext, currentURI, referrerInfo,
            loadGroup, stopDetector](
-              Tuple<bool, Maybe<RefPtr<nsDocShellLoadState>>, Maybe<bool>>&&
-                  aResult) {
+              Tuple<bool, Maybe<NotNull<RefPtr<nsDocShellLoadState>>>,
+                    Maybe<bool>>&& aResult) {
             auto scopeExit = MakeScopeExit([loadGroup, stopDetector]() {
               if (loadGroup) {
                 loadGroup->RemoveRequest(stopDetector, nullptr, NS_OK);
@@ -4099,7 +4099,7 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
               return;
             }
             bool canReload;
-            Maybe<RefPtr<nsDocShellLoadState>> loadState;
+            Maybe<NotNull<RefPtr<nsDocShellLoadState>>> loadState;
             Maybe<bool> reloadingActiveEntry;
 
             Tie(canReload, loadState, reloadingActiveEntry) = aResult;
@@ -4127,7 +4127,7 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
     } else {
       // Parent process
       bool canReload = false;
-      Maybe<RefPtr<nsDocShellLoadState>> loadState;
+      Maybe<NotNull<RefPtr<nsDocShellLoadState>>> loadState;
       Maybe<bool> reloadingActiveEntry;
       if (!mBrowsingContext->IsDiscarded()) {
         mBrowsingContext->Canonical()->NotifyOnHistoryReload(

dom/base/nsDOMNavigationTiming.cpp

@@ -576,6 +576,12 @@ nsDOMNavigationTiming::nsDOMNavigationTiming(nsDocShell* aDocShell,
 void mozilla::ipc::IPDLParamTraits<nsDOMNavigationTiming*>::Write(
     IPC::MessageWriter* aWriter, IProtocol* aActor,
     nsDOMNavigationTiming* aParam) {
+  bool isNull = !aParam;
+  WriteIPDLParam(aWriter, aActor, isNull);
+  if (isNull) {
+    return;
+  }
+
   RefPtr<nsIURI> unloadedURI = aParam->mUnloadedURI.get();
   RefPtr<nsIURI> loadedURI = aParam->mLoadedURI.get();
   WriteIPDLParam(aWriter, aActor, unloadedURI ? Some(unloadedURI) : Nothing());
@@ -605,6 +611,15 @@ void mozilla::ipc::IPDLParamTraits<nsDOMNavigationTiming*>::Write(
 bool mozilla::ipc::IPDLParamTraits<nsDOMNavigationTiming*>::Read(
     IPC::MessageReader* aReader, IProtocol* aActor,
     RefPtr<nsDOMNavigationTiming>* aResult) {
+  bool isNull;
+  if (!ReadIPDLParam(aReader, aActor, &isNull)) {
+    return false;
+  }
+  if (isNull) {
+    *aResult = nullptr;
+    return true;
+  }
+
   auto timing = MakeRefPtr<nsDOMNavigationTiming>(nullptr);
   uint32_t type;
   Maybe<RefPtr<nsIURI>> unloadedURI;

dom/ipc/BrowserBridgeHost.cpp

@@ -44,7 +44,7 @@ bool BrowserBridgeHost::CanRecv() const {
 
 void BrowserBridgeHost::LoadURL(nsDocShellLoadState* aLoadState) {
   MOZ_ASSERT(aLoadState);
-  Unused << mBridge->SendLoadURL(aLoadState);
+  Unused << mBridge->SendLoadURL(WrapNotNull(aLoadState));
 }
 
 void BrowserBridgeHost::ResumeLoad(uint64_t aPendingSwitchId) {

dom/ipc/BrowserBridgeParent.cpp

@@ -166,7 +166,7 @@ IPCResult BrowserBridgeParent::RecvScrollbarPreferenceChanged(
 }
 
 IPCResult BrowserBridgeParent::RecvLoadURL(nsDocShellLoadState* aLoadState) {
-  Unused << mBrowserParent->SendLoadURL(aLoadState,
+  Unused << mBrowserParent->SendLoadURL(WrapNotNull(aLoadState),
                                         mBrowserParent->GetShowInfo());
   return IPC_OK();
 }

dom/ipc/BrowserParent.cpp

@@ -871,7 +871,7 @@ void BrowserParent::LoadURL(nsDocShellLoadState* aLoadState) {
     return;
   }
 
-  Unused << SendLoadURL(aLoadState, GetShowInfo());
+  Unused << SendLoadURL(WrapNotNull(aLoadState), GetShowInfo());
 }
 
 void BrowserParent::ResumeLoad(uint64_t aPendingSwitchID) {

dom/ipc/ContentChild.cpp

@@ -3672,9 +3672,9 @@ mozilla::ipc::IPCResult ContentChild::RecvCrossProcessRedirect(
   RefPtr<ChildProcessChannelListener> processListener =
       ChildProcessChannelListener::GetSingleton();
   // The listener will call completeRedirectSetup or asyncOpen on the channel.
-  processListener->OnChannelReady(
-      loadState, aArgs.loadIdentifier(), std::move(aEndpoints),
-      aArgs.timing().refOr(nullptr), std::move(resolve));
+  processListener->OnChannelReady(loadState, aArgs.loadIdentifier(),
+                                  std::move(aEndpoints), aArgs.timing(),
+                                  std::move(resolve));
   scopeExit.release();
 
   // scopeExit will call CrossProcessRedirectFinished(rv) here

dom/ipc/ContentParent.cpp

@@ -7832,14 +7832,15 @@ mozilla::ipc::IPCResult ContentParent::RecvNotifyOnHistoryReload(
     const MaybeDiscarded<BrowsingContext>& aContext, const bool& aForceReload,
     NotifyOnHistoryReloadResolver&& aResolver) {
   bool canReload = false;
-  Maybe<RefPtr<nsDocShellLoadState>> loadState;
+  Maybe<NotNull<RefPtr<nsDocShellLoadState>>> loadState;
   Maybe<bool> reloadActiveEntry;
   if (!aContext.IsDiscarded()) {
     aContext.get_canonical()->NotifyOnHistoryReload(
         aForceReload, canReload, loadState, reloadActiveEntry);
   }
-  aResolver(Tuple<const bool&, const Maybe<RefPtr<nsDocShellLoadState>>&,
-                  const Maybe<bool>&>(canReload, loadState, reloadActiveEntry));
+  aResolver(
+      Tuple<const bool&, const Maybe<NotNull<RefPtr<nsDocShellLoadState>>>&,
+            const Maybe<bool>&>(canReload, loadState, reloadActiveEntry));
   return IPC_OK();
 }
 

dom/ipc/PBrowser.ipdl

@@ -694,7 +694,7 @@ child:
 
     async CompositorOptionsChanged(CompositorOptions newOptions);
 
-    async LoadURL(nullable nsDocShellLoadState loadState, ParentShowInfo info);
+    async LoadURL(nsDocShellLoadState loadState, ParentShowInfo info);
 
     async CreateAboutBlankContentViewer(nullable nsIPrincipal principal,
                                         nullable nsIPrincipal partitionedPrincipal);

dom/ipc/PBrowserBridge.ipdl

@@ -93,7 +93,7 @@ parent:
   async BeginDestroy();
 
   // DocShell messaging.
-  async LoadURL(nullable nsDocShellLoadState aLoadState);
+  async LoadURL(nsDocShellLoadState aLoadState);
   async ResumeLoad(uint64_t aPendingSwitchID);
 
   // Out of process rendering.

dom/ipc/PContent.ipdl

@@ -995,10 +995,10 @@ child:
     async InitSandboxTesting(Endpoint<PSandboxTestingChild> aEndpoint);
 #endif
 
-    async LoadURI(MaybeDiscardedBrowsingContext aContext, nullable nsDocShellLoadState aLoadState, bool aSetNavigating)
+    async LoadURI(MaybeDiscardedBrowsingContext aContext, nsDocShellLoadState aLoadState, bool aSetNavigating)
         returns (bool aSuccess);
 
-    async InternalLoad(nullable nsDocShellLoadState aLoadState);
+    async InternalLoad(nsDocShellLoadState aLoadState);
 
     async DisplayLoadError(MaybeDiscardedBrowsingContext aContext, nsString aURI);
 
@@ -1804,7 +1804,7 @@ parent:
 
     async NotifyOnHistoryReload(MaybeDiscardedBrowsingContext aContext,
                                 bool aForceReload)
-            returns (bool canReload, nullable nsDocShellLoadState? loadState,
+            returns (bool canReload, nsDocShellLoadState? loadState,
                      bool? reloadActiveEntry);
 
     async HistoryCommit(MaybeDiscardedBrowsingContext aContext,

dom/ipc/PWindowGlobal.ipdl

@@ -104,9 +104,9 @@ parent:
   // BrowsingContext. aTargetBC must be in the same BrowsingContextGroup as this
   // window global.
   async LoadURI(MaybeDiscardedBrowsingContext aTargetBC,
-                nullable nsDocShellLoadState aLoadState, bool aSetNavigating);
+                nsDocShellLoadState aLoadState, bool aSetNavigating);
 
-  async InternalLoad(nullable nsDocShellLoadState aLoadState);
+  async InternalLoad(nsDocShellLoadState aLoadState);
 
   /// Update the URI of the document in this WindowGlobal.
   [LazySend] async UpdateDocumentURI(nullable nsIURI aUri);

netwerk/ipc/DocumentChannelChild.cpp

@@ -108,33 +108,20 @@ DocumentChannelChild::AsyncOpen(nsIStreamListener* aListener) {
   }
   mLoadingContext = loadingContext;
 
-  DocumentChannelCreationArgs args;
-
-  args.loadState() = mLoadState;
-  args.cacheKey() = mCacheKey;
-  args.channelId() = mChannelId;
-  args.asyncOpenTime() = TimeStamp::Now();
-  args.parentInitiatedNavigationEpoch() =
-      loadingContext->GetParentInitiatedNavigationEpoch();
-
   Maybe<IPCClientInfo> ipcClientInfo;
   if (mInitialClientInfo.isSome()) {
     ipcClientInfo.emplace(mInitialClientInfo.ref().ToIPC());
   }
-  args.initialClientInfo() = ipcClientInfo;
-
-  if (mTiming) {
-    args.timing() = Some(mTiming);
-  }
 
+  DocumentChannelElementCreationArgs ipcElementCreationArgs;
   switch (mLoadInfo->GetExternalContentPolicyType()) {
     case ExtContentPolicy::TYPE_DOCUMENT:
     case ExtContentPolicy::TYPE_SUBDOCUMENT: {
       DocumentCreationArgs docArgs;
       docArgs.uriModified() = mUriModified;
       docArgs.isXFOError() = mIsXFOError;
 
-      args.elementCreationArgs() = docArgs;
+      ipcElementCreationArgs = docArgs;
       break;
     }
 
@@ -145,7 +132,7 @@ DocumentChannelChild::AsyncOpen(nsIStreamListener* aListener) {
       objectArgs.contentPolicyType() = mLoadInfo->InternalContentPolicyType();
       objectArgs.isUrgentStart() = UserActivation::IsHandlingUserInput();
 
-      args.elementCreationArgs() = objectArgs;
+      ipcElementCreationArgs = objectArgs;
       break;
     }
 
@@ -165,6 +152,11 @@ DocumentChannelChild::AsyncOpen(nsIStreamListener* aListener) {
       break;
   }
 
+  DocumentChannelCreationArgs args(
+      mozilla::WrapNotNull(mLoadState), TimeStamp::Now(), mChannelId, mCacheKey,
+      mTiming, ipcClientInfo, ipcElementCreationArgs,
+      loadingContext->GetParentInitiatedNavigationEpoch());
+
   gNeckoChild->SendPDocumentChannelConstructor(this, loadingContext, args);
 
   mIsPending = true;

netwerk/ipc/DocumentChannelParent.cpp

@@ -68,19 +68,18 @@ bool DocumentChannelParent::Init(dom::CanonicalBrowsingContext* aContext,
 
       promise = mDocumentLoadListener->OpenDocument(
           loadState, aArgs.cacheKey(), Some(aArgs.channelId()),
-          aArgs.asyncOpenTime(), aArgs.timing().refOr(nullptr),
-          std::move(clientInfo), Some(docArgs.uriModified()),
-          Some(docArgs.isXFOError()), contentParent, &rv);
+          aArgs.asyncOpenTime(), aArgs.timing(), std::move(clientInfo),
+          Some(docArgs.uriModified()), Some(docArgs.isXFOError()),
+          contentParent, &rv);
     } else {
       const ObjectCreationArgs& objectArgs = aArgs.elementCreationArgs();
 
       promise = mDocumentLoadListener->OpenObject(
           loadState, aArgs.cacheKey(), Some(aArgs.channelId()),
-          aArgs.asyncOpenTime(), aArgs.timing().refOr(nullptr),
-          std::move(clientInfo), objectArgs.embedderInnerWindowId(),
-          objectArgs.loadFlags(), objectArgs.contentPolicyType(),
-          objectArgs.isUrgentStart(), contentParent,
-          this /* ObjectUpgradeHandler */, &rv);
+          aArgs.asyncOpenTime(), aArgs.timing(), std::move(clientInfo),
+          objectArgs.embedderInnerWindowId(), objectArgs.loadFlags(),
+          objectArgs.contentPolicyType(), objectArgs.isUrgentStart(),
+          contentParent, this /* ObjectUpgradeHandler */, &rv);
     }
 
     if (NS_FAILED(rv)) {

netwerk/ipc/DocumentLoadListener.cpp

@@ -2099,7 +2099,7 @@ DocumentLoadListener::RedirectToRealChannel(
                           aLoadFlags, cp, std::move(ehArgs));
     if (mTiming) {
       mTiming->Anonymize(args.uri());
-      args.timing() = Some(std::move(mTiming));
+      args.timing() = std::move(mTiming);
     }
 
     auto loadInfo = args.loadInfo();

netwerk/ipc/NeckoChannelParams.ipdlh

@@ -453,11 +453,11 @@ union DocumentChannelElementCreationArgs {
 };
 
 struct DocumentChannelCreationArgs {
-  nullable nsDocShellLoadState loadState;
+  nsDocShellLoadState loadState;
   TimeStamp asyncOpenTime;
   uint64_t channelId;
   uint32_t cacheKey;
-  nullable nsDOMNavigationTiming? timing;
+  nullable nsDOMNavigationTiming timing;
   IPCClientInfo? initialClientInfo;
   DocumentChannelElementCreationArgs elementCreationArgs;
   uint64_t parentInitiatedNavigationEpoch;
@@ -484,7 +484,7 @@ struct RedirectToRealChannelArgs {
   uint32_t loadStateExternalLoadFlags;
   uint32_t loadStateInternalLoadFlags;
   uint32_t loadStateLoadType;
-  nullable nsDOMNavigationTiming? timing;
+  nullable nsDOMNavigationTiming timing;
   nsString srcdocData;
   nullable nsIURI baseUri;
   LoadingSessionHistoryInfo? loadingSessionHistoryInfo;