Closed Bug 1634032 Opened 10 months ago Closed 8 months ago

Lazify the protection panel popup

Categories

(Firefox :: Site Identity, task, P3)

task

Tracking

()

RESOLVED FIXED
Firefox 80
Tracking Status
firefox77 --- wontfix
firefox80 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fxperf:p2])

Attachments

(1 file, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #1634031 +++

These are in browser/components/controlcenter/content/identityPanel.inc.xhtml and browser/components/controlcenter/content/protectionsPanel.inc.xhtml . Unfortunately, there are a few different ways that we access these panels or parts of these panels in the course of page loads / tabswitches that would need to be lazified in order for this not to be delazified immediately (or almost immediately) on pageload, so it's not as simple as just shoving them in a <html:template> tag and getting on with it.

Johann, at this point, it looks like we hide identity panel on navigation, but not the protections panel (though we do hide it when tabswitching). Is there a particular reason for this, or could we assume that we hide both in both cases, and can update their contents onpopupshowing instead of in response to navigation/tabswitches happening?

Ni for comment #0

Flags: needinfo?(jhofmann)

I don't think there's a good reason for this. My first hypothesis was that it might interfere with the minipanel/toast/whatever you want to call it that we show when the ETP toggle switch is used, but we seem to be manually hiding it before initiating the reload (e.g. https://searchfox.org/mozilla-central/rev/2bfe3415fb3a2fba9b1c694bc0b376365e086927/browser/base/content/browser-siteProtections.js#1999).

Yeah I feel like we should feel free to make the popup hide more often if it means we can shave off some perf from lazifying it. However, I'm trying to understand what that means exactly, what are you trying to improve? AFAIK these panels are not updated in any way until they are being opened, i.e. we are not rendering their contents in the background or anything, right?

Flags: needinfo?(jhofmann)

BTW, FWIW, the fact that the panel doesn't react directly to navigation is an invisible bug. Invisible in the sense that it only really manifests when popup auto-hide is disabled. When this is the case, e.g. navigating to reddit from a google search while the protections panel is open results in the protections panel header persisting "google.com" in the text, while the rest of the panel reacts to content blocking events and updates itself.

(In reply to Johann Hofmann [:johannh] from comment #3)

Yeah I feel like we should feel free to make the popup hide more often if it means we can shave off some perf from lazifying it. However, I'm trying to understand what that means exactly, what are you trying to improve?

Window opening / startup time, based on the overhead of creating / running code for all the custom elements of panels and their contents.

AFAIK these panels are not updated in any way until they are being opened, i.e. we are not rendering their contents in the background or anything, right?

They are updated, unfortunately. See the browser-siteIdentity.js and browser-siteProtections.js parts of https://hg.mozilla.org/try/rev/49fe74f15ca36743f8815c1dec683e1352793971 . (Not all of those if conditions may be necessary, I just hacked things up quickly to see how badly things would break, and I saw errors caused by passing null to PanelMultiview.hidePopup, so I if'd all of them out. But yeah, we try to access the popup and its contents when it's not open, unfortunately.)

(In reply to Nihanth Subramanya [:nhnt11] from comment #4)

BTW, FWIW, the fact that the panel doesn't react directly to navigation is an invisible bug. Invisible in the sense that it only really manifests when popup auto-hide is disabled. When this is the case, e.g. navigating to reddit from a google search while the protections panel is open results in the protections panel header persisting "google.com" in the text, while the rest of the panel reacts to content blocking events and updates itself.

It's not really invisible without popup autohide, either:

  1. open e.g. mozilla.org
  2. open devtools console
  3. set up a timed redirect, e.g. setTimeout(() => location.href = 'https://badssl.com/', 5000)
  4. while the timer runs, open the protections panel

The panel stays open. The identity popup closes if you do the same.

Per discussion on matrix, ni for Nihanth to leave some details on how we update the protections panel based on content blocking events etc.

Flags: needinfo?(nhnt11)
Whiteboard: [fxperf] → [fxperf:p2]

The Protections Panel UI has several parts that reflect information about the site in the current tab, and these need to be updated when the site or tab changes. I'll go through them all:

  1. The host in the title
  • Updated in refreshProtectionsPopup[1] which is currently called just before the popup is shown [2]
  1. The ETP toggle switch state
  • Also updated in refreshProtectionsPopup[3]
  1. The category item section
  • Each category item reacts to the pref that controls it, e.g. [4]. The reaction is to set a class on the element that reflects whether the pref is telling us to block or allow trackers of that category.
  • Subviews are updated just before they are opened, e.g. [5]
  • The main panel controller reacts to content blocking events, which currently sets some classes/attributes [6] but this is changing in bug 1572377.
  • The category item order is updated in onpopupshowing [7] or immediately if the panel is already open [8]
  1. The Protections level label ("Standard"/"Strict"/"Custom") at the end of the settings button, as well as the "N blocked" label and its tooltip at the end of the Show Report button
  • We set these when the tracking protection icon in the URL bar identity box is hovered[9], due to needing an async call to get the "N blocked" tracker count. Not sure why the new level label is here though, it doesn't need to be async.

I think that should cover most of it. Let me know if there's anything that I missed or unclear.

[1] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/base/content/browser-siteProtections.js#1844
[2] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/base/content/browser-siteProtections.js#2168
[3] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/base/content/browser-siteProtections.js#1855
[4] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/base/content/browser-siteProtections.js#48-53
[5] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/base/content/browser-siteProtections.js#1468
[6] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/base/content/browser-siteProtections.js#1713,1738-1740,1745
[7] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/components/controlcenter/content/protectionsPanel.inc.xhtml#12
[8] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/base/content/browser-siteProtections.js#1748
[9] https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/base/content/browser-siteProtections.js#1586

Flags: needinfo?(nhnt11)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Depends on: 1572377

Gonna split the identity and protections popup work.

Summary: Lazify the identity and protection panel popups → Lazify the protection panel popup

Comment on attachment 9155259 [details]
Bug 1634032 - use a template to wrap the identity popup while it's not needed, r?johannh

Revision D78894 was moved to bug 1646780. Setting attachment 9155259 [details] to obsolete.

Attachment #9155259 - Attachment is obsolete: true
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/71ca1673171d
use a template to wrap the protections popup while it's not needed, r=nhnt11
Flags: needinfo?(gijskruitbosch+bugs)
See Also: → 1543537
Attachment #9160790 - Attachment is obsolete: true
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/add40bc29da9
use a template to wrap the protections popup while it's not needed, r=nhnt11

Backed out changeset add40bc29da9 (bug 1634032) for browser_controlCenter.js failures

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=DVykZ_AXSqGFBdAvIrTvLg.0&fromchange=add40bc29da9320bd10fa24bb18ae2109338ee8e&searchStr=m%28ss%29&tochange=42fb647fccc7b82c799800e783f617dac7f5d959

Backout link: https://hg.mozilla.org/integration/autoland/rev/42fb647fccc7b82c799800e783f617dac7f5d959

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=309352646&repo=autoland&lineNumber=1783

...
[task 2020-07-10T17:30:41.184Z] 17:30:41     INFO - TEST-PASS | browser/tools/mozscreenshots/controlCenter/browser_controlCenter.js | A valid bounding box was found - 
[task 2020-07-10T17:30:42.364Z] 17:30:42     INFO - _onConfigurationReady
[task 2020-07-10T17:30:42.364Z] 17:30:42     INFO - Combination 19/63: noLWT_trackingProtectionNoElements
[task 2020-07-10T17:30:42.364Z] 17:30:42     INFO - promising trackingProtectionNoElements
[task 2020-07-10T17:30:42.364Z] 17:30:42     INFO - calling trackingProtectionNoElements
[task 2020-07-10T17:30:42.380Z] 17:30:42     INFO - called trackingProtectionNoElements
[task 2020-07-10T17:30:42.442Z] 17:30:42     INFO - TEST-INFO | started process screentopng
[task 2020-07-10T17:30:42.861Z] 17:30:42     INFO - TEST-INFO | screentopng: exit 0
[task 2020-07-10T17:30:42.861Z] 17:30:42     INFO - Buffered messages logged at 17:30:01
[task 2020-07-10T17:30:42.861Z] 17:30:42     INFO - Entering test bound setup
[task 2020-07-10T17:30:42.861Z] 17:30:42     INFO - Buffered messages finished
[task 2020-07-10T17:30:42.862Z] 17:30:42     INFO - TEST-UNEXPECTED-FAIL | browser/tools/mozscreenshots/controlCenter/browser_controlCenter.js | Unexpected exception in [ noLWT, trackingProtectionNoElements ]: TypeError: can't access property "hidePopup", gProtectionsHandler._protectionsPopup is null - 
[task 2020-07-10T17:30:42.862Z] 17:30:42     INFO - Stack trace:
[task 2020-07-10T17:30:42.863Z] 17:30:42     INFO - chrome://mochikit/content/browser-test.js:test_ok:1299
[task 2020-07-10T17:30:42.865Z] 17:30:42     INFO - resource://mozscreenshots/TestRunner.jsm:_performCombo:433
[task 2020-07-10T17:30:42.865Z] 17:30:42     INFO - 	TypeError: can't access property "hidePopup", gProtectionsHandler._protectionsPopup is null
[task 2020-07-10T17:30:42.866Z] 17:30:42     INFO - 	openProtectionsPopup@resource://mozscreenshots/configurations/ControlCenter.jsm:321:3
[task 2020-07-10T17:30:42.866Z] 17:30:42     INFO - applyConfig@resource://mozscreenshots/configurations/ControlCenter.jsm:248:15
[task 2020-07-10T17:30:42.866Z] 17:30:42     INFO - 
[task 2020-07-10T17:30:42.867Z] 17:30:42     INFO - Combination 20/63: noLWT_trackingProtectionEnabled
[task 2020-07-10T17:30:42.867Z] 17:30:42     INFO - promising trackingProtectionEnabled
[task 2020-07-10T17:30:42.867Z] 17:30:42     INFO - calling trackingProtectionEnabled
[task 2020-07-10T17:30:42.867Z] 17:30:42     INFO - called trackingProtectionEnabled
[task 2020-07-10T17:30:42.867Z] 17:30:42     INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-07-10T17:30:42.868Z] 17:30:42     INFO - TEST-UNEXPECTED-FAIL | browser/tools/mozscreenshots/controlCenter/browser_controlCenter.js | Unexpected exception in [ noLWT, trackingProtectionEnabled ]: TypeError: can't access property "hidePopup", gProtectionsHandler._protectionsPopup is null - 
[task 2020-07-10T17:30:42.868Z] 17:30:42     INFO - Stack trace:
[task 2020-07-10T17:30:42.869Z] 17:30:42     INFO - chrome://mochikit/content/browser-test.js:test_ok:1299
[task 2020-07-10T17:30:42.869Z] 17:30:42     INFO - resource://mozscreenshots/TestRunner.jsm:_performCombo:433
[task 2020-07-10T17:30:42.869Z] 17:30:42     INFO - 	TypeError: can't access property "hidePopup", gProtectionsHandler._protectionsPopup is null
[task 2020-07-10T17:30:42.870Z] 17:30:42     INFO - 	openProtectionsPopup@resource://mozscreenshots/configurations/ControlCenter.jsm:321:3
[task 2020-07-10T17:30:42.870Z] 17:30:42     INFO - applyConfig@resource://mozscreenshots/configurations/ControlCenter.jsm:259:15
[task 2020-07-10T17:30:42.871Z] 17:30:42     INFO - 
[task 2020-07-10T17:30:42.871Z] 17:30:42     INFO - Combination 21/63: noLWT_trackingProtectionDisabled
[task 2020-07-10T17:30:42.872Z] 17:30:42     INFO - promising trackingProtectionDisabled
[task 2020-07-10T17:30:42.877Z] 17:30:42     INFO - calling trackingProtectionDisabled
[task 2020-07-10T17:30:42.878Z] 17:30:42     INFO - called trackingProtectionDisabled
[task 2020-07-10T17:30:42.879Z] 17:30:42     INFO - Console message: [JavaScript Warning: "The resource at “http://tracking.example.com/” was blocked because content blocking is enabled." {file: "http://tracking.example.org/browser/browser/tools/mozscreenshots/mozscreenshots/extension/mozscreenshots/browser/resources/lib/controlCenter/tracking.html" line: 0}]
[task 2020-07-10T17:30:42.882Z] 17:30:42     INFO - Console message: [JavaScript Warning: "The resource at “http://tracking.example.com/” was blocked because content blocking is enabled." {file: "http://tracking.example.org/browser/browser/tools/mozscreenshots/mozscreenshots/extension/mozscreenshots/browser/resources/lib/controlCenter/tracking.html" line: 0}]
[task 2020-07-10T17:30:42.918Z] 17:30:42     INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-07-10T17:30:42.920Z] 17:30:42     INFO - TEST-UNEXPECTED-FAIL | browser/tools/mozscreenshots/controlCenter/browser_controlCenter.js | Unexpected exception in [ noLWT, trackingProtectionDisabled ]: TypeError: can't access property "hidePopup", gProtectionsHandler._protectionsPopup is null - 
[task 2020-07-10T17:30:42.920Z] 17:30:42     INFO - Stack trace:
[task 2020-07-10T17:30:42.920Z] 17:30:42     INFO - chrome://mochikit/content/browser-test.js:test_ok:1299
[task 2020-07-10T17:30:42.920Z] 17:30:42     INFO - resource://mozscreenshots/TestRunner.jsm:_performCombo:433
[task 2020-07-10T17:30:42.920Z] 17:30:42     INFO - 	TypeError: can't access property "hidePopup", gProtectionsHandler._protectionsPopup is null
[task 2020-07-10T17:30:42.922Z] 17:30:42     INFO - 	openProtectionsPopup@resource://mozscreenshots/configurations/ControlCenter.jsm:321:3
[task 2020-07-10T17:30:42.922Z] 17:30:42     INFO - applyConfig@resource://mozscreenshots/configurations/ControlCenter.jsm:283:15
[task 2020-07-10T17:30:42.922Z] 17:30:42     INFO - 
[task 2020-07-10T17:30:42.923Z] 17:30:42     INFO - Combination 22/63: compactLight_about
[task 2020-07-10T17:30:42.923Z] 17:30:42     INFO - promising compactLight
[task 2020-07-10T17:30:42.924Z] 17:30:42     INFO - calling compactLight
[task 2020-07-10T17:30:42.924Z] 17:30:42     INFO - called compactLight
[task 2020-07-10T17:30:43.682Z] 17:30:43     INFO - promising about
[task 2020-07-10T17:30:43.682Z] 17:30:43     INFO - calling about
[task 2020-07-10T17:30:43.682Z] 17:30:43     INFO - called about
[task 2020-07-10T17:30:43.779Z] 17:30:43     INFO - Console message: [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”)." {file: "about:rights" line: 0}]
[task 2020-07-10T17:30:43.780Z] 17:30:43     INFO - Console message: [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”)." {file: "about:rights" line: 0}]
[task 2020-07-10T17:30:43.795Z] 17:30:43     INFO - Console message: [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”)." {file: "chrome://global/content/aboutRights.js" line: 32}]
[task 2020-07-10T17:30:44.494Z] 17:30:44     INFO - fulfilled all applyConfig so setting lastCombo.
[task 2020-07-10T17:30:44.494Z] 17:30:44     INFO - Configured UI for [ compactLight, about ] successfully
[task 2020-07-10T17:30:44.494Z] 17:30:44     INFO - TEST-PASS | browser/tools/mozscreenshots/controlCenter/browser_controlCenter.js | A valid bounding box was found - 
...
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(gijskruitbosch+bugs)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6b03a900e2d7
use a template to wrap the protections popup while it's not needed, r=nhnt11
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 80
Regressions: 1652614
Depends on: 1653398
No longer depends on: 1653398
Regressions: 1653398
Blocks: 1692510
You need to log in before you can comment on or make changes to this bug.