Bug 1273850 - Add lots of documentation to explain the scenarios that cause nsFrameLoader::Create to return null. r=smaug
authorJonathan Watt <jwatt@jwatt.org>
Tue, 17 May 2016 14:18:22 +0100
changeset 298159 6523dd4df90d7598ec45574e8643b0dc7b8f37c3
parent 298158 f14bb70f1007497b2010f59e247cb5dfb4951b8f
child 298160 2aef443489891f7fa4bdf9f9b5206da6d9cfcb25
push id77048
push userjwatt@jwatt.org
push dateThu, 19 May 2016 17:47:38 +0000
treeherdermozilla-inbound@6523dd4df90d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1273850
milestone49.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 1273850 - Add lots of documentation to explain the scenarios that cause nsFrameLoader::Create to return null. r=smaug MozReview-Commit-ID: 6UZyql1qHg0
dom/base/nsFrameLoader.cpp
dom/base/nsIDocument.h
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -177,19 +177,40 @@ nsFrameLoader::~nsFrameLoader()
   MOZ_RELEASE_ASSERT(mDestroyCalled);
 }
 
 nsFrameLoader*
 nsFrameLoader::Create(Element* aOwner, bool aNetworkCreated)
 {
   NS_ENSURE_TRUE(aOwner, nullptr);
   nsIDocument* doc = aOwner->OwnerDoc();
+
+  // We never create nsFrameLoaders for elements in resource documents.
+  //
+  // We never create nsFrameLoaders for elements in data documents, unless the
+  // document is a static document.
+  // Static documents are an exception because any sub-documents need an
+  // nsFrameLoader to keep the relevant docShell alive, even though the
+  // nsFrameLoader isn't used to load anything (the sub-document is created by
+  // the static clone process).
+  //
+  // We never create nsFrameLoaders for elements that are not
+  // in-composed-document, unless the element belongs to a static document.
+  // Static documents are an exception because this method is called at a point
+  // in the static clone process before aOwner has been inserted into its
+  // document.  For other types of documents this wouldn't be a problem since
+  // we'd create the nsFrameLoader as necessary after aOwner is inserted into a
+  // document, but the mechanisms that take care of that don't apply for static
+  // documents so we need to create the nsFrameLoader now. (This isn't wasteful
+  // since for a static document we know aOwner will end up in a document and
+  // the nsFrameLoader will be used for its docShell.)
+  //
   NS_ENSURE_TRUE(!doc->IsResourceDoc() &&
-                 ((!doc->IsLoadedAsData() && aOwner->GetComposedDoc()) ||
-                   doc->IsStaticDocument()),
+                 ((!doc->IsLoadedAsData() && aOwner->IsInComposedDoc()) ||
+                  doc->IsStaticDocument()),
                  nullptr);
 
   return new nsFrameLoader(aOwner, aNetworkCreated);
 }
 
 NS_IMETHODIMP
 nsFrameLoader::LoadFrame()
 {
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -2058,23 +2058,35 @@ public:
 
   /**
    * Returns the template content owner document that owns the content of
    * HTMLTemplateElement.
    */
   virtual nsIDocument* GetTemplateContentsOwner() = 0;
 
   /**
-   * true when this document is a static clone of a normal document.
-   * For example print preview and printing use static documents.
+   * Returns true if this document is a static clone of a normal document.
+   *
+   * We create static clones for print preview and printing (possibly other
+   * things in future).
+   *
+   * Note that static documents are also "loaded as data" (if this method
+   * returns true, IsLoadedAsData() will also return true).
    */
   bool IsStaticDocument() { return mIsStaticDocument; }
 
   /**
-   * Clones the document and subdocuments and stylesheet etc.
+   * Clones the document along with any subdocuments, stylesheet, etc.
+   *
+   * The resulting document and everything it contains (including any
+   * sub-documents) are created purely via cloning.  The returned documents and
+   * any sub-documents are "loaded as data" documents to preserve the state as
+   * it was during the clone process (we don't want external resources to load
+   * and replace the cloned resources).
+   *
    * @param aCloneContainer The container for the clone document.
    */
   virtual already_AddRefed<nsIDocument>
   CreateStaticClone(nsIDocShell* aCloneContainer);
 
   /**
    * If this document is a static clone, this returns the original
    * document.