Bug 1204626 - Reparent PWebBrowserPersistDocument to PContent. r=billm

A reference to a document (and the act of saving it) can outlive the
browser tab it was originally loaded in, and this needs to be reflected
in IPC in order to avoid MsgRouteError crashes; see bug for more info.

Note that we still need to pass the PBrowser when starting persistence,
because that's the only handle the parent has on the top-level document;
see comments in this patch for more info.

Also makes TabChildBase::GetDocument public, because it's just a wrapper
around WebNavigation() which is already public, to avoid code duplication.

dom/ipc/ContentChild.cpp

@@ -53,6 +53,7 @@
 #include "mozilla/widget/WidgetMessageUtils.h"
 #include "mozilla/media/MediaChild.h"
 #include "mozilla/BasePrincipal.h"
+#include "mozilla/WebBrowserPersistDocumentChild.h"
 
 #if defined(MOZ_CONTENT_SANDBOX)
 #if defined(XP_WIN)
@@ -2762,6 +2763,45 @@ ContentChild::DeallocPContentPermissionRequestChild(PContentPermissionRequestChi
     return true;
 }
 
+PWebBrowserPersistDocumentChild*
+ContentChild::AllocPWebBrowserPersistDocumentChild(PBrowserChild* aBrowser,
+                                                   const uint64_t& aOuterWindowID)
+{
+  return new WebBrowserPersistDocumentChild();
+}
+
+bool
+ContentChild::RecvPWebBrowserPersistDocumentConstructor(PWebBrowserPersistDocumentChild *aActor,
+                                                        PBrowserChild* aBrowser,
+                                                        const uint64_t& aOuterWindowID)
+{
+  if (NS_WARN_IF(!aBrowser)) {
+    return false;
+  }
+  nsCOMPtr<nsIDocument> rootDoc =
+    static_cast<TabChild*>(aBrowser)->GetDocument();
+  nsCOMPtr<nsIDocument> foundDoc;
+  if (aOuterWindowID) {
+    foundDoc = nsContentUtils::GetSubdocumentWithOuterWindowId(rootDoc, aOuterWindowID);
+  } else {
+    foundDoc = rootDoc;
+  }
+
+  if (!foundDoc) {
+    aActor->SendInitFailure(NS_ERROR_NO_CONTENT);
+  } else {
+    static_cast<WebBrowserPersistDocumentChild*>(aActor)->Start(foundDoc);
+  }
+  return true;
+}
+
+bool
+ContentChild::DeallocPWebBrowserPersistDocumentChild(PWebBrowserPersistDocumentChild* aActor)
+{
+  delete aActor;
+  return true;
+}
+
 bool
 ContentChild::RecvGamepadUpdate(const GamepadChangeEvent& aGamepadEvent)
 {

dom/ipc/ContentChild.h

@@ -204,6 +204,12 @@ public:
                                          const FileDescriptor& aGCLog,
                                          const FileDescriptor& aCCLog) override;
 
+    virtual PWebBrowserPersistDocumentChild* AllocPWebBrowserPersistDocumentChild(PBrowserChild* aBrowser, const uint64_t& aOuterWindowID) override;
+    virtual bool RecvPWebBrowserPersistDocumentConstructor(PWebBrowserPersistDocumentChild *aActor,
+                                                           PBrowserChild *aBrowser,
+                                                           const uint64_t& aOuterWindowID) override;
+    virtual bool DeallocPWebBrowserPersistDocumentChild(PWebBrowserPersistDocumentChild* aActor) override;
+
     virtual bool
     RecvDataStoreNotify(const uint32_t& aAppId, const nsString& aName,
                         const nsString& aManifestURL) override;

dom/ipc/ContentParent.cpp

@@ -94,6 +94,7 @@
 #include "mozilla/Services.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/Telemetry.h"
+#include "mozilla/WebBrowserPersistDocumentParent.h"
 #include "mozilla/unused.h"
 #include "nsAnonymousTemporaryFile.h"
 #include "nsAppRunner.h"
@@ -5260,6 +5261,20 @@ ContentParent::DeallocPContentPermissionRequestParent(PContentPermissionRequestP
     return true;
 }
 
+PWebBrowserPersistDocumentParent*
+ContentParent::AllocPWebBrowserPersistDocumentParent(PBrowserParent* aBrowser,
+                                                     const uint64_t& aOuterWindowID)
+{
+  return new WebBrowserPersistDocumentParent();
+}
+
+bool
+ContentParent::DeallocPWebBrowserPersistDocumentParent(PWebBrowserPersistDocumentParent* aActor)
+{
+  delete aActor;
+  return true;
+}
+
 /* static */ bool
 ContentParent::PermissionManagerAddref(const ContentParentId& aCpId,
                                        const TabId& aTabId)

dom/ipc/ContentParent.h

@@ -710,6 +710,9 @@ private:
     virtual bool DeallocPSpeechSynthesisParent(PSpeechSynthesisParent* aActor) override;
     virtual bool RecvPSpeechSynthesisConstructor(PSpeechSynthesisParent* aActor) override;
 
+    virtual PWebBrowserPersistDocumentParent* AllocPWebBrowserPersistDocumentParent(PBrowserParent* aBrowser, const uint64_t& aOuterWindowID) override;
+    virtual bool DeallocPWebBrowserPersistDocumentParent(PWebBrowserPersistDocumentParent* aActor) override;
+
     virtual bool RecvReadPrefsArray(InfallibleTArray<PrefSetting>* aPrefs) override;
     virtual bool RecvReadFontList(InfallibleTArray<FontListEntry>* retValue) override;
 

dom/ipc/PBrowser.ipdl

@@ -15,7 +15,6 @@ include protocol PFilePicker;
 include protocol PIndexedDBPermissionRequest;
 include protocol PRenderFrame;
 include protocol PPluginWidget;
-include protocol PWebBrowserPersistDocument;
 include DOMTypes;
 include JavaScriptTypes;
 include URIParams;
@@ -111,7 +110,6 @@ prio(normal upto urgent) sync protocol PBrowser
     manages PIndexedDBPermissionRequest;
     manages PRenderFrame;
     manages PPluginWidget;
-    manages PWebBrowserPersistDocument;
 
 both:
     AsyncMessage(nsString aMessage, ClonedMessageData aData, CpowEntry[] aCpows,
@@ -123,8 +121,6 @@ both:
      */
     PRenderFrame();
 
-    PWebBrowserPersistDocument(uint64_t aOuterWindowID);
-
 parent:
     /**
      * Tell the parent process a new accessible document has been created.

dom/ipc/PContent.ipdl

@@ -50,6 +50,7 @@ include protocol PTestShell;
 include protocol PVoicemail;
 include protocol PJavaScript;
 include protocol PRemoteSpellcheckEngine;
+include protocol PWebBrowserPersistDocument;
 include protocol PWebrtcGlobal;
 include protocol PPresentation;
 include DOMTypes;
@@ -450,6 +451,7 @@ prio(normal upto urgent) sync protocol PContent
     manages PVoicemail;
     manages PJavaScript;
     manages PRemoteSpellcheckEngine;
+    manages PWebBrowserPersistDocument;
     manages PWebrtcGlobal;
     manages PPresentation;
 
@@ -486,6 +488,13 @@ both:
 
     PFileDescriptorSet(FileDescriptor fd);
 
+    // For parent->child, aBrowser must be non-null; aOuterWindowID can
+    // be 0 to indicate the browser's current root document, or nonzero
+    // to persist a subdocument.  For child->parent, arguments are
+    // ignored and should be null/zero.
+    PWebBrowserPersistDocument(nullable PBrowser aBrowser,
+                               uint64_t aOuterWindowID);
+
 child:
     /**
      * Enable system-level sandboxing features, if available.  Can

dom/ipc/TabChild.cpp

@@ -38,7 +38,6 @@
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/Move.h"
-#include "mozilla/PWebBrowserPersistDocumentChild.h"
 #include "mozilla/Services.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/TextEvents.h"
@@ -103,7 +102,6 @@
 #include "nsIScriptError.h"
 #include "mozilla/EventForwards.h"
 #include "nsDeviceContext.h"
-#include "mozilla/WebBrowserPersistDocumentChild.h"
 
 #define BROWSER_ELEMENT_CHILD_SCRIPT \
     NS_LITERAL_STRING("chrome://global/content/BrowserElementChild.js")
@@ -3237,36 +3235,3 @@ TabChildGlobal::GetGlobalJSObject()
   NS_ENSURE_TRUE(ref, nullptr);
   return ref->GetJSObject();
 }
-
-PWebBrowserPersistDocumentChild*
-TabChild::AllocPWebBrowserPersistDocumentChild(const uint64_t& aOuterWindowID)
-{
-  return new WebBrowserPersistDocumentChild();
-}
-
-bool
-TabChild::RecvPWebBrowserPersistDocumentConstructor(PWebBrowserPersistDocumentChild *aActor,
-                                                    const uint64_t& aOuterWindowID)
-{
-  nsCOMPtr<nsIDocument> rootDoc = GetDocument();
-  nsCOMPtr<nsIDocument> foundDoc;
-  if (aOuterWindowID) {
-    foundDoc = nsContentUtils::GetSubdocumentWithOuterWindowId(rootDoc, aOuterWindowID);
-  } else {
-    foundDoc = rootDoc;
-  }
-
-  if (!foundDoc) {
-    aActor->SendInitFailure(NS_ERROR_NO_CONTENT);
-  } else {
-    static_cast<WebBrowserPersistDocumentChild*>(aActor)->Start(foundDoc);
-  }
-  return true;
-}
-
-bool
-TabChild::DeallocPWebBrowserPersistDocumentChild(PWebBrowserPersistDocumentChild* aActor)
-{
-  delete aActor;
-  return true;
-}

dom/ipc/TabChild.h

@@ -188,11 +188,12 @@ public:
 
     virtual ScreenIntSize GetInnerSize() = 0;
 
+    // Get the Document for the top-level window in this tab.
+    already_AddRefed<nsIDocument> GetDocument() const;
+
 protected:
     virtual ~TabChildBase();
 
-    // Get the Document for the top-level window in this tab.
-    already_AddRefed<nsIDocument> GetDocument() const;
     // Get the pres-shell of the document for the top-level window in this tab.
     already_AddRefed<nsIPresShell> GetPresShell() const;
 
@@ -503,11 +504,6 @@ public:
 
     virtual ScreenIntSize GetInnerSize() override;
 
-    virtual PWebBrowserPersistDocumentChild* AllocPWebBrowserPersistDocumentChild(const uint64_t& aOuterWindowID) override;
-    virtual bool RecvPWebBrowserPersistDocumentConstructor(PWebBrowserPersistDocumentChild *aActor,
-                                                           const uint64_t& aOuterWindowID) override;
-    virtual bool DeallocPWebBrowserPersistDocumentChild(PWebBrowserPersistDocumentChild* aActor) override;
-
 protected:
     virtual ~TabChild();
 

dom/ipc/TabParent.cpp

@@ -3492,26 +3492,18 @@ TabParent::AsyncPanZoomEnabled() const
   return widget && widget->AsyncPanZoomEnabled();
 }
 
-PWebBrowserPersistDocumentParent*
-TabParent::AllocPWebBrowserPersistDocumentParent(const uint64_t& aOuterWindowID)
-{
-  return new WebBrowserPersistDocumentParent();
-}
-
-bool
-TabParent::DeallocPWebBrowserPersistDocumentParent(PWebBrowserPersistDocumentParent* aActor)
-{
-  delete aActor;
-  return true;
-}
-
 NS_IMETHODIMP
 TabParent::StartPersistence(uint64_t aOuterWindowID,
                             nsIWebBrowserPersistDocumentReceiver* aRecv)
 {
+  nsCOMPtr<nsIContentParent> manager = Manager();
+  if (!manager->IsContentParent()) {
+    return NS_ERROR_UNEXPECTED;
+  }
   auto* actor = new WebBrowserPersistDocumentParent();
   actor->SetOnReady(aRecv);
-  return SendPWebBrowserPersistDocumentConstructor(actor, aOuterWindowID)
+  return manager->AsContentParent()
+    ->SendPWebBrowserPersistDocumentConstructor(actor, this, aOuterWindowID)
     ? NS_OK : NS_ERROR_FAILURE;
   // (The actor will be destroyed on constructor failure.)
 }

dom/ipc/TabParent.h

@@ -442,9 +442,6 @@ public:
                                int32_t& aDragAreaX, int32_t& aDragAreaY);
     layout::RenderFrameParent* GetRenderFrame();
 
-    virtual PWebBrowserPersistDocumentParent* AllocPWebBrowserPersistDocumentParent(const uint64_t& aOuterWindowID) override;
-    virtual bool DeallocPWebBrowserPersistDocumentParent(PWebBrowserPersistDocumentParent* aActor) override;
-
 protected:
     bool ReceiveMessage(const nsString& aMessage,
                         bool aSync,

embedding/components/webbrowserpersist/PWebBrowserPersistDocument.ipdl

@@ -3,7 +3,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-include protocol PBrowser;
+include protocol PContent;
 include protocol PWebBrowserPersistResources;
 include protocol PWebBrowserPersistSerialize;
 
@@ -48,9 +48,9 @@ struct WebBrowserPersistURIMap {
 // child->parent and then passed back.  Subdocuments aren't subactors,
 // because that would impose a lifetime relationship that doesn't
 // exist in the XPIDL; instead they're all managed by the enclosing
-// PBrowser (== TabParent/TabChild).
+// PContent.
 protocol PWebBrowserPersistDocument {
-  manager PBrowser;
+  manager PContent;
   manages PWebBrowserPersistResources;
   manages PWebBrowserPersistSerialize;
 

embedding/components/webbrowserpersist/WebBrowserPersistResourcesChild.cpp

@@ -7,7 +7,7 @@
 #include "WebBrowserPersistResourcesChild.h"
 
 #include "WebBrowserPersistDocumentChild.h"
-#include "mozilla/dom/PBrowserChild.h"
+#include "mozilla/dom/ContentChild.h"
 
 namespace mozilla {
 
@@ -36,12 +36,14 @@ WebBrowserPersistResourcesChild::VisitDocument(nsIWebBrowserPersistDocument* aDo
                                                nsIWebBrowserPersistDocument* aSubDocument)
 {
     auto* subActor = new WebBrowserPersistDocumentChild();
-    dom::PBrowserChild* grandManager = Manager()->Manager();
-    // As a consequence of how PWebBrowserPersistDocumentConstructor can be
-    // sent by both the parent and the child, we must pass the outerWindowID
-    // argument here. We pass 0, though note that this argument is actually
-    // just ignored when passed up to the parent from the child.
-    if (!grandManager->SendPWebBrowserPersistDocumentConstructor(subActor, 0)) {
+    // As a consequence of how PWebBrowserPersistDocumentConstructor
+    // can be sent by both the parent and the child, we must pass the
+    // aBrowser and outerWindowID arguments here, but the values are
+    // ignored by the parent.  In particular, the TabChild in which
+    // persistence started does not necessarily exist at this point;
+    // see bug 1203602.
+    if (!Manager()->Manager()
+        ->SendPWebBrowserPersistDocumentConstructor(subActor, nullptr, 0)) {
         // NOTE: subActor is freed at this point.
         return NS_ERROR_FAILURE;
     }