[InfoBar] Add InfoBarDelegate::GetIconID()

chrome/browser/infobars/infobar_delegate.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_INFOBARS_INFOBAR_DELEGATE_H_
 #define CHROME_BROWSER_INFOBARS_INFOBAR_DELEGATE_H_
 
 #include "base/basictypes.h"
 #include "base/string16.h"
 #include "chrome/browser/infobars/infobar_service.h"
@@ skipping 60 matching lines @@
   // Returns true if the InfoBar should be closed automatically after the page
   // is navigated. By default this returns true if the navigation is to a new
   // page (not including reloads).  Subclasses wishing to change this behavior
   // can override either this function or ShouldExpireInternal(), depending on
   // what level of control they need.
   virtual bool ShouldExpire(const content::LoadCommittedDetails& details) const;
 
   // Called when the user clicks on the close button to dismiss the infobar.
   virtual void InfoBarDismissed();
 
-  // Return the icon to be shown for this InfoBar. If the returned Image is
-  // NULL, no icon is shown.
-  virtual gfx::Image* GetIcon() const;
+  // Return the resource ID of the icon to be shown for this InfoBar.  If the
+  // value is -1, no icon is shown.
+  virtual int GetIconID() const;
 
   // Returns the type of the infobar.  The type determines the appearance (such
   // as background color) of the infobar.
   virtual Type GetInfoBarType() const;
 
   // Type-checking downcast routines:
   virtual AutoLoginInfoBarDelegate* AsAutoLoginInfoBarDelegate();
   virtual ConfirmInfoBarDelegate* AsConfirmInfoBarDelegate();
   virtual ExtensionInfoBarDelegate* AsExtensionInfoBarDelegate();
   virtual InsecureContentInfoBarDelegate* AsInsecureContentInfoBarDelegate();
   virtual MediaStreamInfoBarDelegate* AsMediaStreamInfoBarDelegate();
   virtual RegisterProtocolHandlerInfoBarDelegate*
       AsRegisterProtocolHandlerInfoBarDelegate();
   virtual ScreenCaptureInfoBarDelegate* AsScreenCaptureInfoBarDelegate();
   virtual ThemeInstalledInfoBarDelegate* AsThemePreviewInfobarDelegate();
   virtual ThreeDAPIInfoBarDelegate* AsThreeDAPIInfoBarDelegate();
   virtual TranslateInfoBarDelegate* AsTranslateInfoBarDelegate();
 
   content::WebContents* web_contents() {
     return owner_ ? owner_->web_contents() : NULL;
   }
 
+  // Return the icon to be shown for this InfoBar. If the returned Image is
+  // NULL, no icon is shown.

[sky, 2013/05/14 21:02:03]

nit: document ownership here.

Is there a reason we don't return a gfx::Image here? That way no worries about
ownership.

[gone, 2013/05/14 22:31:44]

I think it's because the ResourceBundle caches everything so that subsequent
loads of the same resource don't incur another loading penalty.  Added a comment
to that effect.

On 2013/05/14 21:02:03, sky wrote:

[Peter Kasting, 2013/05/15 00:18:52]

How does returning an Image instead of an Image* make much difference to that,
though?

Ans aren't Images supposed to be cheap to copy (because they refcount the
important underlying data)?

I'm a fan of not returning pointer types unless we have to; I didn't focus on it
because this wasn't a change from the old code, but perhaps it's worth looking
at since you're touching this anyway.

+  gfx::Image* GetIcon() const;
+
  protected:
   // If |contents| is non-NULL, its active entry's unique ID will be stored
   // using StoreActiveEntryUniqueID automatically.
   explicit InfoBarDelegate(InfoBarService* infobar_service);
 
   // Store the unique id for the active entry in our WebContents, to be used
   // later upon navigation to determine if this InfoBarDelegate should be
   // expired.
   void StoreActiveEntryUniqueID();
 
@@ skipping 12 matching lines @@
   // opened for. Used to help expire on navigations.
   int contents_unique_id_;
 
   // TODO(pkasting): Remove.
   InfoBarService* owner_;
 
   DISALLOW_COPY_AND_ASSIGN(InfoBarDelegate);
 };
 
 #endif  // CHROME_BROWSER_INFOBARS_INFOBAR_DELEGATE_H_