Bug 1379762 part 2. Use a more reliable test to figure out when we can skip firing onload in nsDocumentViewer::LoadComplete. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 17 Jul 2017 23:21:41 -0400
changeset 418110 aa65fd52fbcc7c6f7dc6111ad89471df9f38d9f8
parent 418109 68dccd8c146891ac1dd846bb0fde903351303de3
child 418111 5fc778386eb1e7ff66714919535580bb5f347efd
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1379762
milestone56.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 1379762 part 2. Use a more reliable test to figure out when we can skip firing onload in nsDocumentViewer::LoadComplete. r=smaug Unfortunately, GetRestoringDocument can be false by the time we reach LoadComplete, if part of the restoration process managed to set up and then remove onload blockers. If that happens, we still don't want to fire a load event for a document that has already has one fired. Note that we could also use a boolean on the document to record whether we've fired a load event, as long as we were careful to unset it when the readyState transitions backwards from COMPLETE (e.g. document.open). It's not clear which approach is more robust.
docshell/test/navigation/file_bug1379762-2.html
docshell/test/navigation/mochitest.ini
docshell/test/navigation/test_sessionhistory.html
layout/base/nsDocumentViewer.cpp
new file mode 100644
--- /dev/null
+++ b/docshell/test/navigation/file_bug1379762-2.html
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="utf-8">
+    <title>Bug 1379762</title>
+  </head>
+  <script type="text/just-data">
+    onunload = null; // enable bfcache
+    ++opener.testCount;
+    onpageshow = function(e) {
+      opener.ok(!e.persisted, "Pageshow should not be coming from bfcache " + opener.testCount);
+    }
+    if (opener.testCount == 1) {
+      onload = function () {
+        setTimeout(function() {
+          document.write(testScript);
+        }, 0);
+      }
+    } else if (opener.testCount == 2) {
+      // Do this async, just in case.
+      setTimeout(function() {
+        history.back();
+      }, 0);
+    } else if (opener.testCount == 3) {
+      // Do this async, just in case.
+      setTimeout(function() {
+        history.forward();
+      }, 0);
+    } else if (opener.testCount == 4) {
+      onload = function() {
+        opener.nextTest();
+        window.close();
+      }
+    }
+  </script>
+  <script>
+    var data = document.querySelector("script[type='text/just-data']").textContent;
+    // Store the string that does all out work in a global variable, so we can
+    // get at it later.
+    var testScript = "<script>" + data + "</" + "script>";
+    document.write(testScript);
+  </script>
+</html>
--- a/docshell/test/navigation/mochitest.ini
+++ b/docshell/test/navigation/mochitest.ini
@@ -59,16 +59,16 @@ skip-if = (toolkit == 'android') || (!de
 [test_grandchild.html]
 [test_not-opener.html]
 [test_opener.html]
 [test_popup-navigates-children.html]
 [test_reserved.html]
 skip-if = (toolkit == 'android') || (debug && e10s) #too slow on Android 4.3 aws only; bug 1030403; bug 1263213 for debug e10s
 [test_sessionhistory.html]
 skip-if = toolkit == 'android' #RANDOM
-support-files = file_bug1379762-1.html
+support-files = file_bug1379762-1.html file_bug1379762-2.html
 [test_sibling-matching-parent.html]
 [test_sibling-off-domain.html]
 [test_triggeringprincipal_frame_nav.html]
 [test_triggeringprincipal_window_open.html]
 [test_triggeringprincipal_parent_iframe_window_open.html]
 [test_triggeringprincipal_iframe_iframe_window_open.html]
 [test_contentpolicy_block_window.html]
--- a/docshell/test/navigation/test_sessionhistory.html
+++ b/docshell/test/navigation/test_sessionhistory.html
@@ -29,16 +29,17 @@ var testFiles =
     "file_bug534178.html",           // Session history transaction clean-up.
     "file_fragment_handling_during_load.html",
     "file_nested_frames.html",
     "file_shiftReload_and_pushState.html",
     "file_scrollRestoration.html",
     "file_bug1300461.html",
     "file_bug1326251.html",
     "file_bug1379762-1.html",
+    "file_bug1379762-2.html",
   ];
 var testCount = 0; // Used by the test files.
 
 SimpleTest.waitForExplicitFinish();
 SimpleTest.requestFlakyTimeout("untriaged");
 
 var testWindow;
 function nextTest_() {
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -1058,27 +1058,40 @@ nsDocumentViewer::LoadComplete(nsresult 
 
     // If the document presentation is being restored, we don't want to fire
     // onload to the document content since that would likely confuse scripts
     // on the page.
 
     nsIDocShell *docShell = window->GetDocShell();
     NS_ENSURE_TRUE(docShell, NS_ERROR_UNEXPECTED);
 
-    docShell->GetRestoringDocument(&restoring);
+    // Unfortunately, docShell->GetRestoringDocument() might no longer be set
+    // correctly.  In particular, it can be false by now if someone took it upon
+    // themselves to block onload from inside restoration and unblock it later.
+    // But we can detect the restoring case very simply: by whether our
+    // document's readyState is COMPLETE.
+    restoring = (mDocument->GetReadyStateEnum() ==
+                 nsIDocument::READYSTATE_COMPLETE);
     if (!restoring) {
       NS_ASSERTION(mDocument->IsXULDocument() || // readyState for XUL is bogus
                    mDocument->GetReadyStateEnum() ==
                      nsIDocument::READYSTATE_INTERACTIVE ||
                    // test_stricttransportsecurity.html has old-style
                    // docshell-generated about:blank docs reach this code!
                    (mDocument->GetReadyStateEnum() ==
                       nsIDocument::READYSTATE_UNINITIALIZED &&
                     NS_IsAboutBlank(mDocument->GetDocumentURI())),
                    "Bad readystate");
+#ifdef DEBUG
+      bool docShellThinksWeAreRestoring;
+      docShell->GetRestoringDocument(&docShellThinksWeAreRestoring);
+      MOZ_ASSERT(!docShellThinksWeAreRestoring,
+                 "How can docshell think we are restoring if we don't have a "
+                 "READYSTATE_COMPLETE document?");
+#endif // DEBUG
       nsCOMPtr<nsIDocument> d = mDocument;
       mDocument->SetReadyStateInternal(nsIDocument::READYSTATE_COMPLETE);
 
       RefPtr<nsDOMNavigationTiming> timing(d->GetNavigationTiming());
       if (timing) {
         timing->NotifyLoadEventStart();
       }