From 13c718e2f2958f708624174f0738cc0b95cb8cf4 Mon Sep 17 00:00:00 2001 From: Dorel Luca Date: Mon, 8 Jun 2020 20:17:42 +0300 Subject: [PATCH] Backed out 3 changesets (bug 1622088) for XPCshell and Browser-chrome failurs on browser/abouthomecache/browser_process_crash.js. CLOSED TREE Backed out changeset 238fa307504a (bug 1622088) Backed out changeset ceaa7857baea (bug 1622088) Backed out changeset 9c75ae56f50e (bug 1622088) --- browser/components/BrowserGlue.jsm | 143 ++--------- .../components/newtab/AboutNewTabService.jsm | 20 -- browser/components/newtab/moz.build | 5 +- .../test/browser/abouthomecache/browser.ini | 20 -- .../abouthomecache/browser_basic_endtoend.js | 22 -- .../abouthomecache/browser_bump_version.js | 21 -- .../abouthomecache/browser_no_cache.js | 18 -- .../abouthomecache/browser_overwrite_cache.js | 37 --- .../abouthomecache/browser_process_crash.js | 41 --- .../test/browser/abouthomecache/head.js | 238 ------------------ .../newtab/test/xpcshell/ds_layout.json | 1 - .../test_AboutHomeStartupCacheWorker.js | 220 ---------------- .../newtab/test/xpcshell/topstories.json | 1 - .../newtab/test/xpcshell/xpcshell.ini | 5 - testing/talos/talos.json | 2 +- testing/talos/talos/test.py | 23 -- 16 files changed, 28 insertions(+), 789 deletions(-) delete mode 100644 browser/components/newtab/test/browser/abouthomecache/browser.ini delete mode 100644 browser/components/newtab/test/browser/abouthomecache/browser_basic_endtoend.js delete mode 100644 browser/components/newtab/test/browser/abouthomecache/browser_bump_version.js delete mode 100644 browser/components/newtab/test/browser/abouthomecache/browser_no_cache.js delete mode 100644 browser/components/newtab/test/browser/abouthomecache/browser_overwrite_cache.js delete mode 100644 browser/components/newtab/test/browser/abouthomecache/browser_process_crash.js delete mode 100644 browser/components/newtab/test/browser/abouthomecache/head.js delete mode 100644 browser/components/newtab/test/xpcshell/ds_layout.json delete mode 100644 browser/components/newtab/test/xpcshell/test_AboutHomeStartupCacheWorker.js delete mode 100644 browser/components/newtab/test/xpcshell/topstories.json diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index 4fa996ce9428..2c268e5aaafc 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -5031,10 +5031,6 @@ var AboutHomeStartupCache = { Services.obs.addObserver(this, "ipc:content-created"); Services.obs.addObserver(this, "ipc:content-shutdown"); - this._cacheEntryPromise = new Promise(resolve => { - this._cacheEntryResolver = resolve; - }); - let lci = Services.loadContextInfo.default; let storage = Services.cache2.diskCacheStorage(lci, false); try { @@ -5083,9 +5079,6 @@ var AboutHomeStartupCache = { this._initted = false; this._cacheEntry = null; this._hasWrittenThisSession = false; - this._cacheEntryPromise = null; - this._cacheEntryResolver = null; - this.log.trace("Uninitialized."); this.log.removeAppender(this._appender); this.log = null; @@ -5143,7 +5136,7 @@ var AboutHomeStartupCache = { * Resolves when a fresh version of the cache has been written. */ async cacheNow() { - this.log.trace("Caching now."); + this._hasWrittenThisSession = true; this._cacheProgress = "Getting cache streams"; let { pageInputStream, scriptInputStream } = await this.requestCache(); @@ -5155,8 +5148,6 @@ var AboutHomeStartupCache = { this._cacheProgress = "Writing to cache"; await this.populateCache(pageInputStream, scriptInputStream); this._cacheProgress = "Done"; - this.log.trace("Done writing to cache."); - this._hasWrittenThisSession = true; }, /** @@ -5274,7 +5265,7 @@ var AboutHomeStartupCache = { if (parseInt(version, 10) != this.CACHE_VERSION) { this.log.info("Version does not match! Dooming and closing streams.\n"); // This cache is no good - doom it, and prepare for a new one. - this.clearCache(); + this._cacheEntry = this._cacheEntry.recreate(); this.pagePipe.outputStream.close(); this.scriptPipe.outputStream.close(); return; @@ -5375,120 +5366,40 @@ var AboutHomeStartupCache = { * A stream containing the HTML markup to be saved to the cache. * @param scriptInputStream (nsIInputStream) * A stream containing the JS hydration script to be saved to the cache. - * @returns Promise - * @resolves undefined - * When the cache has been successfully written to. - * @rejects Error - * Rejects with a JS Error if writing any part of the cache happens to - * fail. */ - async populateCache(pageInputStream, scriptInputStream) { - await this.ensureCacheEntry(); + populateCache(pageInputStream, scriptInputStream) { + // Doom the old cache entry, so we can start writing to a new one. + this.log.trace("Populating the cache. Dooming old entry."); + this._cacheEntry = this._cacheEntry.recreate(); - await new Promise((resolve, reject) => { - // Doom the old cache entry, so we can start writing to a new one. - this.log.trace("Populating the cache. Dooming old entry."); - this.clearCache(); + this.log.trace("Opening the page output stream."); + let pageOutputStream = this._cacheEntry.openOutputStream(0, -1); - this.log.trace("Opening the page output stream."); - let pageOutputStream; - try { - pageOutputStream = this._cacheEntry.openOutputStream(0, -1); - } catch (e) { - reject(e); - return; - } + this.log.info("Writing the page cache."); + NetUtil.asyncCopy(pageInputStream, pageOutputStream, () => { + this.log.trace( + "Writing the page data is complete. Now opening the " + + "script output stream." + ); - this.log.info("Writing the page cache."); - NetUtil.asyncCopy(pageInputStream, pageOutputStream, pageResult => { - if (!Components.isSuccessCode(pageResult)) { - this.log.error("Failed to write page. Result: " + pageResult); - reject(new Error(pageResult)); - return; - } + let scriptOutputStream = this._cacheEntry.openAlternativeOutputStream( + "script", + -1 + ); - this.log.trace( - "Writing the page data is complete. Now opening the " + - "script output stream." - ); - - let scriptOutputStream; - try { - scriptOutputStream = this._cacheEntry.openAlternativeOutputStream( - "script", - -1 - ); - } catch (e) { - reject(e); - return; - } - - this.log.info("Writing the script cache."); - NetUtil.asyncCopy( - scriptInputStream, - scriptOutputStream, - scriptResult => { - if (!Components.isSuccessCode(scriptResult)) { - this.log.error("Failed to write script. Result: " + scriptResult); - reject(new Error(scriptResult)); - return; - } - - this.log.trace( - "Writing the script cache is done. Setting version." - ); - try { - this._cacheEntry.setMetaDataElement( - "version", - String(this.CACHE_VERSION) - ); - } catch (e) { - this.log.error("Failed to write version."); - reject(e); - return; - } - this.log.trace(`Version is set to ${this.CACHE_VERSION}.`); - this.log.info("Caching of page and script is done."); - resolve(); - } + this.log.info("Writing the script cache."); + NetUtil.asyncCopy(scriptInputStream, scriptOutputStream, () => { + this.log.trace("Writing the script cache is done. Setting version."); + this._cacheEntry.setMetaDataElement( + "version", + String(this.CACHE_VERSION) ); + this.log.trace(`Version is set to ${this.CACHE_VERSION}.`); + this.log.info("Caching of page and script is done."); }); }); }, - /** - * Returns a Promise that resolves once the nsICacheEntry for the cache - * is available to write to and read from. - * - * @returns Promise - * @resolves nsICacheEntry - * Once the cache entry has become available. - * @rejects String - * Rejects with an error message if getting the cache entry is attempted - * before the AboutHomeStartupCache component has been initialized. - */ - ensureCacheEntry() { - if (!this._initted) { - return Promise.reject( - "Cannot ensureCacheEntry - AboutHomeStartupCache is not initted" - ); - } - - return this._cacheEntryPromise; - }, - - /** - * Clears the contents of the cache. - */ - clearCache() { - this.log.trace("Clearing the cache."); - this._cacheEntry = this._cacheEntry.recreate(); - this._cacheEntryPromise = new Promise(resolve => { - resolve(this._cacheEntry); - }); - this._hasWrittenThisSession = false; - }, - /** * Called when a content process is created. If this is the "privileged * about content process", then the cache streams will be sent to it. @@ -5609,7 +5520,5 @@ var AboutHomeStartupCache = { this._cacheEntry = aEntry; this.makePipes(); this.maybeConnectToPipes(); - - this._cacheEntryResolver(this._cacheEntry); }, }; diff --git a/browser/components/newtab/AboutNewTabService.jsm b/browser/components/newtab/AboutNewTabService.jsm index 80a2d24e393f..506b521f528c 100644 --- a/browser/components/newtab/AboutNewTabService.jsm +++ b/browser/components/newtab/AboutNewTabService.jsm @@ -52,8 +52,6 @@ const { E10SUtils } = ChromeUtils.import( const PREF_ABOUT_HOME_CACHE_ENABLED = "browser.startup.homepage.abouthome_cache.enabled"; -const PREF_ABOUT_HOME_CACHE_TESTING = - "browser.startup.homepage.abouthome_cache.testing"; const PREF_SEPARATE_ABOUT_WELCOME = "browser.aboutwelcome.enabled"; const SEPARATE_ABOUT_WELCOME_URL = "resource://activity-stream/aboutwelcome/aboutwelcome.html"; @@ -122,24 +120,6 @@ const AboutHomeStartupCacheChild = { this._initted = true; }, - /** - * A function that lets us put the AboutHomeStartupCacheChild back into - * its initial state. This is used by tests to let us simulate the startup - * behaviour of the module without having to manually launch a new privileged - * about content process every time. - */ - uninit() { - if (!Services.prefs.getBoolPref(PREF_ABOUT_HOME_CACHE_TESTING, false)) { - throw new Error( - "Cannot uninit AboutHomeStartupCacheChild unless testing." - ); - } - - this._pageInputStream = null; - this._scriptInputStream = null; - this._initted = false; - }, - /** * A public method called from nsIAboutNewTabService that attempts * return an nsIChannel for a cached about:home document that we diff --git a/browser/components/newtab/moz.build b/browser/components/newtab/moz.build index 379f17df9d6d..7c9efa5f9f8e 100644 --- a/browser/components/newtab/moz.build +++ b/browser/components/newtab/moz.build @@ -7,10 +7,7 @@ with Files("**"): BUG_COMPONENT = ("Firefox", "New Tab Page") -BROWSER_CHROME_MANIFESTS += [ - 'test/browser/abouthomecache/browser.ini', - 'test/browser/browser.ini', -] +BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini'] SPHINX_TREES['docs'] = 'docs' SPHINX_TREES['content-src/asrouter/docs'] = 'content-src/asrouter/docs' diff --git a/browser/components/newtab/test/browser/abouthomecache/browser.ini b/browser/components/newtab/test/browser/abouthomecache/browser.ini deleted file mode 100644 index f80c5b1d7dbd..000000000000 --- a/browser/components/newtab/test/browser/abouthomecache/browser.ini +++ /dev/null @@ -1,20 +0,0 @@ -[DEFAULT] -support-files = - head.js -prefs = - browser.tabs.remote.separatePrivilegedContentProcess=true - browser.startup.homepage.abouthome_cache.enabled=true - browser.startup.homepage.abouthome_cache.cache_on_shutdown=false - browser.startup.homepage.abouthome_cache.testing=true - browser.startup.page=1 - browser.newtabpage.activity-stream.discoverystream.endpoints=data: - browser.newtabpage.activity-stream.feeds.section.topstories=true - browser.newtabpage.activity-stream.feeds.section.topstories.options={"provider_name":""} - browser.newtabpage.activity-stream.telemetry.structuredIngestion=false - -[browser_basic_endtoend.js] -[browser_bump_version.js] -[browser_no_cache.js] -[browser_overwrite_cache.js] -[browser_process_crash.js] -skip-if = !e10s || !crashreporter diff --git a/browser/components/newtab/test/browser/abouthomecache/browser_basic_endtoend.js b/browser/components/newtab/test/browser/abouthomecache/browser_basic_endtoend.js deleted file mode 100644 index a2e984509d93..000000000000 --- a/browser/components/newtab/test/browser/abouthomecache/browser_basic_endtoend.js +++ /dev/null @@ -1,22 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -/** - * Tests that the about:home cache gets written on shutdown, and read - * from in the subsequent startup. - */ -add_task(async function test_basic_behaviour() { - await BrowserTestUtils.withNewTab("about:home", async browser => { - // First, clear the cache to test the base case. - await clearCache(); - await simulateRestart(browser); - await ensureCachedAboutHome(browser); - - // Next, test that a subsequent restart also shows the cached - // about:home. - await simulateRestart(browser); - await ensureCachedAboutHome(browser); - }); -}); diff --git a/browser/components/newtab/test/browser/abouthomecache/browser_bump_version.js b/browser/components/newtab/test/browser/abouthomecache/browser_bump_version.js deleted file mode 100644 index e2fc5e593450..000000000000 --- a/browser/components/newtab/test/browser/abouthomecache/browser_bump_version.js +++ /dev/null @@ -1,21 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -/** - * Test that if the "version" metadata on the cache entry doesn't match - * the expectation that we ignore the cache and load the dynamic about:home - * document. - */ -add_task(async function test_bump_version() { - await BrowserTestUtils.withNewTab("about:home", async browser => { - // First, ensure that a pre-existing cache exists. - await simulateRestart(browser); - - let cacheEntry = await AboutHomeStartupCache.ensureCacheEntry(); - cacheEntry.setMetaDataElement("version", "somethingnew"); - await simulateRestart(browser, false /* withAutoShutdownWrite */); - await ensureDynamicAboutHome(browser); - }); -}); diff --git a/browser/components/newtab/test/browser/abouthomecache/browser_no_cache.js b/browser/components/newtab/test/browser/abouthomecache/browser_no_cache.js deleted file mode 100644 index d19bb66a87db..000000000000 --- a/browser/components/newtab/test/browser/abouthomecache/browser_no_cache.js +++ /dev/null @@ -1,18 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -requestLongerTimeout(2); - -/** - * Test that if there's no cache written, that we load the dynamic - * about:home document on startup. - */ -add_task(async function test_no_cache() { - await BrowserTestUtils.withNewTab("about:home", async browser => { - await clearCache(); - await simulateRestart(browser, false /* withAutoShutdownWrite */); - await ensureDynamicAboutHome(browser); - }); -}); diff --git a/browser/components/newtab/test/browser/abouthomecache/browser_overwrite_cache.js b/browser/components/newtab/test/browser/abouthomecache/browser_overwrite_cache.js deleted file mode 100644 index 236503911d5c..000000000000 --- a/browser/components/newtab/test/browser/abouthomecache/browser_overwrite_cache.js +++ /dev/null @@ -1,37 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -/** - * Tests that if a pre-existing about:home cache exists, that it can - * be overwritten with new information. - */ -add_task(async function test_overwrite_cache() { - await BrowserTestUtils.withNewTab("about:home", async browser => { - await simulateRestart(browser); - const TEST_ID = "test_overwrite_cache_h1"; - - // We need the CSP meta tag in about: pages, otherwise we hit assertions in - // debug builds. - await injectIntoCache( - ` - - - - - -

Something new

- - - `, - "window.__FROM_STARTUP_CACHE__ = true;" - ); - await simulateRestart(browser, false /* withAutoShutdownWrite */); - - await SpecialPowers.spawn(browser, [TEST_ID], async testID => { - let target = content.document.getElementById(testID); - Assert.ok(target, "Found the target element"); - }); - }); -}); diff --git a/browser/components/newtab/test/browser/abouthomecache/browser_process_crash.js b/browser/components/newtab/test/browser/abouthomecache/browser_process_crash.js deleted file mode 100644 index 5fb47d87408b..000000000000 --- a/browser/components/newtab/test/browser/abouthomecache/browser_process_crash.js +++ /dev/null @@ -1,41 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -/** - * Test that if the "privileged about content process" crashes, that it - * drops its internal reference to the "privileged about content process" - * process manager, and that a subsequent restart of that process type - * results in a new cached document load. Also tests that crashing of - * any other content process type doesn't clear the process manager - * reference. - */ -add_task(async function test_bump_version() { - await BrowserTestUtils.withNewTab("about:home", async browser => { - await simulateRestart(browser); - let origProcManager = AboutHomeStartupCache._procManager; - - await BrowserTestUtils.crashFrame(browser); - Assert.notEqual( - origProcManager, - AboutHomeStartupCache._procManager, - "Should have dropped the reference to the crashed process" - ); - }); - - let latestProcManager = AboutHomeStartupCache._procManager; - - await BrowserTestUtils.withNewTab("about:home", async browser => { - await ensureCachedAboutHome(browser); - }); - - await BrowserTestUtils.withNewTab("http://example.com", async browser => { - await BrowserTestUtils.crashFrame(browser); - Assert.equal( - latestProcManager, - AboutHomeStartupCache._procManager, - "Should still have the reference to the privileged about process" - ); - }); -}); diff --git a/browser/components/newtab/test/browser/abouthomecache/head.js b/browser/components/newtab/test/browser/abouthomecache/head.js deleted file mode 100644 index c5a06ae0cfde..000000000000 --- a/browser/components/newtab/test/browser/abouthomecache/head.js +++ /dev/null @@ -1,238 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -let { AboutHomeStartupCache } = ChromeUtils.import( - "resource:///modules/BrowserGlue.jsm" -); - -/** - * Shuts down the AboutHomeStartupCache components in the parent process - * and privileged about content process, and then restarts them, simulating - * the parent process having restarted. - * - * @param browser () - * A with about:home running in it. This will be reloaded - * after the restart simultion is complete, and that reload will attempt - * to read any about:home cache contents. - * @param options (object, optional) - * - * An object with the following properties: - * - * withAutoShutdownWrite (boolean, optional): - * Whether or not the shutdown part of the simulation should cause the - * shutdown handler to run, which normally causes the cache to be - * written. Setting this to false is handy if the cache has been - * specially prepared for the subsequent startup, and we don't want to - * overwrite it. This defaults to true. - * - * ensureCacheWinsRace (boolean, optional): - * Ensures that the privileged about content process will be able to - * read the bytes from the streams sent down from the HTTP cache. Use - * this to avoid the HTTP cache "losing the race" against reading the - * about:home document from the omni.ja. This defaults to true. - * - * @returns Promise - * @resolves undefined - * Resolves once the restart simulation is complete, and the - * pointed at about:home finishes reloading. - */ -// eslint-disable-next-line no-unused-vars -async function simulateRestart( - browser, - { withAutoShutdownWrite, ensureCacheWinsRace } = { - withAutoShutdownWrite: true, - ensureCacheWinsRace: true, - } -) { - info("Simulating restart of the browser"); - let processManager = browser.messageManager.processMessageManager; - if (processManager.remoteType !== E10SUtils.PRIVILEGEDABOUT_REMOTE_TYPE) { - throw new Error( - "prepareLoadFromCache should only be called on a browser " + - "loaded in the privileged about content process." - ); - } - - if (withAutoShutdownWrite) { - info("Simulating shutdown write"); - await AboutHomeStartupCache.onShutdown(); - info("Shutdown write done"); - } else { - info("Intentionally skipping shutdown write"); - } - - AboutHomeStartupCache.uninit(); - - info("Waiting for AboutHomeStartupCacheChild to uninit"); - await SpecialPowers.spawn(browser, [], async () => { - let { AboutHomeStartupCacheChild } = ChromeUtils.import( - "resource:///modules/AboutNewTabService.jsm" - ); - AboutHomeStartupCacheChild.uninit(); - }); - info("AboutHomeStartupCacheChild uninitted"); - - AboutHomeStartupCache.init(); - - AboutHomeStartupCache.sendCacheInputStreams(processManager); - - info("Waiting for AboutHomeStartupCache cache entry"); - await AboutHomeStartupCache.ensureCacheEntry(); - info("Got AboutHomeStartupCache cache entry"); - - if (ensureCacheWinsRace) { - info("Ensuring cache bytes are available"); - await SpecialPowers.spawn(browser, [], async () => { - let { AboutHomeStartupCacheChild } = ChromeUtils.import( - "resource:///modules/AboutNewTabService.jsm" - ); - let pageStream = AboutHomeStartupCacheChild._pageInputStream; - let scriptStream = AboutHomeStartupCacheChild._scriptInputStream; - await ContentTaskUtils.waitForCondition(() => { - return pageStream.available() && scriptStream.available(); - }); - }); - } - - info("Waiting for about:home to load"); - let loaded = BrowserTestUtils.browserLoaded(browser, false, "about:home"); - BrowserTestUtils.loadURI(browser, "about:home"); - await loaded; - info("about:home loaded"); -} - -/** - * Writes a page string and a script string into the cache for - * the next about:home load. - * - * @param page (String) - * The HTML content to write into the cache. This cannot be the empty - * string. - * @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. - * @returns Promise - * @resolves undefined - * When the page and script content has been successfully written. - */ -// eslint-disable-next-line no-unused-vars -async function injectIntoCache(page, script) { - if (!page || !script) { - throw new Error("Cannot injectIntoCache with falsey values"); - } - - await AboutHomeStartupCache.ensureCacheEntry(); - - let pageInputStream = Cc[ - "@mozilla.org/io/string-input-stream;1" - ].createInstance(Ci.nsIStringInputStream); - - pageInputStream.setUTF8Data(page); - - let scriptInputStream = Cc[ - "@mozilla.org/io/string-input-stream;1" - ].createInstance(Ci.nsIStringInputStream); - - scriptInputStream.setUTF8Data(script); - - await AboutHomeStartupCache.populateCache(pageInputStream, scriptInputStream); -} - -/** - * Clears out any pre-existing about:home cache. - * @returns Promise - * @resolves undefined - * Resolves when the cache is cleared. - */ -// eslint-disable-next-line no-unused-vars -async function clearCache() { - info("Test is clearing the cache"); - AboutHomeStartupCache.clearCache(); - await AboutHomeStartupCache.ensureCacheEntry(); - info("Test has cleared the cache."); -} - -/** - * Tests that the about:home document loaded in a passed was - * one from the cache. - * - * We test for this by looking for some tell-tale signs of the cached - * document: - * - * 1. The about:home?jscache