Refactor TaskManager's favicon retrieval approach

Instead of listening to unreliable WebContentsObserver events, we now
make use of the FaviconDriverObserver.

R=nick@chromium.org
BUG=530486,528924

Review URL: https://codereview.chromium.org/1338023002

Cr-Commit-Position: refs/heads/master@{#348773}

chrome/browser/task_management/providers/web_contents/renderer_task.cc

@@ -78,9 +78,17 @@ RendererTask::RendererTask(const base::string16& title,
   // All renderer tasks are capable of reporting network usage, so the default
   // invalid value of -1 doesn't apply here.
   OnNetworkBytesRead(0);
+
+  // Tag the web_contents with a |ContentFaviconDriver| (if needed) so that
+  // we can use it to observe favicons changes.
+  favicon::CreateContentFaviconDriverForWebContents(web_contents);
+  favicon::ContentFaviconDriver::FromWebContents(web_contents)->AddObserver(
+      this);
 }
 
 RendererTask::~RendererTask() {
+  favicon::ContentFaviconDriver::FromWebContents(web_contents())->
+      RemoveObserver(this);
 }
 
 void RendererTask::Activate() {
@@ -138,6 +146,14 @@ blink::WebCache::ResourceTypeStats RendererTask::GetWebCacheStats() const {
   return webcache_stats_;
 }
 
+void RendererTask::OnFaviconAvailable(const gfx::Image& image) {
+}
+
+void RendererTask::OnFaviconUpdated(favicon::FaviconDriver* favicon_driver,
+                                    bool icon_url_changed) {
+  UpdateFavicon();
+}
+
 // static
 base::string16 RendererTask::GetTitleFromWebContents(
     content::WebContents* web_contents) {

chrome/browser/task_management/providers/web_contents/renderer_task.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_TASK_MANAGEMENT_PROVIDERS_WEB_CONTENTS_RENDERER_TASK_H_
 
 #include "chrome/browser/task_management/providers/task.h"
+#include "components/favicon/core/favicon_driver_observer.h"
 #include "content/public/browser/navigation_entry.h"
 
 class ProcessResourceUsage;
@@ -19,7 +20,8 @@ namespace task_management {
 
 // Defines an abstract base class for various types of renderer process tasks
 // such as background contents, tab contents, ... etc.
-class RendererTask : public Task {
+class RendererTask : public Task,
+                     public favicon::FaviconDriverObserver {
  public:
   RendererTask(const base::string16& title,
                const gfx::ImageSkia* icon,
@@ -34,8 +36,8 @@ class RendererTask : public Task {
   virtual void UpdateTitle() = 0;
 
   // An abstract method that will be called when the event
-  // WebContentsObserver::DocumentOnLoadCompletedInMainFrame() occurs, so that
-  // concrete tasks can update their favicons.
+  // FaviconDriverObserver::OnFaviconUpdated() occurs, so that concrete tasks
+  // can update their favicons.
   virtual void UpdateFavicon() = 0;
 
   // task_management::Task:
@@ -50,6 +52,11 @@ class RendererTask : public Task {
   bool ReportsWebCacheStats() const override;
   blink::WebCache::ResourceTypeStats GetWebCacheStats() const override;
 
+  // favicon::FaviconDriverObserver:
+  void OnFaviconAvailable(const gfx::Image& image) override;
+  void OnFaviconUpdated(favicon::FaviconDriver* favicon_driver,
+                        bool icon_url_changed) override;
+
  protected:
   // Returns the title of the given |web_contents|.
   static base::string16 GetTitleFromWebContents(

chrome/browser/task_management/providers/web_contents/web_contents_task_provider.cc

@@ -54,10 +54,7 @@ class WebContentsEntry : public content::WebContentsObserver {
   void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) override;
-  void DocumentOnLoadCompletedInMainFrame() override;
   void TitleWasSet(content::NavigationEntry* entry, bool explicit_set) override;
-  void DidUpdateFaviconURL(
-      const std::vector<content::FaviconURL>& candidates) override;
 
  private:
   // Defines a callback for WebContents::ForEachFrame() to create a
@@ -174,20 +171,6 @@ void WebContentsEntry::DidNavigateMainFrame(
   itr->second->UpdateTitle();
 }
 
-void WebContentsEntry::DocumentOnLoadCompletedInMainFrame() {
-  // Note: Requesting the task to update its favicon, in any earlier occurring
-  // event than this one, may not work properly. We also need to listen to
-  // WebContentsObserver::DidUpdateFaviconURL().
-  auto itr = tasks_by_frames_.find(web_contents()->GetMainFrame());
-  if (itr == tasks_by_frames_.end()) {
-    // TODO(afakhry): Validate whether this actually happens in practice.
-    NOTREACHED();
-    return;
-  }
-
-  itr->second->UpdateFavicon();
-}
-
 void WebContentsEntry::TitleWasSet(content::NavigationEntry* entry,
                                    bool explicit_set) {
   auto itr = tasks_by_frames_.find(web_contents()->GetMainFrame());
@@ -200,18 +183,6 @@ void WebContentsEntry::TitleWasSet(content::NavigationEntry* entry,
   itr->second->UpdateTitle();
 }
 
-void WebContentsEntry::DidUpdateFaviconURL(
-    const std::vector<content::FaviconURL>& candidates) {
-  auto itr = tasks_by_frames_.find(web_contents()->GetMainFrame());
-  if (itr == tasks_by_frames_.end()) {
-    // TODO(afakhry): Validate whether this actually happens in practice.
-    NOTREACHED();
-    return;
-  }
-
-  itr->second->UpdateFavicon();
-}
-
 void WebContentsEntry::CreateTaskForFrame(RenderFrameHost* render_frame_host) {
   DCHECK_EQ(0U, tasks_by_frames_.count(render_frame_host));