Bug 1761473 - Get rid of deprecated downloadToDisk() attachments method... (ef2e110e) · Commits · The Tor Project / Applications / Tor Browser

browser/components/asrouter/modules/ASRouter.sys.mjs

+26 −13

Original line number	Diff line number	Diff line
		@@ -36,7 +36,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
		"resource:///modules/asrouter/ASRouterTriggerListeners.sys.mjs",
		AttributionCode: "resource:///modules/AttributionCode.sys.mjs",
		BookmarksBarButton: "resource:///modules/asrouter/BookmarksBarButton.sys.mjs",
		Downloader: "resource://services-settings/Attachments.sys.mjs",
		UnstoredDownloader: "resource://services-settings/Attachments.sys.mjs",
		ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs",
		FeatureCalloutBroker:
		"resource:///modules/asrouter/FeatureCalloutBroker.sys.mjs",
		@@ -271,8 +271,8 @@ export const MessageLoaderUtils = {
		* "ms-language-packs" collection. E.g. for "en-US" with version "v1",
		* the Fluent file is attched to the record with ID "cfr-v1-en-US".
		*
		* 2). The Remote Settings downloader is able to detect the duplicate download
		* requests for the same attachment and ignore the redundent requests automatically.
		* 2). To prevent duplicate downloads, we verify that the local file matches
		* the attachment on the Remote Settings record.
		*
		* @param {object} provider An AS router provider
		* @param {string} provider.id The id of the provider
		@@ -306,16 +306,29 @@ export const MessageLoaderUtils = {
		.collection(RS_COLLECTION_L10N)
		.getRecord(recordId);
		if (record && record.data) {
		const downloader = new lazy.Downloader(
		// Check that the file on disk is the same as the one on the server.
		// If the file is the same, we don't need to download it again.
		const localFile = lazy.RemoteL10n.cfrFluentFilePath;
		const { size: remoteSize } = record.data.attachment;
		if (
		!(await IOUtils.exists(localFile)) \|\|
		(await IOUtils.stat(localFile)).size !== remoteSize
		) {
		// Here we are using the UnstoredDownloader to download the attachment
		// because we don't want to store it in the (default) IndexedDB cache.
		const downloader = new lazy.UnstoredDownloader(
		RS_MAIN_BUCKET,
		RS_COLLECTION_L10N,
		"browser",
		"newtab"
		RS_COLLECTION_L10N
		);
		// Await here in order to capture the exceptions for reporting.
		await downloader.downloadToDisk(record.data, {
		const { buffer } = await downloader.download(record.data, {
		retries: RS_DOWNLOAD_MAX_RETRIES,
		});
		// Write on disk.
		await IOUtils.write(localFile, new Uint8Array(buffer), {
		tmpPath: `${localFile}.tmp`,
		});
		}
		lazy.RemoteL10n.reloadL10n();
		} else {
		MessageLoaderUtils._handleRemoteSettingsUndesiredEvent(

browser/components/asrouter/modules/RemoteL10n.sys.mjs

+20 −8

Original line number	Diff line number	Diff line
		@@ -164,6 +164,24 @@ export class _RemoteL10n {
		}
		}

		get cfrFluentFileDir() {
		return PathUtils.join(
		Services.dirsvc.get("ProfLD", Ci.nsIFile).path,
		"settings",
		"main",
		"ms-language-packs"
		);
		}

		get cfrFluentFilePath() {
		return PathUtils.join(
		this.cfrFluentFileDir,
		"browser",
		"newtab",
		"asrouter.ftl"
		);
		}

		/**
		* Creates a new DOMLocalization instance with the Fluent file from Remote Settings.
		*
		@@ -176,23 +194,17 @@ export class _RemoteL10n {
		let useRemoteL10n = Services.prefs.getBoolPref(USE_REMOTE_L10N_PREF, true);
		if (useRemoteL10n && !L10nRegistry.getInstance().hasSource("cfr")) {
		const appLocale = Services.locale.appLocaleAsBCP47;
		const l10nFluentDir = PathUtils.join(
		Services.dirsvc.get("ProfLD", Ci.nsIFile).path,
		"settings",
		"main",
		"ms-language-packs"
		);
		let cfrIndexedFileSource = new L10nFileSource(
		"cfr",
		"app",
		[appLocale],
		`file://${l10nFluentDir}/`,
		`${PathUtils.toFileURI(this.cfrFluentFileDir)}/`,
		{
		addResourceOptions: {
		allowOverrides: true,
		},
		},
		[`file://${l10nFluentDir}/browser/newtab/asrouter.ftl`]
		[PathUtils.toFileURI(this.cfrFluentFilePath)]
		);
		L10nRegistry.getInstance().registerSources([cfrIndexedFileSource]);
		} else if (!useRemoteL10n && L10nRegistry.getInstance().hasSource("cfr")) {

browser/components/asrouter/tests/unit/ASRouter.test.js

+43 −13

Original line number	Diff line number	Diff line
		@@ -269,12 +269,12 @@ describe("ASRouter", () => {
		return this;
		}
		getRecord() {
		return Promise.resolve({ data: {} });
		return Promise.resolve({ data: { attachment: { size: 42 } } });
		}
		},
		Downloader: class {
		UnstoredDownloader: class {
		download() {
		return Promise.resolve("/path/to/download");
		return Promise.resolve({ buffer: "fake buffer" });
		}
		},
		NimbusFeatures: fakeNimbusFeatures,
		@@ -298,6 +298,9 @@ describe("ASRouter", () => {
		// This is just a subset of supported locales that happen to be used in
		// the test.
		isLocaleSupported: locale => ["en-US", "ja-JP-mac"].includes(locale),
		// PathUtils.join() is mocked in `unit-entry.js`, only filenames count.
		cfrFluentFileDir: "ms-language-packs",
		cfrFluentFilePath: "asrouter.ftl",
		},
		});
		await createRouterAndInit();
		@@ -1030,8 +1033,9 @@ describe("ASRouter", () => {
		sandbox
		.stub(MessageLoaderUtils, "_getRemoteSettingsMessages")
		.resolves([{ id: "message_1" }]);
		sandbox.stub(global.IOUtils, "exists").resolves(false);
		const spy = sandbox.spy();
		global.Downloader.prototype.downloadToDisk = spy;
		global.UnstoredDownloader.prototype.download = spy;
		const provider = {
		id: "cfr",
		enabled: true,
		@@ -2680,11 +2684,12 @@ describe("ASRouter", () => {
		sandbox
		.stub(MessageLoaderUtils, "_getRemoteSettingsMessages")
		.resolves([{ id: "message_1" }]);
		spy = sandbox.spy();
		global.Downloader.prototype.downloadToDisk = spy;
		sandbox.stub(global.IOUtils, "exists").resolves(false);
		spy = sandbox.spy(global.UnstoredDownloader.prototype.download);
		global.UnstoredDownloader.prototype.download = spy;
		});
		it("should be called with the expected dir path", async () => {
		const dlSpy = sandbox.spy(global, "Downloader");
		const writeSpy = sandbox.spy(global.IOUtils, "write");

		sandbox
		.stub(global.Services.locale, "appLocaleAsBCP47")
		@@ -2692,14 +2697,39 @@ describe("ASRouter", () => {

		await MessageLoaderUtils._remoteSettingsLoader(provider, {});

		assert.calledWith(
		dlSpy,
		"main",
		"ms-language-packs",
		"browser",
		"newtab"
		assert.calledOnce(spy);
		assert.calledWithMatch(
		writeSpy,
		"asrouter.ftl", // PathUtils.join() is mocked in `unit-entry.js` and only returns the filename.
		sinon.match.any,
		{ tmpPath: "asrouter.ftl.tmp" }
		);
		});
		it("should download if local file has different size", async () => {
		global.IOUtils.exists.resolves(true);
		sandbox.stub(global.IOUtils, "stat").resolves({ size: 1337 });
		sandbox
		.stub(global.Services.locale, "appLocaleAsBCP47")
		.get(() => "en-US");

		await MessageLoaderUtils._remoteSettingsLoader(provider, {});

		assert.calledOnce(spy);
		});
		it("should not download if local file has same size", async () => {
		global.IOUtils.exists.resolves(true);
		sandbox.stub(global.IOUtils, "stat").resolves({ size: 42 });
		sandbox
		.stub(global.KintoHttpClient.prototype, "getRecord")
		.resolves({ data: { attachment: { size: 42 } } });
		sandbox
		.stub(global.Services.locale, "appLocaleAsBCP47")
		.get(() => "en-US");

		await MessageLoaderUtils._remoteSettingsLoader(provider, {});

		assert.notCalled(spy);
		});
		it("should allow fetch for known locales", async () => {
		sandbox
		.stub(global.Services.locale, "appLocaleAsBCP47")

browser/components/asrouter/tests/unit/unit-entry.js

+3 −0

Original line number	Diff line number	Diff line
		@@ -346,6 +346,9 @@ const TEST_GLOBAL = {
		getLocalProfileDir() {
		return Promise.resolve("/");
		},
		toFileURI(path) {
		return `file://${path}`;
		},
		},
		PlacesUtils: {
		get bookmarks() {

services/settings/Attachments.sys.mjs

+16 −105

Original line number	Diff line number	Diff line
		@@ -499,67 +499,6 @@ export class Downloader {
		async prune(excludeIds) {
		return this.cacheImpl.prune(excludeIds);
		}

		/**
		* @deprecated See https://bugzilla.mozilla.org/show_bug.cgi?id=1634127
		*
		* Download the record attachment into the local profile directory
		* and return a file:// URL that points to the local path.
		*
		* No-op if the file was already downloaded and not corrupted.
		*
		* @param {Object} record A Remote Settings entry with attachment.
		* @param {Object} options Some download options.
		* @param {Number} options.retries Number of times download should be retried (default: `3`)
		* @throws {Downloader.DownloadError} if the file could not be fetched.
		* @throws {Downloader.BadContentError} if the downloaded file integrity is not valid.
		* @throws {Downloader.ServerInfoError} if the server response is not valid.
		* @throws {NetworkError} if fetching the attachment fails.
		* @returns {String} the absolute file path to the downloaded attachment.
		*/
		async downloadToDisk(record, options = {}) {
		const { retries = 3 } = options;
		const {
		attachment: { filename, size, hash },
		} = record;
		const localFilePath = PathUtils.join(
		PathUtils.localProfileDir,
		...this.folders,
		filename
		);
		const localFileUrl = PathUtils.toFileURI(localFilePath);

		await this._makeDirs();

		let retried = 0;
		while (true) {
		if (
		await lazy.RemoteSettingsWorker.checkFileHash(localFileUrl, size, hash)
		) {
		return localFileUrl;
		}
		// File does not exist or is corrupted.
		if (retried > retries) {
		throw new Downloader.BadContentError(localFilePath);
		}
		try {
		// Download and write on disk.
		const buffer = await this.downloadAsBytes(record, {
		checkHash: false, // Hash will be checked on file.
		retries: 0, // Already in a retry loop.
		});
		await IOUtils.write(localFilePath, new Uint8Array(buffer), {
		tmpPath: `${localFilePath}.tmp`,
		});
		} catch (e) {
		if (retried >= retries) {
		throw e;
		}
		}
		retried++;
		}
		}

		/**
		* Download the record attachment and return its content as bytes.
		*
		@@ -609,31 +548,6 @@ export class Downloader {
		}
		}

		/**
		* @deprecated See https://bugzilla.mozilla.org/show_bug.cgi?id=1634127
		*
		* Delete the record attachment downloaded locally.
		* This is the counterpart of `downloadToDisk()`.
		* Use `deleteDownloaded()` if `download()` was used to retrieve
		* the attachment.
		*
		* No-op if the related file does not exist.
		*
		* @param record A Remote Settings entry with attachment.
		*/
		async deleteFromDisk(record) {
		const {
		attachment: { filename },
		} = record;
		const path = PathUtils.join(
		PathUtils.localProfileDir,
		...this.folders,
		filename
		);
		await IOUtils.remove(path);
		await this._rmDirs();
		}

		async _fetchAttachment(url) {
		const headers = new Headers();
		headers.set("Accept-Encoding", "gzip");
		@@ -688,25 +602,22 @@ export class Downloader {

		// Separate variable to allow tests to override this.
		static _RESOURCE_BASE_URL = "resource://app/defaults";

		async _makeDirs() {
		const dirPath = PathUtils.join(PathUtils.localProfileDir, ...this.folders);
		await IOUtils.makeDirectory(dirPath, { createAncestors: true });
		}

		async _rmDirs() {
		for (let i = this.folders.length; i > 0; i--) {
		const dirPath = PathUtils.join(
		PathUtils.localProfileDir,
		...this.folders.slice(0, i)
		);
		try {
		await IOUtils.remove(dirPath);
		} catch (e) {
		// This could fail if there's something in
		// the folder we're not permitted to remove.
		break;
		}
		}
		/**
		* A bare downloader that does not store anything in cache.
		*/
		export class UnstoredDownloader extends Downloader {
		get cacheImpl() {
		const cacheImpl = {
		get: async () => {},
		set: async () => {},
		setMultiple: async () => {},
		delete: async () => {},
		prune: async () => {},
		hasData: async () => false,
		};
		Object.defineProperty(this, "cacheImpl", { value: cacheImpl });
		return cacheImpl;
		}
		}