1362224 - Speed up setting *Action icon images during tab switch

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect)

Description

[Kris Maglione [:kmag]]

Selecting the icon and generating the CSS for these can be pretty expensive. We should try to cache both rather than recalculate them for every navigation and/or tab switch.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1362224: Cache per-tab icon data for browserAction/pageAction.

Review commit: https://reviewboard.mozilla.org/r/136632/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136632/

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1362224: requestAnimationFrame before changing browserAction/pageAction buttons.

Review commit: https://reviewboard.mozilla.org/r/136634/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136634/

Comment 3

[Kris Maglione [:kmag]]

Attachment: Bug 1362224: Speed up permission checks a bit.

Review commit: https://reviewboard.mozilla.org/r/136636/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136636/

Comment 4

[Kris Maglione [:kmag]]

Attachment: Bug 1362224: Cached normalized object for simple icon URLs.

Review commit: https://reviewboard.mozilla.org/r/136638/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136638/

Comment 5

[Kris Maglione [:kmag]]

Attachment: Bug 1362224: Cache tab incognito state.

Review commit: https://reviewboard.mozilla.org/r/136644/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136644/

Comment 6

[Andrew Swan [:aswan]]

Comment on attachment 8864972 [details]
Bug 1362224: Cache per-tab icon data for browserAction/pageAction.

https://reviewboard.mozilla.org/r/136632/#review139738

Comment 7

[Andrew Swan [:aswan]]

Comment on attachment 8864974 [details]
Bug 1362224: Speed up permission checks a bit.

https://reviewboard.mozilla.org/r/136636/#review139740

Comment 8

[Andrew Swan [:aswan]]

Comment on attachment 8864978 [details]
Bug 1362224: Cached normalized object for simple icon URLs.

https://reviewboard.mozilla.org/r/136638/#review139744

::: toolkit/components/extensions/ExtensionUtils.jsm:326
(Diff revision 1)
>  }
>  
>  // Manages icon details for toolbar buttons in the |pageAction| and
>  // |browserAction| APIs.
>  let IconDetails = {
> +  iconCache: new DefaultWeakMap(() => new Map()),

add a comment documenting the structure please.

Comment 9

[Andrew Swan [:aswan]]

Comment on attachment 8864982 [details]
Bug 1362224: Cache tab incognito state.

https://reviewboard.mozilla.org/r/136644/#review139748

Comment 10

[Andrew Swan [:aswan]]

Comment on attachment 8864973 [details]
Bug 1362224: requestAnimationFrame before changing browserAction/pageAction buttons.

https://reviewboard.mozilla.org/r/136634/#review139752

r=me with the `promiseAnimationFrame()` changes we discussed on IRC

Comment 11

[Kris Maglione [:kmag]]

Comment on attachment 8864973 [details]
Bug 1362224: requestAnimationFrame before changing browserAction/pageAction buttons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136634/diff/1-2/

Comment 12

[Kris Maglione [:kmag]]

Comment on attachment 8864974 [details]
Bug 1362224: Speed up permission checks a bit.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136636/diff/1-2/

Comment 13

[Kris Maglione [:kmag]]

Comment on attachment 8864978 [details]
Bug 1362224: Cached normalized object for simple icon URLs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136638/diff/1-2/

Comment 14

[Kris Maglione [:kmag]]

Comment on attachment 8864982 [details]
Bug 1362224: Cache tab incognito state.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136644/diff/1-2/

Comment 15

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80e82d441dca0df90cd4c994da887ee19459d8f3
Bug 1362224: Cache per-tab icon data for browserAction/pageAction. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa1764039854ed505e3e6690b07598653a69ccf3
Bug 1362224: requestAnimationFrame before changing browserAction/pageAction buttons. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/72a3bc67fccb4179fc87aec626bd5cf8051e3bd0
Bug 1362224: Speed up permission checks a bit. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4013bac9f353468739db52fcd7ea724178d6232
Bug 1362224: Cached normalized object for simple icon URLs. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/79511b594380b3d30a89db9084182f5cb5b1f588
Bug 1362224: Cache tab incognito state. r=aswan

Comment 16

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/80e82d441dca
https://hg.mozilla.org/mozilla-central/rev/aa1764039854
https://hg.mozilla.org/mozilla-central/rev/72a3bc67fccb
https://hg.mozilla.org/mozilla-central/rev/d4013bac9f35
https://hg.mozilla.org/mozilla-central/rev/79511b594380

Comment 17

[Kris Maglione [:kmag]]

Comment on attachment 8864972 [details]
Bug 1362224: Cache per-tab icon data for browserAction/pageAction.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: This patch improves tab switch performance when WebExtension pageAction or browserAction buttons are active. Since Screenshots uses a browserAction button, this should help offset the performance impact of enabling it in 54.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: N/A
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low-risk
[Why is the change risky/not risky?]: The changes are fairly simple, and should only risk affecting add-ons with per-tab pageAction and browserAction icons, which are relatively rare.
[String changes made/needed]: None

Comment 18

[Kris Maglione [:kmag]]

Comment on attachment 8864974 [details]
Bug 1362224: Speed up permission checks a bit.

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: This should speed up a number of operations in WebExtension code, and should help mitigate the performance impact of enabling Screenshots in beta.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: N/A
[Needs manual test from QE? If yes, steps to reproduce]: N/A
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This is a very simple change which replaces a fairly expensive operation with an cheaper equivalent.
[String changes made/needed]: None.

Comment 19

[Gerry Chang [:gchang]]

Screenshots will go live in Fx55 so we don't have to uplift these patches in 54. Beta54-. Mark 54 won't fix.