Bug 325352 part 2. Don't no-op document.open if our parser has a script nesting level equal to 0. r=hsivonen
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 08 May 2019 16:03:37 +0000
changeset 531911 264fe248bca56c113116bd5cfde490ebb554e56a
parent 531910 990c8a382cf38762b2d81bbcba3c56e1b9920bec
child 531912 0ec8134b1b62b3a3115e3b8ae939b49c1340dfc6
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershsivonen
bugs325352
milestone68.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 325352 part 2. Don't no-op document.open if our parser has a script nesting level equal to 0. r=hsivonen Differential Revision: https://phabricator.services.mozilla.com/D30314
dom/html/nsHTMLDocument.cpp
testing/web-platform/tests/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/parser-no-script-nesting.window.js
--- a/dom/html/nsHTMLDocument.cpp
+++ b/dom/html/nsHTMLDocument.cpp
@@ -1010,35 +1010,33 @@ Document* nsHTMLDocument::Open(const Opt
 
   // Step 4 -- make sure we're same-origin (not just same origin-domain) with
   // the entry document.
   if (!callerDoc->NodePrincipal()->Equals(NodePrincipal())) {
     aError.Throw(NS_ERROR_DOM_SECURITY_ERR);
     return nullptr;
   }
 
-  // Step 5 -- if we have an active parser, just no-op.
-  // If we already have a parser we ignore the document.open call.
+  // Step 5 -- if we have an active parser with a nonzero script nesting level,
+  // just no-op.
+  //
+  // If we have a parser that has a zero script nesting level, we need to
+  // properly terminate it before setting up a new parser.  See the similar code
+  // in WriteCommon that handles the !IsInsertionPointDefined() case and should
+  // stay in sync with this code.
+  if (mParser && !mParser->HasNonzeroScriptNestingLevel()) {
+    // Make sure we don't re-enter.
+    IgnoreOpensDuringUnload ignoreOpenGuard(this);
+    mParser->Terminate();
+    MOZ_RELEASE_ASSERT(!mParser, "mParser should have been null'd out");
+  }
+
+  // The mParserAborted check here is probably wrong.  Removing it is
+  // tracked in https://bugzilla.mozilla.org/show_bug.cgi?id=1475000
   if (mParser || mParserAborted) {
-    // The WHATWG spec used to say: "If the document has an active parser that
-    // isn't a script-created parser, and the insertion point associated with
-    // that parser's input stream is not undefined (that is, it does point to
-    // somewhere in the input stream), then the method does nothing. Abort these
-    // steps and return the Document object on which the method was invoked."
-    // Note that aborting a parser leaves the parser "active" with its insertion
-    // point "not undefined". We track this using mParserAborted, because
-    // aborting a parser nulls out mParser.
-    //
-    // Actually, the spec says something slightly different now, about having
-    // an "active parser whose script nesting level is greater than 0".  It
-    // does not mention insertion points at all.  Not sure whether it matters
-    // in practice.  It seems like "script nesting level" replaced the
-    // insertion point concept?  Anyway,
-    // https://bugzilla.mozilla.org/show_bug.cgi?id=1475000 is probably
-    // relevant here.
     return this;
   }
 
   // Step 6 -- check for open() during unload.  Per spec, this is just a check
   // of the ignore-opens-during-unload counter, but our unload event code
   // doesn't affect that counter yet (unlike pagehide and beforeunload, which
   // do), so we check for unload directly.
   if (ShouldIgnoreOpens()) {
@@ -1318,17 +1316,19 @@ void nsHTMLDocument::WriteCommon(const n
       // Instead of implying a call to document.open(), ignore the call.
       nsContentUtils::ReportToConsole(
           nsIScriptError::warningFlag, NS_LITERAL_CSTRING("DOM Events"), this,
           nsContentUtils::eDOM_PROPERTIES, "DocumentWriteIgnored", nullptr, 0,
           mDocumentURI);
       return;
     }
     // The spec doesn't tell us to ignore opens from here, but we need to
-    // ensure opens are ignored here.
+    // ensure opens are ignored here.  See similar code in Open() that handles
+    // the case of an existing parser which is not currently running script and
+    // should stay in sync with this code.
     IgnoreOpensDuringUnload ignoreOpenGuard(this);
     mParser->Terminate();
     MOZ_RELEASE_ASSERT(!mParser, "mParser should have been null'd out");
   }
 
   if (!mParser) {
     if (mIgnoreDestructiveWritesCounter) {
       // Instead of implying a call to document.open(), ignore the call.
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/parser-no-script-nesting.window.js
@@ -0,0 +1,18 @@
+async_test(t => {
+  const frame = document.createElement("iframe");
+  frame.onload = t.step_func_done(() => {
+    frame.onload = null;
+    const doc = frame.contentDocument;
+
+    // Each open() call should reset the DOM for the document, because even
+    // though there is a live parser, that parser's script insertion level is 0.
+    for (var i = 0; i < 5; ++i) {
+      doc.open();
+      doc.write("hello");
+    }
+    doc.close()
+    assert_equals(doc.documentElement.textContent, "hello");
+  });
+  t.add_cleanup(() => frame.remove());
+  document.body.appendChild(frame);
+}, "open() should reset the DOM even if there's an active parser, if that parser's script insertion level is 0");