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

chrome/browser/favicon/favicon_tab_helper.h

Index: chrome/browser/favicon/favicon_tab_helper.h
diff --git a/chrome/browser/favicon/favicon_tab_helper.h b/chrome/browser/favicon/favicon_tab_helper.h
index a5ef1ae2fd1f198a738976d08623f37a9145372a..836f8c5f66854bbda30b93275fdad8ea019daf53 100644
--- a/chrome/browser/favicon/favicon_tab_helper.h
+++ b/chrome/browser/favicon/favicon_tab_helper.h
@@ -51,6 +51,16 @@ class FaviconTabHelper : public content::WebContentsObserver,
   // 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) {
@@ -94,6 +104,8 @@ class FaviconTabHelper : public content::WebContentsObserver,
   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