Closed Bug 1745763 Opened 2 years ago Closed 1 year ago

[DNR] Prototype of static rules

Categories

(WebExtensions :: Request Handling, enhancement, P2)

enhancement
Points:
5

Tracking

(firefox109 fixed)

RESOLVED FIXED
109 Branch
Tracking Status
firefox109 --- fixed

People

(Reporter: robwu, Assigned: rpl)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [mv3-m2])

Attachments

(3 files, 1 obsolete file)

Implement static registration of rules packaged with an extension + runtime management of them:

  • rule_resources in manifest.json
  • declarativeNetRequest.getAvailableStaticRuleCount
  • declarativeNetRequest.getEnabledRulesets
  • declarativeNetRequest.updateEnabledRulesets
Points: --- → 5
Blocks: 1782685
Assignee: rob → lgreco

This draft patch adds a per-extension startupCache file for the DNR data store,
I decided to split it out from the other patch because we may consider a
different strategy (e.g. one startup cache file for all extensions) and to
move this part in its own followup bugzilla issue.

Depends on D162401

Attachment #9304094 - Attachment description: WIP: Bug 1745763 - prototype DNR rules store, static rule loading and getEnabledRulesets API method. → Bug 1745763 - prototype DNR rules store, static rule loading and getEnabledRulesets API method. r?robwu!
See Also: → 1803363
See Also: → 1803365
See Also: → 1803369
See Also: → 1803370

Comment on attachment 9304095 [details]
WIP: Bug 1745763 - prototype DNR rules store per-extension startupCache file.

Revision D162402 was moved to bug 1803365. Setting attachment 9304095 [details] to obsolete.

Attachment #9304095 - Attachment is obsolete: true
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/3217096eece8
prototype DNR rules store, static rule loading and getEnabledRulesets API method. r=robwu
https://hg.mozilla.org/integration/autoland/rev/f588f8df3989
Implement DNR updateEnabledRulesets API method. r=robwu
https://hg.mozilla.org/integration/autoland/rev/84c1f1c3fd36
Implement DNR getAvailableStaticRuleCount. r=robwu

Backed out 3 changesets (Bug 1745763) for causing xpcshell failures on test_ext_dnr_static_rules.js.
Backout link
Push with failures <--> X7
Failure Log

Flags: needinfo?(lgreco)

(In reply to Marian-Vasile Laza from comment #7)

Backed out 3 changesets (Bug 1745763) for causing xpcshell failures on test_ext_dnr_static_rules.js.
Backout link
Push with failures <--> X7
Failure Log

I'm on it, I looked to the failure logs and noticed that the timeout failure was triggered in a tsan build.
I had a theory about what was going on and confirmed it with some more push to try focused on the job running on the tsan build:

  • the tsan failure is triggered by the additions to the test_ext_dnr_static_rules.js test file by the last of the 3 patches (D163084), where we have introduced the restriction of the maximum amount of static rules allowed and a new test case that is testing the behaviors when the extension reached the maximum amount of static rules being enabled
  • one of the test helpers introduced in earlier patches is explicitly asserting with a deepEqual that the rules being loaded matches the rules expected to be loaded, which would produce useful failure logs when the number of rules being asserted is small enough for the output to be readable and manageable, but with the new test loading as many rules as allowed that assertion produces a huge assertion message and takes quiet a long time, which on slower builds like tsan builds may become much more noticeable than in an optimized build
  • on slower builds like the tsan builds, the time required to assert with deep equal all rules takes long enough that the event page from the test extensions may have got to be suspended on idle by the time the test case is using the event page again to run some DNR API calls for the assertions that follows in that test case, and that is likely triggering the timeout due to a test message being awaited on that is never going to be send (and exchanging test messages doesn't expect the idle timeout, which is on purpose and expected).

Locally I've prepared an update for the third patch (some small tweaks only on the test helper side) that I'm currently verifying in a push to try, once that try push confirms that the timeout failure is not triggered anymore I'll update the patch on phabricator and push to autoland the updated patches.

Flags: needinfo?(lgreco)
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/cff2252cc41b
prototype DNR rules store, static rule loading and getEnabledRulesets API method. r=robwu
https://hg.mozilla.org/integration/autoland/rev/29cc33a13950
Implement DNR updateEnabledRulesets API method. r=robwu
https://hg.mozilla.org/integration/autoland/rev/d51a61a52a3b
Implement DNR getAvailableStaticRuleCount. r=robwu
Regressions: 1803801
See Also: → 1809721
Blocks: 1816319
Blocks: 1820870
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: