Optimize icon setters some more

RESOLVED FIXED in Firefox 57

Status

()

Toolkit
WebExtensions: Frontend
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [qf])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

Comment hidden (empty)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Whiteboard: [qf]

Comment 3

4 months ago
mozreview-review
Comment on attachment 8898526 [details]
Bug 1391472: Part 1 - Remove integer property check from IconDetails.normalize.

https://reviewboard.mozilla.org/r/169888/#review175420
Attachment #8898526 - Flags: review?(mixedpuppy) → review+

Comment 4

4 months ago
mozreview-review
Comment on attachment 8898527 [details]
Bug 1391472: Part 2 - Cache normalized icon data for non-string values.

https://reviewboard.mozilla.org/r/169890/#review175430

::: toolkit/components/extensions/ExtensionParent.jsm:1241
(Diff revision 1)
>    //
>    // If no context is specified, instead of throwing an error, this
>    // function simply logs a warning message.
>    normalize(details, extension, context = null) {
> -    if (!details.imageData && typeof details.path === "string") {
> -      let icons = this.iconCache.get(extension);
> +    if (!details.imageData && details.path) {
> +      let key = details.path;

It might be nice to change "key" to "path" or something?  Reading the doc above made this confusing until I read the _normalize code.
Attachment #8898527 - Flags: review?(mixedpuppy) → review+
(Assignee)

Comment 5

4 months ago
mozreview-review-reply
Comment on attachment 8898527 [details]
Bug 1391472: Part 2 - Cache normalized icon data for non-string values.

https://reviewboard.mozilla.org/r/169890/#review175430

> It might be nice to change "key" to "path" or something?  Reading the doc above made this confusing until I read the _normalize code.

I decided to change it from `path` to `key` since sometimes it's a stringified path object rather than a simple path, and that could get confusing. I'll add some comments, though.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9eaa25ea1117887f5c6ca4da757909905e108907
Bug 1391472: Part 1 - Remove integer property check from IconDetails.normalize. r=mixedpuppy

https://hg.mozilla.org/integration/mozilla-inbound/rev/c8a7319bea7b3ad50d7490bac8c874e231a49c72
Bug 1391472: Part 2 - Cache normalized icon data for non-string values. r=mixedpuppy

Comment 7

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9eaa25ea1117
https://hg.mozilla.org/mozilla-central/rev/c8a7319bea7b
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.