commit 50ae7d88b8df9c9cc0eef01980485883350bda2a
parent 50e377eea9254676916a682438187fe1a7f20ae2
Author: Rob Wu <rob@robwu.nl>
Date: Fri, 9 Jan 2026 12:53:51 +0000
Bug 2006233 - Accept partially initialized DNR store on browser startup r=rpl
Differential Revision: https://phabricator.services.mozilla.com/D278132
Diffstat:
3 files changed, 125 insertions(+), 24 deletions(-)
diff --git a/toolkit/components/extensions/ExtensionDNRStore.sys.mjs b/toolkit/components/extensions/ExtensionDNRStore.sys.mjs
@@ -1174,6 +1174,7 @@ class RulesetsStore {
let storeData = this.#getDefaults(extension);
this._data.set(extension.uuid, storeData);
this._dataPromises.set(extension.uuid, Promise.resolve(storeData));
+ this._startupCacheData.delete(extension.uuid);
this.unloadOnShutdown(extension);
}
}
@@ -1199,14 +1200,6 @@ class RulesetsStore {
#promiseStartupCacheLoaded() {
if (!this._ensureCacheLoaded) {
- if (this._data.size) {
- return Promise.reject(
- new Error(
- "Unexpected non-empty DNRStore data. DNR startupCache data load aborted."
- )
- );
- }
-
const startTime = ChromeUtils.now();
const timerId = Glean.extensionsApisDnr.startupCacheReadTime.start();
this._ensureCacheLoaded = (async () => {
@@ -1224,16 +1217,16 @@ class RulesetsStore {
await IOUtils.remove(cacheFilePath, { ignoreAbsent: true });
return;
}
- if (this._data.size) {
- Cu.reportError(
- `Unexpected non-empty DNRStore data. DNR startupCache data load dropped.`
- );
- return;
- }
for (const [
extUUID,
cacheStoreData,
] of decodedData.cacheData.entries()) {
+ if (this._data.has(extUUID)) {
+ // Already initialized elsewhere (#initExtension), initially with
+ // empty data, but potentially already expanded with more data by
+ // now. Ignore this startupcache entry.
+ continue;
+ }
if (StoreData.isStaleCacheEntry(extUUID, cacheStoreData)) {
StoreData.clearLastUpdateTagPref(extUUID);
continue;
@@ -1363,7 +1356,21 @@ class RulesetsStore {
// (because the StoreData instances get converted into plain objects when
// serialized into the startupCache structured clone blobs).
async #readStoreDataFromStartupCache(extension) {
+ if (this._data.has(extension.uuid)) {
+ // This should be unreachable.
+ Cu.reportError(
+ `Unexpected non-empty DNRStore data. DNR startupCache data load aborted for ${extension.id}.`
+ );
+ return;
+ }
await this.#promiseStartupCacheLoaded();
+ if (this._data.has(extension.uuid)) {
+ // This should be unreachable.
+ Cu.reportError(
+ `Unexpected non-empty DNRStore data. DNR startupCache data load dropped for ${extension.id}.`
+ );
+ return;
+ }
if (!this._startupCacheData.has(extension.uuid)) {
Glean.extensionsApisDnr.startupCacheEntries.miss.add(1);
diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_dnr_dynamic_rules.js b/toolkit/components/extensions/test/xpcshell/test_ext_dnr_dynamic_rules.js
@@ -967,14 +967,27 @@ add_task(async function test_register_dynamic_rules_after_restart() {
// been detected at the previous session. Caching too much or too little in
// StartupCache will trigger test failures in assertStoreReadsSinceLastCall.
const sandboxStoreSpies = sinon.createSandbox();
- const dnrStore = ExtensionDNRStore._getStoreForTesting();
- const spyReadDNRStore = sandboxStoreSpies.spy(dnrStore, "_readData");
+ let dnrStore = ExtensionDNRStore._getStoreForTesting();
+ const spyReadDNRStore = sandboxStoreSpies.spy(
+ Object.getPrototypeOf(dnrStore),
+ "_readData"
+ );
function assertStoreReadsSinceLastCall(expectedCount, description) {
equal(spyReadDNRStore.callCount, expectedCount, description);
spyReadDNRStore.resetHistory();
}
+ async function promiseRestartManagerWithRealisticDNRStoreInMemory() {
+ await AddonTestUtils.promiseShutdownManager();
+ // Recreate the DNR store to ensure that we test a realistic state without
+ // cached in-memory state.
+ dnrStore = ExtensionDNRStore._recreateStoreForTesting();
+ // After the store is recreated for testing, we will still be able to track
+ // reads, because spyReadDNRStore was attached to the prototype above.
+ await AddonTestUtils.promiseStartupManager();
+ }
+
let { extension } = await runAsDNRExtension({
unloadTestAtEnd: false,
id: "test-dnr-restart-and-rule-registration-changes@test-extension",
@@ -1014,7 +1027,7 @@ add_task(async function test_register_dynamic_rules_after_restart() {
});
await extension.awaitMessage("onInstalled");
assertStoreReadsSinceLastCall(1, "Read once at initial startup");
- await promiseRestartManager();
+ await promiseRestartManagerWithRealisticDNRStoreInMemory();
await extension.awaitMessage("onStartup");
assertStoreReadsSinceLastCall(0, "Read skipped due to hasDynamicRules=false");
@@ -1030,7 +1043,7 @@ add_task(async function test_register_dynamic_rules_after_restart() {
await callTestMessageHandler(extension, "zero_to_one_rule");
assertStoreReadsSinceLastCall(0, "No further reads before restart");
- await promiseRestartManager();
+ await promiseRestartManagerWithRealisticDNRStoreInMemory();
await extension.awaitMessage("onStartup");
// Regression test 2: before bug 1921353 was fixed, even with a fix to the
@@ -1047,7 +1060,7 @@ add_task(async function test_register_dynamic_rules_after_restart() {
// the initialization is skipped as expected.
await callTestMessageHandler(extension, "one_to_zero_rules");
- await promiseRestartManager();
+ await promiseRestartManagerWithRealisticDNRStoreInMemory();
await extension.awaitMessage("onStartup");
assertStoreReadsSinceLastCall(0, "Read skipped because rules were cleared");
equal(
@@ -1059,7 +1072,47 @@ add_task(async function test_register_dynamic_rules_after_restart() {
// not expect another read from disk.
assertStoreReadsSinceLastCall(0, "Read still skipped despite API call");
+ // Regression test for bug 2006233: The skipped read (for extension without
+ // enabled rules) should not prevent the read for extension2 (with rules).
+ const { extension: extension2 } = await runAsDNRExtension({
+ unloadTestAtEnd: false,
+ id: "test-dnr-restart-with-rules-after-one-without@test-extension",
+ background: () => {
+ const dnr = browser.declarativeNetRequest;
+
+ browser.runtime.onInstalled.addListener(async () => {
+ await dnr.updateDynamicRules({
+ addRules: [{ id: 1, condition: {}, action: { type: "block" } }],
+ });
+ browser.test.sendMessage("dynamic_rules_registered_on_install");
+ });
+ browser.runtime.onStartup.addListener(async () => {
+ browser.test.assertEq(
+ 1,
+ (await dnr.getDynamicRules()).length,
+ "Rule still registered after restart"
+ );
+ browser.test.sendMessage("dynamic_rules_still_registered");
+ });
+ },
+ });
+ await extension2.awaitMessage("dynamic_rules_registered_on_install");
+ assertStoreReadsSinceLastCall(1, "Read for new extension");
+
+ await dnrStore.waitSaveCacheDataForTesting();
+
+ await promiseRestartManagerWithRealisticDNRStoreInMemory();
+ await extension.awaitMessage("onStartup");
+ await extension2.awaitMessage("dynamic_rules_still_registered");
+ assertStoreReadsSinceLastCall(1, "Read due to extension2 with dynamic rules");
+
+ ok(
+ dnrStore._data.get(extension2.uuid).isFromStartupCache(),
+ "extension2 StoreData should be initialized from startup cache"
+ );
+
await extension.unload();
+ await extension2.unload();
sandboxStoreSpies.restore();
});
diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_dnr_static_rules.js b/toolkit/components/extensions/test/xpcshell/test_ext_dnr_static_rules.js
@@ -332,14 +332,27 @@ add_task(async function test_enable_disabled_static_rules_after_restart() {
// been detected at the previous session. Caching too much or too little in
// StartupCache will trigger test failures in assertStoreReadsSinceLastCall.
const sandboxStoreSpies = sinon.createSandbox();
- const dnrStore = ExtensionDNRStore._getStoreForTesting();
- const spyReadDNRStore = sandboxStoreSpies.spy(dnrStore, "_readData");
+ let dnrStore = ExtensionDNRStore._getStoreForTesting();
+ const spyReadDNRStore = sandboxStoreSpies.spy(
+ Object.getPrototypeOf(dnrStore),
+ "_readData"
+ );
function assertStoreReadsSinceLastCall(expectedCount, description) {
equal(spyReadDNRStore.callCount, expectedCount, description);
spyReadDNRStore.resetHistory();
}
+ async function promiseRestartManagerWithRealisticDNRStoreInMemory() {
+ await AddonTestUtils.promiseShutdownManager();
+ // Recreate the DNR store to ensure that we test a realistic state without
+ // cached in-memory state.
+ dnrStore = ExtensionDNRStore._recreateStoreForTesting();
+ // After the store is recreated for testing, we will still be able to track
+ // reads, because spyReadDNRStore was attached to the prototype above.
+ await AddonTestUtils.promiseStartupManager();
+ }
+
const rule_resources = [
{
id: "ruleset_initially_disabled",
@@ -359,7 +372,7 @@ add_task(async function test_enable_disabled_static_rules_after_restart() {
await extension.awaitMessage("bgpage:ready");
assertStoreReadsSinceLastCall(1, "Read once at initial startup");
- await AddonTestUtils.promiseRestartManager();
+ await promiseRestartManagerWithRealisticDNRStoreInMemory();
// Note: ordinarily, event pages do not wake up after a browser restart,
// unless a relevant event such as runtime.onStartup is triggered. But as
// noted in bug 1822735, "event pages without any event listeners" will be
@@ -393,7 +406,7 @@ add_task(async function test_enable_disabled_static_rules_after_restart() {
await assertDNRGetEnabledRulesets(extension, ["ruleset_initially_disabled"]);
assertStoreReadsSinceLastCall(0, "No further reads before restart");
- await AddonTestUtils.promiseRestartManager();
+ await promiseRestartManagerWithRealisticDNRStoreInMemory();
await extension.awaitMessage("bgpage:ready");
// Regression test 2: before bug 1921353 was fixed, even with a fix to the
@@ -413,7 +426,7 @@ add_task(async function test_enable_disabled_static_rules_after_restart() {
await extension.awaitMessage("updateEnabledRulesets:done");
await assertDNRGetEnabledRulesets(extension, []);
- await AddonTestUtils.promiseRestartManager();
+ await promiseRestartManagerWithRealisticDNRStoreInMemory();
await extension.awaitMessage("bgpage:ready");
assertStoreReadsSinceLastCall(0, "Read skipped because rules were disabled");
await assertDNRGetEnabledRulesets(extension, []);
@@ -421,7 +434,35 @@ add_task(async function test_enable_disabled_static_rules_after_restart() {
// so we do not expect another read from disk.
assertStoreReadsSinceLastCall(0, "Read still skipped despite API call");
+ // Regression test for bug 2006233: The skipped read (for extension without
+ // enabled rules) should not prevent the read for extension2 (with rules).
+ const extension2 = ExtensionTestUtils.loadExtension(
+ getDNRExtension({
+ rule_resources: [{ id: "my_ruleset", enabled: true, path: "rules.json" }],
+ files: { "rules.json": JSON.stringify([getDNRRule()]) },
+ id: "dnr@initially-enabled",
+ })
+ );
+ await extension2.startup();
+ await extension2.awaitMessage("bgpage:ready");
+ assertStoreReadsSinceLastCall(1, "Read for new extension");
+
+ await dnrStore.waitSaveCacheDataForTesting();
+
+ await promiseRestartManagerWithRealisticDNRStoreInMemory();
+
+ await extension.awaitMessage("bgpage:ready");
+ await extension2.awaitMessage("bgpage:ready");
+ assertStoreReadsSinceLastCall(1, "Read due to extension2 with enabled rules");
+ await assertDNRGetEnabledRulesets(extension2, ["my_ruleset"]);
+
+ ok(
+ dnrStore._data.get(extension2.uuid).isFromStartupCache(),
+ "extension2 StoreData should be initialized from startup cache"
+ );
+
await extension.unload();
+ await extension2.unload();
sandboxStoreSpies.restore();
});
