Closed Bug 1274881 Opened 8 years ago Closed 8 years ago

Get rid of imgLoader::GetInstance

Categories

(Core :: Graphics: ImageLib, defect)

Product:
Core
Component:
Graphics: ImageLib
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla49
Tracking Flags:
Tracking Status
firefox49 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file, 1 obsolete file)

patch
4.02 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review 
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.
Attached 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)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter 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: 8 years ago

          status-firefox49:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49

          You need to log in
          before you can comment on or make changes to this bug.