Bug 1519855 - Notify client about sources after BFCache navigations, r=jdescottes.

Differential Revision: https://phabricator.services.mozilla.com/D38440 --HG-- extra : moz-landing-system : lando
2019-08-03 03:12:02 +00:00 · 2019-08-03 03:12:02 +00:00 · 73b3de39bd
--- a/devtools/client/debugger/src/actions/navigation.js
+++ b/devtools/client/debugger/src/actions/navigation.js
@ -6,10 +6,7 @@

 import { clearDocuments } from "../utils/editor";
 import sourceQueue from "../utils/source-queue";
-import { getSourceList } from "../reducers/sources";
-import { waitForMs } from "../utils/utils";

-import { newGeneratedSources } from "./sources";
 import { updateWorkers } from "./debuggee";

 import { clearWasmStates } from "../utils/wasm";
@ -72,15 +69,7 @@ export function connect(
 * @static
 */
 export function navigated() {
-  return async function({ dispatch, getState, client, panel }: ThunkArgs) {
-    // this time out is used to wait for sources. If we have 0 sources,
-    // it is likely that the sources are being loaded from the bfcache,
-    // and we should make an explicit request to the server to load them.
-    await waitForMs(100);
-    if (getSourceList(getState()).length == 0) {
-      const sources = await client.fetchSources();
-      dispatch(newGeneratedSources(sources));
-    }
+  return async function({ panel }: ThunkArgs) {
    panel.emit("reloaded");
  };
 }
--- a/devtools/client/debugger/test/mochitest/browser.ini
+++ b/devtools/client/debugger/test/mochitest/browser.ini
@ -151,3 +151,4 @@ skip-if = (os == 'linux' && debug) || (os == 'linux' && asan) || ccov  #Bug 1456
 [browser_dbg-merge-scopes.js]
 [browser_dbg-message-run-to-completion.js]
 [browser_dbg-remember-expanded-scopes.js]
+[browser_dbg-bfcache.js]
--- a/devtools/client/debugger/test/mochitest/browser_dbg-bfcache.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg-bfcache.js
@ -0,0 +1,14 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
+
+// Test that sources appear in the debugger when navigating using the BFCache.
+add_task(async function() {
+  const dbg = await initDebugger("doc-bfcache1.html");
+  await navigate(dbg, "doc-bfcache2.html", "doc-bfcache2.html");
+  invokeInTab("goBack");
+  await waitForSources(dbg, "doc-bfcache1.html");
+  invokeInTab("goForward");
+  await waitForSources(dbg, "doc-bfcache2.html");
+  ok("Found sources after BFCache navigations");
+});
--- a/devtools/client/debugger/test/mochitest/examples/doc-bfcache1.html
+++ b/devtools/client/debugger/test/mochitest/examples/doc-bfcache1.html
@ -0,0 +1,6 @@
+<body>
+First Page!
+<script>
+function goForward() { history.go(1); }
+</script>
+</body>
--- a/devtools/client/debugger/test/mochitest/examples/doc-bfcache2.html
+++ b/devtools/client/debugger/test/mochitest/examples/doc-bfcache2.html
@ -0,0 +1,6 @@
+<body>
+Second Page!
+<script>
+function goBack() { history.go(-1); }
+</script>
+</body>
--- a/devtools/server/actors/targets/browsing-context.js
+++ b/devtools/server/actors/targets/browsing-context.js
@ -1283,7 +1283,7 @@ const browsingContextTargetPrototype = {
      }

      // Then fake window-ready and navigate on the given document
-      this._windowReady(window, true);
+      this._windowReady(window, { isFrameSwitching: true });
      DevToolsUtils.executeSoon(() => {
        this._navigate(window, true);
      });
@ -1311,7 +1311,7 @@ const browsingContextTargetPrototype = {
   * debugging, which may have been disabled temporarily by the
   * DebuggerProgressListener.
   */
-  _windowReady(window, isFrameSwitching = false) {
+  _windowReady(window, { isFrameSwitching, isBFCache } = {}) {
    const isTopLevel = window == this.window;

    // We just reset iframe list on WillNavigate, so we now list all existing
@ -1323,6 +1323,7 @@ const browsingContextTargetPrototype = {
    this.emit("window-ready", {
      window: window,
      isTopLevel: isTopLevel,
+      isBFCache,
      id: getWindowID(window),
    });
  },
@ -1602,20 +1603,22 @@ DebuggerProgressListener.prototype = {
    const window = evt.target.defaultView;
    const innerID = getWindowID(window);

-    // This method is alled on DOMWindowCreated and pageshow
-    // The common scenario is DOMWindowCreated, which is fired when the document
-    // loads. But we are to listen for pageshow in order to handle BFCache.
-    // When a page does into the BFCache, a pagehide event is fired with persisted=true
-    // but it doesn't necessarely mean persisted will be true for the pageshow
-    // event fired when the page is reloaded from the BFCache (see bug 1378133)
-    // So just check if we already know this window and accept any that isn't known yet
+    // This handler is called for two events: "DOMWindowCreated" and "pageshow".
+    // Bail out if we already processed this window.
    if (this._knownWindowIDs.has(innerID)) {
      return;
    }
-
-    this._targetActor._windowReady(window);
-
    this._knownWindowIDs.set(innerID, window);
+
+    // For a regular page navigation, "DOMWindowCreated" is fired before
+    // "pageshow". If the current event is "pageshow" but we have not processed
+    // the window yet, it means this is a BF cache navigation. In theory,
+    // `event.persisted` should be set for BF cache navigation events, but it is
+    // not always available, so we fallback on checking if "pageshow" is the
+    // first event received for a given window (see Bug 1378133).
+    const isBFCache = evt.type == "pageshow";
+
+    this._targetActor._windowReady(window, { isBFCache });
  }, "DebuggerProgressListener.prototype.onWindowCreated"),

  onWindowHidden: DevToolsUtils.makeInfallible(function(evt) {
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@ -1412,10 +1412,14 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
    return { frames: frames.filter(x => !!x) };
  },

-  onSources: function(request) {
+  addAllSources() {
    for (const source of this.dbg.findSources()) {
      this._addSource(source);
    }
+  },
+
+  onSources: function(request) {
+    this.addAllSources();

    // No need to flush the new source packets here, as we are sending the
    // list of sources out immediately and we don't need to invoke the
@ -1781,7 +1785,7 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
    return {};
  },

-  _onWindowReady: function({ isTopLevel, window }) {
+  _onWindowReady: function({ isTopLevel, isBFCache, window }) {
    if (isTopLevel && this.state != "detached") {
      this.sources.reset();
      this.clearDebuggees();
@ -1797,6 +1801,12 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
    if (this.attached) {
      this.dbg.addDebuggees();
    }
+
+    // BFCache navigations reuse old sources, so send existing sources to the
+    // client instead of waiting for onNewScript debugger notifications.
+    if (isBFCache) {
+      this.addAllSources();
+    }
  },

  _onWillNavigate: function({ isTopLevel }) {