Get rid of imgLoader::GetInstance

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

imgLoader::GetInstance creates a separate imgLoader instance that is used only by two pieces of code. Either these bits of code should be using imgLoader::Singleton like everyone else, or we should document why they need to be using a separate imgLoader instance.

imgLoader::GetInstance was added in bug 801061. It sounds like there might have been some out of band discussion as to why it was introduced, but since that discussion didn't happen in the bug and nobody's bothered to document the method, who knows what the rational was.
Posted patch patch (obsolete) — Splinter Review
Assignee: nobody → jwatt
Attachment #8755245 - Flags: review?(jmuizelaar)
(In reply to Jonathan Watt [:jwatt] from comment #0)
> imgLoader::GetInstance was added in bug 801061.

Along with the GetInstance call in nsContextMenuInfo.cpp.

FWIW the call in OSXNotificationCenter.mm came from https://hg.mozilla.org/mozilla-central/rev/e8f0814e85c5 (bug 852648). Again, no explanation as to why we're not using imgLoader::Singleton.
Attachment #8755245 - Flags: review?(jmuizelaar) → review?(tnikkel)
Posted patch patchSplinter Review
Actually OSXNotificationCenter::ShowAlertWithIconData is implementing the method of the same name on interface nsIAlertsIconData, so those icons could be coming from a website. While we're here we should fix this code to use a/the imgLoader that has had RespectPrivacyNotifications() called on it.
Attachment #8755245 - Attachment is obsolete: true
Attachment #8755245 - Flags: review?(tnikkel)
Attachment #8755633 - Flags: review?(tnikkel)
Attachment #8755633 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/e866748db4e3
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.