stylo: "layout.css.stylo-blocklist.enabled" pref's default value should be false

RESOLVED FIXED in Firefox 57

Status

()

defect
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: cpeterson, Assigned: jeremychen)

Tracking

57 Branch
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox56 unaffected, firefox57+ fixed, firefox58 fixed)

Details

Attachments

(1 attachment)

[Tracking Requested - why for this release]:

This is a minor Stylo performance bug we should fix in Beta 57.

We ship an empty Stylo domain blocklist in Beta 57 and Nightly 58. We included a test domain that we removed last week in bug 1409958.

Since the default blocklist ("layout.css.stylo-blocklist.blocked_domains" pref) is empty, we should set the "layout.css.stylo-blocklist.enabled" pref = false so we don't waste time trying to read the empty blocklist pref. If we need to ship our Stylo system add-on to update the blocklist pref, it can also set the enabled pref = true.
Xidorn, do you have a moment to fix and uplift this one-line pref change?

We should disable the Stylo blocklist pref to avoid the slow path of trying to parse the (empty) "blocked_domains" pref string. If we need to update the blocklist after release, the Stylo system add-on can set the enabled pref = true at the same time.
Flags: needinfo?(xidorn+moz)
This would be a good one to fix, tracked for 57.
Since this is not handled yet so far, and I kind of have some time slot for creating this simple patch, so, I'm going to upload a patch here.

Hi xidorn, could you help push this simple patch for me once you finish the review?

Hi Chris, could you request the uplift for me once this patch get merged to m-c?
Flags: needinfo?(cpeterson)
Comment on attachment 8922485 [details]
Bug 1411517 - stylo: "layout.css.stylo-blocklist.enabled" pref's default value should be false.

https://reviewboard.mozilla.org/r/193562/#review198780
Attachment #8922485 - Flags: review?(xidorn+moz) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/96f54c7e0a1a
stylo: "layout.css.stylo-blocklist.enabled" pref's default value should be false. r=xidorn
Comment on attachment 8922485 [details]
Bug 1411517 - stylo: "layout.css.stylo-blocklist.enabled" pref's default value should be false.

Approval Request Comment
[Feature/Bug causing the regression]: stylo blocklist
[User impact if declined]: maybe slightly slower than it should be, but no big impact
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: just flip a pref to skip a bunch of new code path which we are not using at the moment
[String changes made/needed]: n/a
Flags: needinfo?(xidorn+moz)
Flags: needinfo?(cpeterson)
Attachment #8922485 - Flags: approval-mozilla-beta?
(In reply to Jeremy Chen [:jeremychen] UTC+8 (away 24 Oct – 12 Nov) from comment #3)
> Since this is not handled yet so far, and I kind of have some time slot for
> creating this simple patch, so, I'm going to upload a patch here.

Thanks for helping while you are on your trip! :)
https://hg.mozilla.org/mozilla-central/rev/96f54c7e0a1a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee: nobody → jeremychen
Comment on attachment 8922485 [details]
Bug 1411517 - stylo: "layout.css.stylo-blocklist.enabled" pref's default value should be false.

Beta57+
Attachment #8922485 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.