Bug 1550811. Back out the second part of the fix for bug 325352 until we have a fix for the crash issues it causes. r=hsivonen
authorBoris Zbarsky <bzbarsky@mit.edu>
Sat, 11 May 2019 13:52:27 +0000
changeset 532358 08eb61c337951a2f136faa08b029e34234aae08b
parent 532357 464bde42a1588d89f7822d175f86db7f93d211ea
child 532359 6103006e3172f0ef258a30256b8cc7a6553e7f14
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
bugs1550811, 325352
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 1550811. Back out the second part of the fix for bug 325352 until we have a fix for the crash issues it causes. r=hsivonen Differential Revision: https://phabricator.services.mozilla.com/D30682
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,33 +1010,35 @@ 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 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
+  // Step 5 -- if we have an active parser, just no-op.
+  // If we already have a parser we ignore the document.open call.
   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()) {
@@ -1316,19 +1318,17 @@ 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.  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.
+    // ensure opens are ignored here.
     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.
deleted file mode 100644
--- a/testing/web-platform/tests/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/parser-no-script-nesting.window.js
+++ /dev/null
@@ -1,18 +0,0 @@
-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");