mirror of
https://github.com/mozilla/gecko-dev.git
synced 2025-03-04 15:51:37 +00:00
Bug 1823390 - Replace extension.readJSON with fetch to reduce perf impact of loading the DNR ruleset. r=robwu
`DNRStore`'s `#getManifestStaticRulesets` method is currently using `extension.readJSON` to load the JSON files for the DNR static rulesets enabled, but internally `extension.readJSON` calls `String.prototype.replace` with a regular expression to remove inline comments from the JSON files data retrieved and that have a noticebeable impact due to the size that the DNR rulesets size may likely have (especially compared to the usual size of manifest.json and locale files may have). Given that Chrome doesn't support inline comments in DNR rulesets, we can consider replacing the call to `extension.readJSON` with `fetch`, which wouldn't allow inline comments in the rulesets JSON files but it would have a smaller impact due to not going through a call to `String.prototype.replace` and through the js RegExp evaluations. Alternatively we may consider keeping the call to `extension.readJSON` but tweak it to make sure we opt out from the call to `String.prototype.replace` for this particular call (by also tweaking readJSON method to allow that). Differential Revision: https://phabricator.services.mozilla.com/D172604
This commit is contained in:
parent
f0025f6986
commit
f34bba48bf
@ -582,9 +582,6 @@ class RulesetsStore {
|
||||
} = lazy.ExtensionDNRLimits;
|
||||
|
||||
for (let [idx, { id, enabled, path }] of ruleResources.entries()) {
|
||||
// Retrieve the file path from the normalized path.
|
||||
path = Services.io.newURI(path).filePath;
|
||||
|
||||
// If passed enabledRulesetIds is used to determine if the enabled
|
||||
// rules in the manifest should be overridden from the list of
|
||||
// enabled static rulesets stored on disk.
|
||||
@ -615,13 +612,15 @@ class RulesetsStore {
|
||||
|
||||
const rawRules =
|
||||
enabled &&
|
||||
(await extension.readJSON(path).catch(err => {
|
||||
Cu.reportError(err);
|
||||
enabled = false;
|
||||
extension.packagingError(
|
||||
`Reading declarative_net_request static rules file ${path}: ${err.message}`
|
||||
);
|
||||
}));
|
||||
(await fetch(path)
|
||||
.then(res => res.json())
|
||||
.catch(err => {
|
||||
Cu.reportError(err);
|
||||
enabled = false;
|
||||
extension.packagingError(
|
||||
`Reading declarative_net_request static rules file ${path}: ${err.message}`
|
||||
);
|
||||
}));
|
||||
|
||||
// Skip rulesets that are not enabled or can't be enabled (e.g. if we got error on loading or
|
||||
// parsing the rules JSON file).
|
||||
|
@ -700,18 +700,96 @@ add_task(async function test_ruleset_validation() {
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
description: "invalid ruleset JSON - unexpected comments",
|
||||
rule_resources: [
|
||||
{
|
||||
id: "invalid_ruleset_with_comments",
|
||||
path: "invalid_ruleset_with_comments.json",
|
||||
enabled: true,
|
||||
},
|
||||
],
|
||||
files: {
|
||||
"invalid_ruleset_with_comments.json":
|
||||
"/* an unexpected inline comment */\n[]",
|
||||
},
|
||||
expectInstallFailed: false,
|
||||
expected: [
|
||||
{
|
||||
message: /Reading declarative_net_request .*invalid_ruleset_with_comments\.json: JSON.parse: unexpected character/,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
description: "invalid ruleset JSON - empty string",
|
||||
rule_resources: [
|
||||
{
|
||||
id: "invalid_ruleset_emptystring",
|
||||
path: "invalid_ruleset_emptystring.json",
|
||||
enabled: true,
|
||||
},
|
||||
],
|
||||
files: {
|
||||
"invalid_ruleset_emptystring.json": JSON.stringify(""),
|
||||
},
|
||||
expectInstallFailed: false,
|
||||
expected: [
|
||||
{
|
||||
message: /Reading declarative_net_request .*invalid_ruleset_emptystring\.json: rules file must contain an Array/,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
description: "invalid ruleset JSON - object",
|
||||
rule_resources: [
|
||||
{
|
||||
id: "invalid_ruleset_object",
|
||||
path: "invalid_ruleset_object.json",
|
||||
enabled: true,
|
||||
},
|
||||
],
|
||||
files: {
|
||||
"invalid_ruleset_object.json": JSON.stringify({}),
|
||||
},
|
||||
expectInstallFailed: false,
|
||||
expected: [
|
||||
{
|
||||
message: /Reading declarative_net_request .*invalid_ruleset_object\.json: rules file must contain an Array/,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
description: "invalid ruleset JSON - null",
|
||||
rule_resources: [
|
||||
{
|
||||
id: "invalid_ruleset_null",
|
||||
path: "invalid_ruleset_null.json",
|
||||
enabled: true,
|
||||
},
|
||||
],
|
||||
files: {
|
||||
"invalid_ruleset_null.json": JSON.stringify(null),
|
||||
},
|
||||
expectInstallFailed: false,
|
||||
expected: [
|
||||
{
|
||||
message: /Reading declarative_net_request .*invalid_ruleset_null\.json: rules file must contain an Array/,
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
for (const {
|
||||
description,
|
||||
declarative_net_request,
|
||||
rule_resources,
|
||||
files,
|
||||
expected,
|
||||
expectInstallFailed = true,
|
||||
} of invalidRulesetIdCases) {
|
||||
info(`Test manifest validation: ${description}`);
|
||||
let extension = ExtensionTestUtils.loadExtension(
|
||||
getDNRExtension({ rule_resources, declarative_net_request })
|
||||
getDNRExtension({ rule_resources, declarative_net_request, files })
|
||||
);
|
||||
|
||||
const { messages } = await AddonTestUtils.promiseConsoleOutput(async () => {
|
||||
|
Loading…
x
Reference in New Issue
Block a user