Bug 1673716 - Ensure the about:home startup cache can only be used by one BrowsingContext. r=Gijs

It is theoretically possible for two BrowsingContexts to be loading about:home
at the same time during startup (for example, if the user passed in a series
of URLs to open via the command-line, including multiple about:home's).

This patch ensures that only one of those BrowsingContexts (the first to load)
gets to consume the cached streams.

Differential Revision: https://phabricator.services.mozilla.com/D97518

browser/components/BrowserGlue.jsm

@@ -232,8 +232,12 @@ let JSWINDOWACTORS = {
       },
     },
     // The wildcard on about:newtab is for the ?endpoint query parameter
-    // that is used for snippets debugging.
-    matches: ["about:home", "about:welcome", "about:newtab*"],
+    // that is used for snippets debugging. The wildcard for about:home
+    // is similar, and also allows for falling back to loading the
+    // about:home document dynamically if an attempt is made to load
+    // about:home?jscache from the AboutHomeStartupCache as a top-level
+    // load.
+    matches: ["about:home*", "about:welcome", "about:newtab*"],
     remoteTypes: ["privilegedabout"],
   },
 

browser/components/newtab/AboutNewTabService.jsm

@@ -93,6 +93,19 @@ const AboutHomeStartupCacheChild = {
   CACHE_REQUEST_MESSAGE: "AboutHomeStartupCache:CacheRequest",
   CACHE_RESPONSE_MESSAGE: "AboutHomeStartupCache:CacheResponse",
   CACHE_USAGE_RESULT_MESSAGE: "AboutHomeStartupCache:UsageResult",
+  STATES: {
+    UNAVAILABLE: 0,
+    UNCONSUMED: 1,
+    PAGE_CONSUMED: 2,
+    PAGE_AND_SCRIPT_CONSUMED: 3,
+    FAILED: 4,
+  },
+  REQUEST_TYPE: {
+    PAGE: 0,
+    SCRIPT: 1,
+  },
+  _state: 0,
+  _consumerBCID: null,
 
   /**
    * Called via a process script very early on in the process lifetime. This
@@ -129,6 +142,7 @@ const AboutHomeStartupCacheChild = {
     this._pageInputStream = pageInputStream;
     this._scriptInputStream = scriptInputStream;
     this._initted = true;
+    this.setState(this.STATES.UNCONSUMED);
   },
 
   /**
@@ -159,6 +173,8 @@ const AboutHomeStartupCacheChild = {
     this._pageInputStream = null;
     this._scriptInputStream = null;
     this._initted = false;
+    this._state = this.STATES.UNAVAILABLE;
+    this._consumerBCID = null;
   },
 
   /**
@@ -166,12 +182,15 @@ const AboutHomeStartupCacheChild = {
    * return an nsIChannel for a cached about:home document that we
    * were initialized with. If we failed to be initted with the
    * cache, or the input streams that we were sent have no data
-   * yet available, this function returns null. The caller should =
+   * yet available, this function returns null. The caller should
    * fall back to generating the page dynamically.
    *
    * This function will be called when loading about:home, or
    * about:home?jscache - the latter returns the cached script.
    *
+   * It is expected that the same BrowsingContext that loads the cached
+   * page will also load the cached script.
+   *
    * @param uri (nsIURI)
    *   The URI for the requested page, as passed by nsIAboutNewTabService.
    * @param loadInfo (nsILoadInfo)
@@ -184,7 +203,27 @@ const AboutHomeStartupCacheChild = {
       return null;
     }
 
-    let isScriptRequest = uri.query === "jscache";
+    if (this._state >= this.STATES.PAGE_AND_SCRIPT_CONSUMED) {
+      return null;
+    }
+
+    let requestType =
+      uri.query === "jscache"
+        ? this.REQUEST_TYPE.SCRIPT
+        : this.REQUEST_TYPE.PAGE;
+
+    // If this is a page request, then we need to be in the UNCONSUMED state,
+    // since we expect the page request to come first. If this is a script
+    // request, we expect to be in PAGE_CONSUMED state, since the page cache
+    // stream should he been consumed already.
+    if (
+      (requestType === this.REQUEST_TYPE.PAGE &&
+        this._state !== this.STATES.UNCONSUMED) ||
+      (requestType === this.REQUEST_TYPE_SCRIPT &&
+        this._state !== this.STATES.PAGE_CONSUMED)
+    ) {
+      return null;
+    }
 
     // If by this point, we don't have anything in the streams,
     // then either the cache was too slow to give us data, or the cache
@@ -194,16 +233,18 @@ const AboutHomeStartupCacheChild = {
     // We only do this on the page request, because by the time
     // we get to the script request, we should have already drained
     // the page input stream.
-    if (!isScriptRequest) {
+    if (requestType === this.REQUEST_TYPE.PAGE) {
       try {
         if (
           !this._scriptInputStream.available() ||
           !this._pageInputStream.available()
         ) {
+          this.setState(this.STATES.FAILED);
           this.reportUsageResult(false /* success */);
           return null;
         }
       } catch (e) {
+        this.setState(this.STATES.FAILED);
         if (e.result === Cr.NS_BASE_STREAM_CLOSED) {
           this.reportUsageResult(false /* success */);
           return null;
@@ -212,17 +253,37 @@ const AboutHomeStartupCacheChild = {
       }
     }
 
+    if (
+      requestType === this.REQUEST_TYPE.SCRIPT &&
+      this._consumerBCID !== loadInfo.browsingContextID
+    ) {
+      // Some other document is somehow requesting the script - one
+      // that didn't originally request the page. This is not allowed.
+      this.setState(this.STATES.FAILED);
+      return null;
+    }
+
     let channel = Cc[
       "@mozilla.org/network/input-stream-channel;1"
     ].createInstance(Ci.nsIInputStreamChannel);
     channel.QueryInterface(Ci.nsIChannel);
     channel.setURI(uri);
     channel.loadInfo = loadInfo;
-    channel.contentStream = isScriptRequest
-      ? this._scriptInputStream
-      : this._pageInputStream;
+    channel.contentStream =
+      requestType === this.REQUEST_TYPE.PAGE
+        ? this._pageInputStream
+        : this._scriptInputStream;
 
-    this.reportUsageResult(true /* success */);
+    if (requestType === this.REQUEST_TYPE.SCRIPT) {
+      this.setState(this.STATES.PAGE_AND_SCRIPT_CONSUMED);
+      this.reportUsageResult(true /* success */);
+    } else {
+      this.setState(this.STATES.PAGE_CONSUMED);
+      // Stash the BrowsingContext ID so that when the script stream
+      // attempts to be consumed, we ensure that it's from the same
+      // BrowsingContext that loaded the page.
+      this._consumerBCID = loadInfo.browsingContextID;
+    }
 
     return channel;
   },
@@ -303,6 +364,25 @@ const AboutHomeStartupCacheChild = {
       this._cacheWorker = null;
     }
   },
+
+  /**
+   * Transitions the AboutHomeStartupCacheChild from one state
+   * to the next, where each state is defined in this.STATES.
+   *
+   * States can only be transitioned in increasing order, otherwise
+   * an error is logged.
+   */
+  setState(state) {
+    if (state > this._state) {
+      this._state = state;
+    } else {
+      console.error(
+        "AboutHomeStartupCacheChild could not transition from state " +
+          `${this._state} to ${state}`,
+        new Error().stack
+      );
+    }
+  },
 };
 
 /**

browser/components/newtab/docs/v2-system-addon/about_home_startup_cache.md

@@ -46,7 +46,7 @@ This singleton component lives inside of the "privileged about content process",
 
 When the `AboutRedirector` in the "privileged about content process" notices that a request has been made to `about:home`, it asks `nsIAboutNewTabService` to return a new `nsIChannel` for that document. The `AboutNewTabChildService` then checks to see if the `AboutHomeStartupCacheChild` can return an `nsIChannel` for any cached content.
 
-If, at this point, nothing has been streamed from the parent, we fall back to loading the dynamic `about:home` document. This might occur if the cache doesn't exist yet, or if we were too slow to pull it off of the disk.
+If, at this point, nothing has been streamed from the parent, we fall back to loading the dynamic `about:home` document. This might occur if the cache doesn't exist yet, or if we were too slow to pull it off of the disk. Subsequent attempts to load `about:home` will bypass the cache and load the dynamic document instead. This is true even if the privileged about content process crashes and a new one is created.
 
 The `AboutHomeStartupCacheChild` will also be responsible for generating the cache periodically. Periodically, the `AboutNewTabService` will send down the most up-to-date state for `about:home` from the parent process, and then the `AboutHomeStartupCacheChild` will generate document markup using ReactDOMServer within a `ChromeWorker`. After that's generated, the "privileged about content process" will send up `nsIInputStream` instances for both the markup and the script for the initial page state. The `AboutHomeStartupCache` singleton inside of `BrowserGlue` is responsible for receiving those `nsIInputStream`'s and persisting them in the HTTP cache for the next start.
 

browser/components/newtab/test/browser/abouthomecache/browser.ini

@@ -29,4 +29,5 @@ skip-if = asan || debug #Bug 1651277
 [browser_overwrite_cache.js]
 [browser_process_crash.js]
 skip-if = !e10s || !crashreporter
+[browser_same_consumer.js]
 [browser_sanitize.js]

browser/components/newtab/test/browser/abouthomecache/browser_overwrite_cache.js

@@ -22,6 +22,7 @@ add_task(async function test_overwrite_cache() {
         </head>
         <body>
           <h1 id="${TEST_ID}">Something new</h1>
+          <div id="root"></div>
         </body>
         <script src="about:home?jscache"></script>
       </html>`,

browser/components/newtab/test/browser/abouthomecache/browser_same_consumer.js

@@ -0,0 +1,52 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Tests that if a page attempts to load the script stream without
+ * having also loaded the page stream, that it will fail and get
+ * the default non-cached script.
+ */
+add_task(async function test_same_consumer() {
+  await BrowserTestUtils.withNewTab("about:home", async browser => {
+    await simulateRestart(browser);
+
+    // We need the CSP meta tag in about: pages, otherwise we hit assertions in
+    // debug builds.
+    //
+    // We inject a script that sets a __CACHE_CONSUMED__ property to true on
+    // the window element. We'll test to ensure that if we try to load the
+    // script cache from a different BrowsingContext that this property is
+    // not set.
+    await injectIntoCache(
+      `
+      <html>
+        <head>
+        <meta http-equiv="Content-Security-Policy" content="default-src 'none'; object-src 'none'; script-src resource: chrome:; connect-src https:; img-src https: data: blob:; style-src 'unsafe-inline';">
+        </head>
+        <body>
+          <h1>A fake about:home page</h1>
+          <div id="root"></div>
+        </body>
+      </html>`,
+      "window.__CACHE_CONSUMED__ = true;"
+    );
+    await simulateRestart(browser, false /* withAutoShutdownWrite */);
+
+    // Attempting to load the script from the cache should fail, and instead load
+    // the markup.
+    await BrowserTestUtils.withNewTab("about:home?jscache", async browser2 => {
+      await SpecialPowers.spawn(browser2, [], async () => {
+        Assert.ok(
+          !Cu.waiveXrays(content).__CACHE_CONSUMED__,
+          "Should not have found __CACHE_CONSUMED__ property"
+        );
+        Assert.ok(
+          content.document.body.classList.contains("activity-stream"),
+          "Should have found activity-stream class on <body> element"
+        );
+      });
+    });
+  });
+});

browser/components/newtab/test/browser/abouthomecache/head.js

@@ -144,7 +144,10 @@ async function simulateRestart(
  *
  * @param page (String)
  *   The HTML content to write into the cache. This cannot be the empty
- *   string.
+ *   string. Note that this string should contain a node that has an
+ *   id of "root", in order for the newtab scripts to attach correctly.
+ *   Otherwise, an exception might get thrown which can cause shutdown
+ *   leaks.
  * @param script (String)
  *   The JS content to write into the cache that can be loaded via
  *   about:home?jscache. This cannot be the empty string.
@@ -158,6 +161,10 @@ async function injectIntoCache(page, script) {
     throw new Error("Cannot injectIntoCache with falsey values");
   }
 
+  if (!page.includes(`id="root"`)) {
+    throw new Error("Page markup must include a root node.");
+  }
+
   await AboutHomeStartupCache.ensureCacheEntry();
 
   let pageInputStream = Cc[