Bug 1733535 - Replace OS.File with IOUtils in the attribution code. r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D128715
This commit is contained in:
Evgenia Kotovich 2021-10-25 08:38:52 +00:00
parent 87b96afd76
commit b7899c1c92
5 changed files with 51 additions and 27 deletions

View File

@ -3,7 +3,16 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict"; "use strict";
var EXPORTED_SYMBOLS = ["AttributionCode"]; var EXPORTED_SYMBOLS = ["AttributionCode", "AttributionIOUtils"];
/**
* This is a policy object used to override behavior for testing.
*/
const AttributionIOUtils = {
write: async (path, bytes) => IOUtils.write(path, bytes),
read: async path => IOUtils.read(path),
exists: async path => IOUtils.exists(path),
};
const { XPCOMUtils } = ChromeUtils.import( const { XPCOMUtils } = ChromeUtils.import(
"resource://gre/modules/XPCOMUtils.jsm" "resource://gre/modules/XPCOMUtils.jsm"
@ -13,7 +22,6 @@ ChromeUtils.defineModuleGetter(
"AppConstants", "AppConstants",
"resource://gre/modules/AppConstants.jsm" "resource://gre/modules/AppConstants.jsm"
); );
ChromeUtils.defineModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm");
ChromeUtils.defineModuleGetter( ChromeUtils.defineModuleGetter(
this, this,
"Services", "Services",
@ -124,7 +132,7 @@ var AttributionCode = {
// Ignore the exception due to a directory that already exists. // Ignore the exception due to a directory that already exists.
} }
let bytes = new TextEncoder().encode(code); let bytes = new TextEncoder().encode(code);
await OS.File.writeAtomic(file.path, bytes); await AttributionIOUtils.write(file.path, bytes);
}, },
/** /**
@ -259,7 +267,7 @@ var AttributionCode = {
if ( if (
AppConstants.platform == "macosx" && AppConstants.platform == "macosx" &&
!(await OS.File.exists(attributionFile.path)) !(await AttributionIOUtils.exists(attributionFile.path))
) { ) {
log.debug( log.debug(
`getAttrDataAsync: macOS && !exists("${attributionFile.path}")` `getAttrDataAsync: macOS && !exists("${attributionFile.path}")`
@ -317,9 +325,9 @@ var AttributionCode = {
let bytes; let bytes;
try { try {
bytes = await OS.File.read(attributionFile.path); bytes = await AttributionIOUtils.read(attributionFile.path);
} catch (ex) { } catch (ex) {
if (ex instanceof OS.File.Error && ex.becauseNoSuchFile) { if (ex instanceof DOMException && ex.name == "NotFoundError") {
log.debug( log.debug(
`getAttrDataAsync: !exists("${ `getAttrDataAsync: !exists("${
attributionFile.path attributionFile.path
@ -379,7 +387,7 @@ var AttributionCode = {
*/ */
async deleteFileAsync() { async deleteFileAsync() {
try { try {
await OS.File.remove(this.attributionFile.path); await IOUtils.remove(this.attributionFile.path);
} catch (ex) { } catch (ex) {
// The attribution file may already have been deleted, // The attribution file may already have been deleted,
// or it may have never been installed at all; // or it may have never been installed at all;

View File

@ -54,6 +54,7 @@ function getQuarantineDatabasePath() {
* @throws NS_ERROR_UNEXPECTED if there is a quarantine GUID, but it is malformed. * @throws NS_ERROR_UNEXPECTED if there is a quarantine GUID, but it is malformed.
*/ */
async function getQuarantineAttributes(path) { async function getQuarantineAttributes(path) {
// TODO: Bug 1736331 replace OS.File.macGetXAttr with an alternative.
let bytes = await OS.File.macGetXAttr(path, "com.apple.quarantine"); let bytes = await OS.File.macGetXAttr(path, "com.apple.quarantine");
if (!bytes) { if (!bytes) {
throw new Components.Exception( throw new Components.Exception(

View File

@ -6,6 +6,9 @@ ChromeUtils.defineModuleGetter(
const { MacAttribution } = ChromeUtils.import( const { MacAttribution } = ChromeUtils.import(
"resource:///modules/MacAttribution.jsm" "resource:///modules/MacAttribution.jsm"
); );
const { AttributionIOUtils } = ChromeUtils.import(
"resource:///modules/AttributionCode.jsm"
);
const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm"); const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
async function assertCacheExistsAndIsEmpty() { async function assertCacheExistsAndIsEmpty() {
@ -16,11 +19,11 @@ async function assertCacheExistsAndIsEmpty() {
); );
histogram.clear(); histogram.clear();
ok(await OS.File.exists(AttributionCode.attributionFile.path)); ok(await AttributionIOUtils.exists(AttributionCode.attributionFile.path));
Assert.deepEqual( Assert.deepEqual(
"", "",
new TextDecoder().decode( new TextDecoder().decode(
await OS.File.read(AttributionCode.attributionFile.path) await AttributionIOUtils.read(AttributionCode.attributionFile.path)
) )
); );
@ -49,21 +52,19 @@ add_task(async function test_write_error() {
"BROWSER_ATTRIBUTION_ERRORS" "BROWSER_ATTRIBUTION_ERRORS"
); );
let oldExists = AttributionIOUtils.exists;
let oldWrite = AttributionIOUtils.write;
try { try {
// Clear any existing telemetry // Clear any existing telemetry
histogram.clear(); histogram.clear();
// Force the file to not exist and then cause a write error. This is delicate // Force the file to not exist and then cause a write error. This is delicate
// because various background tasks may invoke `OS.File.writeAtomic` while // because various background tasks may invoke `IOUtils.writeAtomic` while
// this test is running. Be careful to only stub the one call. // this test is running. Be careful to only stub the one call.
const writeAtomic = sandbox.stub(OS.File, "writeAtomic"); AttributionIOUtils.exists = () => false;
writeAtomic AttributionIOUtils.write = () => {
.withArgs( throw new Error("write_error");
sinon.match(AttributionCode.attributionFile.path), };
sinon.match.any
)
.throws(() => new Error("write_error"));
OS.File.writeAtomic.callThrough();
// Try to read the attribution code. // Try to read the attribution code.
let result = await AttributionCode.getAttrDataAsync(); let result = await AttributionCode.getAttrDataAsync();
@ -75,6 +76,8 @@ add_task(async function test_write_error() {
TelemetryTestUtils.assertHistogram(histogram, INDEX_WRITE_ERROR, 1); TelemetryTestUtils.assertHistogram(histogram, INDEX_WRITE_ERROR, 1);
} finally { } finally {
AttributionIOUtils.exists = oldExists;
AttributionIOUtils.write = oldWrite;
await AttributionCode.deleteFileAsync(); await AttributionCode.deleteFileAsync();
AttributionCode._clearCache(); AttributionCode._clearCache();
histogram.clear(); histogram.clear();
@ -214,6 +217,7 @@ add_task(async function test_broken_referrer() {
generateQuarantineGUID(), generateQuarantineGUID(),
].join(";"); ].join(";");
let bytes = new TextEncoder().encode(string); let bytes = new TextEncoder().encode(string);
// TODO: Bug 1736331 replace OS.File.macSetXAttr with an alternative.
await OS.File.macSetXAttr( await OS.File.macSetXAttr(
MacAttribution.applicationPath, MacAttribution.applicationPath,
"com.apple.quarantine", "com.apple.quarantine",

View File

@ -3,7 +3,9 @@ ChromeUtils.defineModuleGetter(
"TelemetryTestUtils", "TelemetryTestUtils",
"resource://testing-common/TelemetryTestUtils.jsm" "resource://testing-common/TelemetryTestUtils.jsm"
); );
const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm"); const { AttributionIOUtils } = ChromeUtils.import(
"resource:///modules/AttributionCode.jsm"
);
add_task(async function test_parse_error() { add_task(async function test_parse_error() {
if (AppConstants.platform == "macosx") { if (AppConstants.platform == "macosx") {
@ -62,7 +64,7 @@ add_task(async function test_read_error() {
const histogram = Services.telemetry.getHistogramById( const histogram = Services.telemetry.getHistogramById(
"BROWSER_ATTRIBUTION_ERRORS" "BROWSER_ATTRIBUTION_ERRORS"
); );
const sandbox = sinon.createSandbox();
// Delete the file to trigger a read error // Delete the file to trigger a read error
await AttributionCode.deleteFileAsync(); await AttributionCode.deleteFileAsync();
AttributionCode._clearCache(); AttributionCode._clearCache();
@ -70,10 +72,19 @@ add_task(async function test_read_error() {
histogram.clear(); histogram.clear();
// Force the file to exist but then cause a read error // Force the file to exist but then cause a read error
const exists = sandbox.stub(OS.File, "exists"); let oldExists = AttributionIOUtils.exists;
exists.resolves(true); AttributionIOUtils.exists = () => true;
const read = sandbox.stub(OS.File, "read");
read.throws(() => new Error("read_error")); let oldRead = AttributionIOUtils.read;
AttributionIOUtils.read = () => {
throw new Error("read_error");
};
registerCleanupFunction(() => {
AttributionIOUtils.exists = oldExists;
AttributionIOUtils.read = oldRead;
});
// Try to read the file // Try to read the file
await AttributionCode.getAttrDataAsync(); await AttributionCode.getAttrDataAsync();
@ -82,5 +93,4 @@ add_task(async function test_read_error() {
// Clear any existing telemetry // Clear any existing telemetry
histogram.clear(); histogram.clear();
sandbox.restore();
}); });

View File

@ -84,7 +84,6 @@ async function setupStubs() {
const { AppConstants } = ChromeUtils.import( const { AppConstants } = ChromeUtils.import(
"resource://gre/modules/AppConstants.jsm" "resource://gre/modules/AppConstants.jsm"
); );
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm"); const { sinon } = ChromeUtils.import("resource://testing-common/Sinon.jsm");
// This depends on the caller to invoke it by name. We do try to // This depends on the caller to invoke it by name. We do try to
@ -117,5 +116,7 @@ async function setupStubs() {
// directory for the attribution file, needed on both macOS and Windows. We // directory for the attribution file, needed on both macOS and Windows. We
// don't ignore existing paths because we're inside a temporary directory: // don't ignore existing paths because we're inside a temporary directory:
// this should never be invoked twice for the same test. // this should never be invoked twice for the same test.
await OS.File.makeDir(applicationFile.path, { from: do_get_tempdir().path }); await IOUtils.makeDirectory(applicationFile.path, {
from: do_get_tempdir().path,
});
} }