Skip to content

Commit 3b64a17

Browse files
Bug 1830725: Synchronous nsWindowsPackageManager::GetCampaignId blocks main thread for many seconds r=nrishel
Made GetCampaignId asynchronous to not block anything. Also fix Bug 1872933 (broken build) Differential Revision: https://phabricator.services.mozilla.com/D188520
1 parent 05f593b commit 3b64a17

File tree

5 files changed

+294
-143
lines changed

5 files changed

+294
-143
lines changed

browser/components/attribution/AttributionCode.sys.mjs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,12 @@ export var AttributionCode = {
6464
* Wrapper to pull campaign IDs from MSIX builds.
6565
* This function solely exists to make it easy to mock out for tests.
6666
*/
67-
get msixCampaignId() {
68-
return Cc["@mozilla.org/windows-package-manager;1"]
69-
.createInstance(Ci.nsIWindowsPackageManager)
70-
.getCampaignId();
67+
async msixCampaignId() {
68+
const windowsPackageManager = Cc[
69+
"@mozilla.org/windows-package-manager;1"
70+
].createInstance(Ci.nsIWindowsPackageManager);
71+
72+
return windowsPackageManager.campaignId();
7173
},
7274

7375
/**
@@ -346,7 +348,7 @@ export var AttributionCode = {
346348
)}`
347349
);
348350
let encoder = new TextEncoder();
349-
bytes = encoder.encode(encodeURIComponent(this.msixCampaignId));
351+
bytes = encoder.encode(encodeURIComponent(await this.msixCampaignId()));
350352
} else {
351353
bytes = await AttributionIOUtils.read(attributionFile.path);
352354
}

browser/components/attribution/test/browser/browser_AttributionCode_telemetry.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,17 @@ add_task(async function test_read_error() {
8080
throw new Error("read_error");
8181
};
8282

83+
// On MSIX builds, AttributionIOUtils.read is not used; AttributionCode.msixCampaignId is.
84+
// Ensure we override that as well.
85+
let oldMsixCampaignId = AttributionCode.msixCampaignId;
86+
AttributionCode.msixCampaignId = async () => {
87+
throw new Error("read_error");
88+
};
89+
8390
registerCleanupFunction(() => {
8491
AttributionIOUtils.exists = oldExists;
8592
AttributionIOUtils.read = oldRead;
93+
AttributionCode.msixCampaignId = oldMsixCampaignId;
8694
});
8795

8896
// Try to read the file

browser/components/attribution/test/xpcshell/test_AttributionCode.js

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ add_task(async () => {
2020
* to make sure we reject bad ones and accept good ones.
2121
*/
2222
add_task(async function testValidAttrCodes() {
23+
let msixCampaignIdStub = sinon.stub(AttributionCode, "msixCampaignId");
24+
2325
let currentCode = null;
2426
for (let entry of validAttrCodes) {
2527
currentCode = entry.code;
@@ -36,28 +38,32 @@ add_task(async function testValidAttrCodes() {
3638
// In real life, the attribution codes returned from Microsoft APIs
3739
// are not URI encoded, and the AttributionCode code that deals with
3840
// them expects that - so we have to simulate that as well.
39-
sinon
40-
.stub(AttributionCode, "msixCampaignId")
41-
.get(() => decodeURIComponent(currentCode));
41+
msixCampaignIdStub.callsFake(async () => decodeURIComponent(currentCode));
4242
} else {
4343
await AttributionCode.writeAttributionFile(currentCode);
4444
}
4545
AttributionCode._clearCache();
4646
let result = await AttributionCode.getAttrDataAsync();
47+
4748
Assert.deepEqual(
4849
result,
4950
entry.parsed,
5051
"Parsed code should match expected value, code was: " + currentCode
5152
);
5253
}
5354
AttributionCode._clearCache();
55+
56+
// Restore the msixCampaignId stub so that other tests don't fail stubbing it
57+
msixCampaignIdStub.restore();
5458
});
5559

5660
/**
5761
* Make sure codes with various formatting errors are not seen as valid.
5862
*/
5963
add_task(async function testInvalidAttrCodes() {
64+
let msixCampaignIdStub = sinon.stub(AttributionCode, "msixCampaignId");
6065
let currentCode = null;
66+
6167
for (let code of invalidAttrCodes) {
6268
currentCode = code;
6369

@@ -73,9 +79,7 @@ add_task(async function testInvalidAttrCodes() {
7379
continue;
7480
}
7581

76-
sinon
77-
.stub(AttributionCode, "msixCampaignId")
78-
.get(() => decodeURIComponent(currentCode));
82+
msixCampaignIdStub.callsFake(async () => decodeURIComponent(currentCode));
7983
} else {
8084
await AttributionCode.writeAttributionFile(currentCode);
8185
}
@@ -88,6 +92,9 @@ add_task(async function testInvalidAttrCodes() {
8892
);
8993
}
9094
AttributionCode._clearCache();
95+
96+
// Restore the msixCampaignId stub so that other tests don't fail stubbing it
97+
msixCampaignIdStub.restore();
9198
});
9299

93100
/**

toolkit/system/windowsPackageManager/nsIWindowsPackageManager.idl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ interface nsIWindowsPackageManager : nsISupports
2323
*/
2424
unsigned long long getInstalledDate();
2525

26-
/* Retrieves the campaignId, if any, a user's Microsoft Store install is
26+
/* Asynchronously retrieves the campaignId, if any, a user's Microsoft Store install is
2727
* associated with. These are present if the user clicked a "ms-window-store://"
2828
* or "https://" link that included a "cid" query argument the very first time
2929
* they installed the app. (This value appears to be cached forever, so
@@ -37,5 +37,6 @@ interface nsIWindowsPackageManager : nsISupports
3737
* a non-packaged build.
3838
* @throw NS_ERROR_FAILURE for any other errors
3939
*/
40-
AString getCampaignId();
40+
[implicit_jscontext]
41+
Promise campaignId();
4142
};

0 commit comments

Comments
 (0)