Sending CFR contentblocking event in an onStateChange listener is too excessive
Categories
(Firefox :: Protections UI, defect, P1)
Tracking
()
People
(Reporter: johannh, Assigned: johannh)
Details
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
[Tracking Requested - why for this release]:
Perf issue with messaging system in release
I suppose this is intended to be sent on stopped state only: https://searchfox.org/mozilla-beta/rev/50dc5e18d7429a505345f610fae358ef385e1ecb/browser/base/content/browser-siteProtections.js#1677
Instead it causes a lot of messages after stopping document load and a few on tab switch etc.
That seems completely unnecessary to me
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
These events were sent excessively on onStateChange. This patch makes it so that:
- we only listen to top-level document changes (we used to send all events from iframes)
- we don't send events unless there's tracking content on the page
- we only send a single event on tab switch instead of multiple
Pushed by jhofmann@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/46d18d723c88 Optimize sending SiteProtection:ContentBlockingEvent events. r=nhnt11
Comment 3•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Comment 4•4 years ago
|
||
Hi Johann, does this need a Beta approval request?
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 5•4 years ago
|
||
Comment on attachment 9115611 [details]
Bug 1603511 - Optimize sending SiteProtection:ContentBlockingEvent events. r=nhnt11
Beta/Release Uplift Approval Request
- User impact if declined: Perf hit from too eager CFR events
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The biggest risk would be a reduced number of CFR doorhangers shown, otherwise this patch isn't too complicated and has baked in Nightly for a while.
- String changes made/needed: None
Comment 6•4 years ago
|
||
Comment on attachment 9115611 [details]
Bug 1603511 - Optimize sending SiteProtection:ContentBlockingEvent events. r=nhnt11
approved for 72.0b9
Comment 7•4 years ago
|
||
bugherder uplift |
Description
•