1634032 - Lazify the protection panel popup

Assignee

Description

•

4 years ago

+++ 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?

:Gijs (he/him)

Assignee

Comment 1

•

4 years ago

Ni for comment #0

Flags: needinfo?(jhofmann)

(inactive) Nihanth Subramanya [:nhnt11]

Comment 2

•

4 years ago

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).

Johann Hofmann [:johannh]

Comment 3

•

4 years ago

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)

(inactive) Nihanth Subramanya [:nhnt11]

Comment 4

•

4 years ago

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.

:Gijs (he/him)

Assignee

Comment 5

•

4 years ago

(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.)

:Gijs (he/him)

Assignee

Comment 6

•

4 years ago

(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:

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

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

:Gijs (he/him)

Assignee

Comment 7

•

4 years ago

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)

Mike Conley (:mconley) (:⚙️)

Updated

•

4 years ago

Whiteboard: [fxperf] → [fxperf:p2]

(inactive) Nihanth Subramanya [:nhnt11]

Comment 8

•

4 years ago

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:

The host in the title

Updated in refreshProtectionsPopup[1] which is currently called just before the popup is shown [2]

The ETP toggle switch state

Also updated in refreshProtectionsPopup[3]

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]

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.

Flags: needinfo?(nhnt11)

:Gijs (he/him)

Assignee

Updated

•

4 years ago

Assignee: nobody → gijskruitbosch+bugs

Status: NEW → ASSIGNED

:Gijs (he/him)

Assignee

Updated

•

4 years ago

Depends on: 1572377

:Gijs (he/him)

Assignee

Comment 9

•

4 years ago

Attached file Bug 1634032 - use a template to wrap the identity popup while it's not needed, r?johannh (obsolete) — Details

:Gijs (he/him)

Assignee

Comment 10

•

4 years ago

Gonna split the identity and protections popup work.

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

Phabricator Automation

Comment 11

•

4 years ago

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

:Gijs (he/him)

Assignee

Comment 12

•

4 years ago

Attached file Bug 1634032 - use a template to wrap the protections popup while it's not needed, r?nhnt11 — Details

:Gijs (he/him)

Assignee

Comment 13

•

4 years ago

Attached file Bug 1634032 - only update per-category block information in the protections panel while it's open or when it's about to be shown, r?nhnt11 (obsolete) — Details

Depends on D81926

Pulsebot

Comment 14

•

4 years ago

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

Atila Butkovits

Comment 15

•

4 years ago

Backed out for for assertion failure on DOMJSProxyHandler.cpp.

Backout link: https://hg.mozilla.org/integration/autoland/rev/15e333214ed42030b96de301b8eadd323fd67573

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=YZG-pjXURHeLxJ3egxPFnw.0&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=6c8547a37a638627bfef90a36236edd74f5ffaf7

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

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

Flags: needinfo?(gijskruitbosch+bugs)

:Gijs (he/him)

Assignee

Updated

•

4 years ago

Flags: needinfo?(gijskruitbosch+bugs)

Updated

•

4 years ago

Attachment #9160790 - Attachment is obsolete: true

Pulsebot

Comment 16

•

4 years ago

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

Bogdan Tara[:bogdan_tara | bogdant]

Comment 17

•

4 years ago

•

Edited

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)

:Gijs (he/him)

Assignee

Comment 18

•

4 years ago

Bah, I'm having no luck at all this week.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c0f0fe3a80af6f6f77cc60a605f4f831a91ded6

Flags: needinfo?(gijskruitbosch+bugs)

Pulsebot

Comment 19

•

4 years ago

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

Andreea Pavel [:apavel]

Comment 20

•

4 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/6b03a900e2d7

Status: ASSIGNED → RESOLVED

Closed: 4 years ago

status-firefox80: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 80

Steven Englehardt [:englehardt]

Updated

•

4 years ago

Regressions: 1652614

Ryan VanderMeulen [:RyanVM]

Updated

•

4 years ago

status-firefox77: affected → wontfix

Matthew N. [:MattN]

Updated

•

4 years ago

Depends on: 1653398

:Gijs (he/him)

Assignee

Updated

•

4 years ago

No longer depends on: 1653398

Regressions: 1653398

Paul Zühlcke [:pbz]

Updated

•

3 years ago

Blocks: 1692510

Bug 1634032 - use a template to wrap the identity popup while it's not needed, r?johannh 4 years ago :Gijs (he/him) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1634032 - use a template to wrap the protections popup while it's not needed, r?nhnt11 4 years ago :Gijs (he/him) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1634032 - only update per-category block information in the protections panel while it's open or when it's about to be shown, r?nhnt11 4 years ago :Gijs (he/him) 47 bytes, text/x-phabricator-request		Details \| Review