Use high resolution icons where possible for streamlined hosted app icons.

chrome/browser/favicon/favicon_tab_helper.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_FAVICON_FAVICON_TAB_HELPER_H_
 #define CHROME_BROWSER_FAVICON_FAVICON_TAB_HELPER_H_
 
 #include <vector>
 
 #include "base/basictypes.h"
@@ skipping 33 matching lines @@
   // entries, which should rarely happen.
   gfx::Image GetFavicon() const;
 
   // Returns true if we have the favicon for the page.
   bool FaviconIsValid() const;
 
   // Returns whether the favicon should be displayed. If this returns false, no
   // space is provided for the favicon, and the favicon is never displayed.
   virtual bool ShouldDisplayFavicon();
 
+  // Returns the current tab's favicon urls. Returns NULL if the current tab's
+  // favicon urls have not been populated yet.
+  //
+  // Note that this is not guaranteed to be populated until the tab is fully
+  // loaded. Callers that want to handle favicon urls before the page has loaded
+  // should implement WebContentsObserver::DidUpdateFaviconURL.

[pkotwicz, 2013/12/02 01:00:57]

"Callers that want to handle favicon urls before the page has loaded should
implement WebContentsObserver::DidUpdateFaviconURL." is misleading. I think that
what you mean is that a caller may want to add an observer in case that the
favicon URLs have not been set yet. I think that goes without saying in the
context of the comment on line 54 and 55 and that you can just remove the
sentence.

[calamity, 2013/12/03 05:52:15]

Done.

+  const std::vector<content::FaviconURL>* GetFaviconURLs() {
+    return favicon_urls_.get();
+  }
+
   // Allows the client to determine if they want to fetch the Favicons as
   // they are discovered.
   void set_should_fetch_icons(bool fetch) {
     should_fetch_icons_ = fetch;
   }
 
   // content::WebContentsObserver override. Must be public, because also
   // called from PrerenderContents.
   virtual void DidUpdateFaviconURL(
       int32 page_id,
@@ skipping 23 matching lines @@
   virtual void NavigateToPendingEntry(
       const GURL& url,
       content::NavigationController::ReloadType reload_type) OVERRIDE;
   virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
 
   Profile* profile_;
   bool should_fetch_icons_;
 
+  scoped_ptr<std::vector<content::FaviconURL> > favicon_urls_;

[pkotwicz, 2013/12/02 01:00:57]

Can this just be a std::vector<content::FaviconURL> ?

[calamity, 2013/12/03 05:52:15]

This would require a separate bool to track whether the favicons have been
populated (or making a hard assumption that there is always a favicon URL). Is
that preferable?

[pkotwicz, 2013/12/03 19:06:49]

Given the current implementation, we should make the hard assumption.
(Note RenderViewImpl::SendUpdateFaviconURL())
WebContentsObserver::DidUpdateFaviconURL() will never be called with an empty
vector.

A browser test would be useful to ensure that we get a non empty vector in
WebContentsObserver::DidUpdateFaviconURL in when the page does not provide any
favicons.

+
   scoped_ptr<FaviconHandler> favicon_handler_;
 
   // Handles downloading touchicons. It is NULL if
   // browser_defaults::kEnableTouchIcon is false.
   scoped_ptr<FaviconHandler> touch_icon_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(FaviconTabHelper);
 };
 
 #endif  // CHROME_BROWSER_FAVICON_FAVICON_TAB_HELPER_H_