1572377 - Move updating the protections panel out of gProtectionsHandler.onContentBlockingEvent

Status: RESOLVED FIXED

(Firefox :: Protections UI, enhancement, P2)

Description

[Johann Hofmann [:johannh]]

We do a bunch of things (example) in gProtectionsHandler.onContentBlockingEvent and gProtectionsHandler.onLocationChange that will update the protections panel to fit the new content blocking state.

Doing this while the panel isn't open is not necessary, AFAIK. We should do this lazily based on the set attribute when the panel is opened or when the panel is already open.

Comment 1

[Johann Hofmann [:johannh]]

Marinella, would you be interested in working on this?

The idea is that the above linked lines should be copied to refreshProtectionsPopup, and that the existing lines in onContentBlockingEvent should only be run if this condition is true (that is, the popup is already showing).

Comment 2

[Marinella]

Yes I'll take care of this.

Comment 3

[Marinella]

Attachment: Bug 1572377 - Updatng the protection panel only when the panel is opened or already opened. r=johannh

Comment 4

[Marinella]

Johann I've submitted for revision only a part of the code, that one inside onContentBlockingEvent
To put the code in the refreshProtectionsPopup I have to reassign anyDetected - anyBlocking and hasException, but I'm not sure of which part of code I must insert in the function.

Comment 5

[Johann Hofmann [:johannh]]

Hey Marinella, I'm a bit unsure about your question. You're right that we probably need to make anyDetected, anyBlocking and hasException available to other functions. We could do that by using them as member variables of gProtectionsHandler, i.e assigning this.anyDetected =.

Does that answer your question? :)

Comment 6

[Marinella]

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

Hey Marinella, I'm a bit unsure about your question. You're right that we probably need to make anyDetected, anyBlocking and hasException available to other functions. We could do that by using them as member variables of gProtectionsHandler, i.e assigning this.anyDetected =.

Does that answer your question? :)

Hi Johann, I wrote you in chat but maybe it's the wrong place.
I'm sorry, but I'm a beginner in JavaScript so I'm not sure how to manage this variables.
In onContentBlockingEvent they are:

this.anyDetected = false; let anyBlocking = false; let hasException = ContentBlockingAllowList.includes( gBrowser.selectedBrowser );

So, must I erase them from onContentBlockingEvent and paste them in the var gProtectionHandler to change the closure?

Comment 7

[Marinella]

Attachment: Bug 1572377 - changes to the first commit Move updating the protections panel out of gProtectionsHandler.onContentBlockingEvent. r=johannh

Comment 8

[Marinella]

Attachment: Bug 1572377 - made some variable change to the last commit. r=johannh

Depends on D67495

Comment 9

[Marinella]

Attachment: Bug 1572377 - Updating the protection panel only when the panel is opened or already opened. r=johannh

Bug 1572377 - made some variable change to the last commit. r=johannh

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a38011a0bed3
Updating the protection panel only when the panel is opened or already opened. r=johannh

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset a38011a0bed3 (bug 1572377) for protectionsUI mochitest failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=594b7e88632d370ed612fdb74ac94a03b4fc0710&searchStr=windows%2C10%2Cx64%2Cquantumrender%2Cdebug%2Cmochitests%2Ctest-windows10-64-qr%2Fdebug-mochitest-browser-chrome-e10s-1%2Cm%28bc1%29&tochange=3b1e479e5db5a62ae91084cb25fbc0dd0cfbbfce&selectedTaskRun=L23UjRa5SFGTtRtLcQH2VA-0

Backout link: https://hg.mozilla.org/integration/autoland/rev/3b1e479e5db5a62ae91084cb25fbc0dd0cfbbfce

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

[task 2020-04-29T15:55:28.343Z] 15:55:28     INFO - TEST-START | browser/base/content/test/protectionsUI/browser_protectionsUI.js
[task 2020-04-29T15:55:28.429Z] 15:55:28     INFO - GECKO(6620) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to c:\users\task_1588175233\appdata\local\temp\tmp7tw9cy.mozrunner\runtests_leaks_tab_pid2468.log
[task 2020-04-29T15:55:28.434Z] 15:55:28     INFO - GECKO(6620) | [2468, Main Thread] WARNING: XPCOM_MEM_BLOAT_LOG is set, disabling native allocations.: file /builds/worker/checkouts/gecko/tools/profiler/core/platform.cpp, line 224
[task 2020-04-29T15:55:28.452Z] 15:55:28     INFO - GECKO(6620) | [Parent 10508, Main Thread] WARNING: NS_ENSURE_TRUE(mPresShell) failed: file /builds/worker/checkouts/gecko/layout/generic/nsFrameSelection.cpp, line 1512
[task 2020-04-29T15:55:28.621Z] 15:55:28     INFO - GECKO(6620) | [Child 2468: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 00000211665B0800 == 1 [pid = 2468] [id = {8a26d7b5-f1d5-4379-8abd-561b69ae7205}]
[task 2020-04-29T15:55:28.621Z] 15:55:28     INFO - GECKO(6620) | [Child 2468: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 1 (0000021166E65090) [pid = 2468] [serial = 1] [outer = 0000000000000000]
[task 2020-04-29T15:55:28.622Z] 15:55:28     INFO - GECKO(6620) | [Child 2468: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 2 (0000021166E74800) [pid = 2468] [serial = 2] [outer = 0000021166E65090]
[task 2020-04-29T15:55:28.782Z] 15:55:28     INFO - GECKO(6620) | [Child 2468: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 3 (00000211665B0000) [pid = 2468] [serial = 3] [outer = 0000021166E65090]
[task 2020-04-29T15:55:28.902Z] 15:55:28     INFO - GECKO(6620) | [Child 2468: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 000002116857B400 == 2 [pid = 2468] [id = {ff6185e3-017d-401e-a978-139766b6d6dc}]
[task 2020-04-29T15:55:28.902Z] 15:55:28     INFO - GECKO(6620) | [Child 2468: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 4 (0000021166E68520) [pid = 2468] [serial = 4] [outer = 0000000000000000]
[task 2020-04-29T15:55:28.902Z] 15:55:28     INFO - GECKO(6620) | [Parent 10508, Main Thread] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/dom/security/FramingChecker.cpp, line 201
[task 2020-04-29T15:55:28.920Z] 15:55:28     INFO - GECKO(6620) | [Child 2468: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 5 (0000021166E77000) [pid = 2468] [serial = 5] [outer = 0000021166E68520]
[task 2020-04-29T15:55:29.550Z] 15:55:29     INFO - GECKO(6620) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to c:\users\task_1588175233\appdata\local\temp\tmp7tw9cy.mozrunner\runtests_leaks_tab_pid11172.log
[task 2020-04-29T15:55:29.550Z] 15:55:29     INFO - GECKO(6620) | [11172, Main Thread] WARNING: XPCOM_MEM_BLOAT_LOG is set, disabling native allocations.: file /builds/worker/checkouts/gecko/tools/profiler/core/platform.cpp, line 224
[task 2020-04-29T15:55:31.015Z] 15:55:31     INFO - GECKO(6620) | [Parent 10508: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 12 (00000241668A6BC0) [pid = 10508] [serial = 1] [outer = 0000000000000000] [url = chrome://gfxsanity/content/sanityparent.html]
[task 2020-04-29T15:55:31.624Z] 15:55:31     INFO - GECKO(6620) | [Child 7228: Main Thread]: I/DocShellAndDOMWindowLeak --DOCSHELL 0000015AEE0A7000 == 0 [pid = 7228] [id = {86556871-a61d-4e41-8554-2fe20d207401}] [url = about:blank]
[task 2020-04-29T15:55:32.619Z] 15:55:32     INFO - GECKO(6620) | [Child 4736: Main Thread]: I/DocShellAndDOMWindowLeak --DOCSHELL 000001C1CB8B8000 == 1 [pid = 4736] [id = {0dd691e7-772f-48be-840c-157d04ec1f46}] [url = chrome://gfxsanity/content/sanitytest.html]
[task 2020-04-29T15:55:34.242Z] 15:55:34     INFO - TEST-INFO | started process screenshot
[task 2020-04-29T15:55:34.308Z] 15:55:34     INFO - TEST-INFO | screenshot: exit 0
[task 2020-04-29T15:55:34.308Z] 15:55:34     INFO - Buffered messages logged at 15:55:28
[task 2020-04-29T15:55:34.308Z] 15:55:34     INFO - Entering test bound setup
[task 2020-04-29T15:55:34.308Z] 15:55:34     INFO - Leaving test bound setup
[task 2020-04-29T15:55:34.308Z] 15:55:34     INFO - Entering test bound testToggleSwitch
[task 2020-04-29T15:55:34.309Z] 15:55:34     INFO - Console message: [JavaScript Warning: "The resource at “http://trackertest.org/” was blocked because content blocking is enabled." {file: "http://tracking.example.org/browser/browser/base/content/test/protectionsUI/trackingPage.html" line: 0}]
[task 2020-04-29T15:55:34.309Z] 15:55:34     INFO - Buffered messages finished
[task 2020-04-29T15:55:34.309Z] 15:55:34     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/protectionsUI/browser_protectionsUI.js | Uncaught exception - undefined - timed out after 50 tries.
[task 2020-04-29T15:55:34.309Z] 15:55:34     INFO - Leaving test bound testToggleSwitch
...

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11c9a89c4758
Updating the protection panel only when the panel is opened or already opened. r=Gijs

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/11c9a89c4758