Bug 1550524 part 2. Add an explicit StartExecutor method on nsHtml5Parser, for use from document.open(). r=hsivonen
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 14 May 2019 19:46:26 +0000
changeset 474554 3eef21fc64b778f57aaa416f2b0c2ed469033a10
parent 474553 0c1f4aee5de04bbef034b98f39c4e367b34cc5ba
child 474555 68b3e0acc50371007c682e55c79158d98d6325ae
push id36042
push userdvarga@mozilla.com
push dateTue, 21 May 2019 04:19:40 +0000
treeherdermozilla-central@ca560ff55451 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershsivonen
bugs1550524
milestone69.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 1550524 part 2. Add an explicit StartExecutor method on nsHtml5Parser, for use from document.open(). r=hsivonen The code has mostly moved, but there are a few simplifications: 1) If !GetStreamParser(), then GetChannel() always returns null and hence we never set isSrcdoc to true. Which is good, because we don't want to apply the special srcdoc-parsing rules to document.open() stuff. So we just pass false to setIsSrcdocDocument(): It's the same behavior as before, but a lot clearer. I've confirmed that code coverage says the "isSrcdoc = NS_IsSrcdocChannel(channel)" line is unreached in our tests. 2) In the document.write-after-document.open case, aContentType is now always "text/html" (because that's what document.open sets mContentTypeForWriteCalls to. So the block checking for it not being "text/html" was dead code (also confirmed via code coverage results) and I'm just removing it. Differential Revision: https://phabricator.services.mozilla.com/D30751
dom/html/crashtests/1550524.html
dom/html/crashtests/crashtests.list
dom/html/nsHTMLDocument.cpp
parser/html/nsHtml5Parser.cpp
parser/html/nsHtml5Parser.h
new file mode 100644
--- /dev/null
+++ b/dom/html/crashtests/1550524.html
@@ -0,0 +1,7 @@
+<iframe></iframe>
+<script>
+  var doc = frames[0].document;
+  doc.open();
+  doc.open();
+  doc.close();
+</script>
--- a/dom/html/crashtests/crashtests.list
+++ b/dom/html/crashtests/crashtests.list
@@ -84,10 +84,11 @@ load 1343886-1.html
 load 1343886-2.xml
 load 1343886-3.xml
 load 1350972.html
 load 1386905.html
 asserts(0-4) load 1401726.html
 load 1412173.html
 load 1440523.html
 load 1547057.html
+load 1550524.html
 load 1550881-1.html
 load 1550881-2.html
--- a/dom/html/nsHTMLDocument.cpp
+++ b/dom/html/nsHTMLDocument.cpp
@@ -1188,16 +1188,21 @@ Document* nsHTMLDocument::Open(const Opt
     // start with the new referrer policy.
     nsHtml5TreeOpExecutor* executor = nullptr;
     executor = static_cast<nsHtml5TreeOpExecutor*>(mParser->GetContentSink());
     if (executor && mReferrerPolicySet) {
       executor->SetSpeculationReferrerPolicy(
           static_cast<ReferrerPolicy>(mReferrerPolicy));
     }
   }
+  nsresult rv = parser->StartExecutor();
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    aError.Throw(rv);
+    return nullptr;
+  }
 
   // Some internal non-spec bookkeeping.
   mContentTypeForWriteCalls.AssignLiteral("text/html");
 
   if (shell) {
     // Prepare the docshell and the document viewer for the impending
     // out-of-band document.write()
     shell->PrepareForNewContentModel();
--- a/parser/html/nsHtml5Parser.cpp
+++ b/parser/html/nsHtml5Parser.cpp
@@ -190,45 +190,17 @@ nsresult nsHtml5Parser::Parse(const nsAS
   nsCOMPtr<nsIParser> kungFuDeathGrip(this);
 
   // Gripping the other objects just in case, since the other old grip
   // required grips to these, too.
   RefPtr<nsHtml5StreamParser> streamKungFuDeathGrip(GetStreamParser());
   mozilla::Unused << streamKungFuDeathGrip;  // Not used within function
   RefPtr<nsHtml5TreeOpExecutor> executor(mExecutor);
 
-  if (!executor->HasStarted()) {
-    NS_ASSERTION(!GetStreamParser(),
-                 "Had stream parser but document.write started life cycle.");
-    // This is the first document.write() on a document.open()ed document
-    executor->SetParser(this);
-    mTreeBuilder->setScriptingEnabled(executor->IsScriptEnabled());
-
-    bool isSrcdoc = false;
-    nsCOMPtr<nsIChannel> channel;
-    rv = GetChannel(getter_AddRefs(channel));
-    if (NS_SUCCEEDED(rv)) {
-      isSrcdoc = NS_IsSrcdocChannel(channel);
-    }
-    mTreeBuilder->setIsSrcdocDocument(isSrcdoc);
-
-    mTokenizer->start();
-    executor->Start();
-    if (!aContentType.EqualsLiteral("text/html")) {
-      mTreeBuilder->StartPlainText();
-      mTokenizer->StartPlainText();
-    }
-    /*
-     * If you move the following line, be very careful not to cause
-     * WillBuildModel to be called before the document has had its
-     * script global object set.
-     */
-    rv = executor->WillBuildModel(eDTDMode_unknown);
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
+  MOZ_RELEASE_ASSERT(executor->HasStarted());
 
   // Return early if the parser has processed EOF
   if (executor->IsComplete()) {
     return NS_OK;
   }
 
   if (aLastCall && aSourceBuffer.IsEmpty() && !aKey) {
     // document.close()
@@ -665,16 +637,36 @@ nsresult nsHtml5Parser::ParseUntilBlocke
       }
       if (mBlocked) {
         return NS_OK;
       }
     }
   }
 }
 
+nsresult nsHtml5Parser::StartExecutor() {
+  MOZ_ASSERT(!GetStreamParser(),
+             "Had stream parser but document.write started life cycle.");
+  // This is part of the setup document.open() does.
+  RefPtr<nsHtml5TreeOpExecutor> executor(mExecutor);
+  executor->SetParser(this);
+  mTreeBuilder->setScriptingEnabled(executor->IsScriptEnabled());
+
+  mTreeBuilder->setIsSrcdocDocument(false);
+
+  mTokenizer->start();
+  executor->Start();
+
+  /*
+   * We know we're in document.open(), so our document must already
+   * have a script global andthe WillBuildModel call is safe.
+   */
+  return executor->WillBuildModel(eDTDMode_unknown);
+}
+
 nsresult nsHtml5Parser::Initialize(mozilla::dom::Document* aDoc, nsIURI* aURI,
                                    nsISupports* aContainer,
                                    nsIChannel* aChannel) {
   return mExecutor->Init(aDoc, aURI, aContainer, aChannel);
 }
 
 void nsHtml5Parser::StartTokenizer(bool aScriptingEnabled) {
   bool isSrcdoc = false;
--- a/parser/html/nsHtml5Parser.h
+++ b/parser/html/nsHtml5Parser.h
@@ -249,16 +249,23 @@ class nsHtml5Parser final : public nsIPa
     mInsertionPointPermanentlyUndefined = true;
   }
 
   /**
    * Parse until pending data is exhausted or a script blocks the parser
    */
   nsresult ParseUntilBlocked();
 
+  /**
+   * Start our executor.  This is meant to be used from document.open() _only_
+   * and does some work similar to what nsHtml5StreamParser::OnStartRequest does
+   * for normal parses.
+   */
+  nsresult StartExecutor();
+
  private:
   virtual ~nsHtml5Parser();
 
   // State variables
 
   /**
    * Whether the last character tokenized was a carriage return (for CRLF)
    */