Remove favicon-handling logic from prerender.

The normal FaviconTabHelper logic kicks in fine on WebContentses not attached
to a Browser yet. Prerender's custom logic duplicates the image candidate logic
and, e.g., loses touch icons.

To preserve the Android behavior where favicon fetches are delayed until swap,
use the PrerenderResourceThrottle. Favicons on Android also seem to have been
broken by the original version of that change.  This fixes them.

Fix PrerenderBrowserTest.PrerenderFavicon to account for the race where the
favicon is ready before we start listening for the update.

BUG=341180,249179,171073
TEST=PrerenderBrowserTest.PrerenderFavicon

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@249761 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 25 insertions, 69 deletions

diff --git a/chrome/browser/favicon/favicon_tab_helper.cc b/chrome/browser/favicon/favicon_tab_helper.cc
index cbc55a0..245265d 100644
--- a/chrome/browser/favicon/favicon_tab_helper.cc
+++ b/chrome/browser/favicon/favicon_tab_helper.cc
@@ -37,8 +37,7 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(FaviconTabHelper);
 
 FaviconTabHelper::FaviconTabHelper(WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
-      profile_(Profile::FromBrowserContext(web_contents->GetBrowserContext())),
-      should_fetch_icons_(true) {
+      profile_(Profile::FromBrowserContext(web_contents->GetBrowserContext())) {
   favicon_handler_.reset(new FaviconHandler(profile_, this,
                                             FaviconHandler::FAVICON));
   if (chrome::kEnableTouchIcon)
@@ -50,9 +49,6 @@ FaviconTabHelper::~FaviconTabHelper() {
 }
 
 void FaviconTabHelper::FetchFavicon(const GURL& url) {
-  if (!should_fetch_icons_)
-    return;
-
   favicon_handler_->FetchFavicon(url);
   if (touch_icon_handler_.get())
     touch_icon_handler_->FetchFavicon(url);
diff --git a/chrome/browser/favicon/favicon_tab_helper.h b/chrome/browser/favicon/favicon_tab_helper.h
index 7758121..d2ff0e7 100644
--- a/chrome/browser/favicon/favicon_tab_helper.h
+++ b/chrome/browser/favicon/favicon_tab_helper.h
@@ -57,12 +57,6 @@ class FaviconTabHelper : public content::WebContentsObserver,
     return favicon_urls_;
   }
 
-  // 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(
@@ -98,7 +92,6 @@ class FaviconTabHelper : public content::WebContentsObserver,
       const content::FrameNavigateParams& params) OVERRIDE;
 
   Profile* profile_;
-  bool should_fetch_icons_;
 
   std::vector<content::FaviconURL> favicon_urls_;
 
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc
index 4e3f2e5..f33bd84 100644
--- a/chrome/browser/prerender/prerender_browsertest.cc
+++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -2992,19 +2992,23 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderLocalStorageWrite) {
 }
 
 // Checks that the favicon is properly loaded on prerender.
-IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
-                       DISABLED_PrerenderFavicon) {
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderFavicon) {
   scoped_ptr<TestPrerender> prerender =
       PrerenderTestURL("files/prerender/prerender_favicon.html",
                        FINAL_STATUS_USED,
                        1);
-  ASSERT_TRUE(prerender->contents());
-  content::WindowedNotificationObserver favicon_update_watcher(
-      chrome::NOTIFICATION_FAVICON_UPDATED,
-      content::Source<WebContents>(
-          prerender->contents()->prerender_contents()));
   NavigateToDestURL();
-  favicon_update_watcher.Wait();
+
+  if (!FaviconTabHelper::FromWebContents(
+          GetActiveWebContents())->FaviconIsValid()) {
+    // If the favicon has not been set yet, wait for it to be.
+    content::WindowedNotificationObserver favicon_update_watcher(
+        chrome::NOTIFICATION_FAVICON_UPDATED,
+        content::Source<WebContents>(GetActiveWebContents()));
+    favicon_update_watcher.Wait();
+  }
+  EXPECT_TRUE(FaviconTabHelper::FromWebContents(
+      GetActiveWebContents())->FaviconIsValid());
 }
 
 // Checks that when a prerendered page is swapped in to a referring page, the
diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc
index 827eda4..ad20810 100644
--- a/chrome/browser/prerender/prerender_contents.cc
+++ b/chrome/browser/prerender/prerender_contents.cc
@@ -11,7 +11,6 @@
 #include "base/bind.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/history/history_tab_helper.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/prerender/prerender_field_trial.h"
@@ -38,7 +37,6 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/browser/web_contents_view.h"
-#include "content/public/common/favicon_url.h"
 #include "content/public/common/frame_navigate_params.h"
 #include "ui/gfx/rect.h"
 
@@ -301,12 +299,6 @@ void PrerenderContents::StartPrerendering(
   prerender_contents_.reset(
       CreateWebContents(alias_session_storage_namespace.get()));
   TabHelpers::AttachTabHelpers(prerender_contents_.get());
-#if defined(OS_ANDROID)
-  // Delay icon fetching until the contents are getting swapped in
-  // to conserve network usage in mobile devices.
-  FaviconTabHelper::FromWebContents(
-      prerender_contents_.get())->set_should_fetch_icons(false);
-#endif  // defined(OS_ANDROID)
   content::WebContentsObserver::Observe(prerender_contents_.get());
 
   web_contents_delegate_.reset(new WebContentsDelegateImpl(this));
@@ -498,20 +490,6 @@ void PrerenderContents::NotifyPrerenderCreatedMatchCompleteReplacement(
                                                                replacement));
 }
 
-void PrerenderContents::DidUpdateFaviconURL(
-    int32 page_id,
-    const std::vector<content::FaviconURL>& urls) {
-  VLOG(1) << "PrerenderContents::OnUpdateFaviconURL" << icon_url_;
-  for (std::vector<content::FaviconURL>::const_iterator it = urls.begin();
-       it != urls.end(); ++it) {
-    if (it->icon_type == content::FaviconURL::FAVICON) {
-      icon_url_ = it->icon_url;
-      VLOG(1) << icon_url_;
-      return;
-    }
-  }
-}
-
 bool PrerenderContents::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   // The following messages we do want to consume.
diff --git a/chrome/browser/prerender/prerender_contents.h b/chrome/browser/prerender/prerender_contents.h
index f67a142..aa5c7aa 100644
--- a/chrome/browser/prerender/prerender_contents.h
+++ b/chrome/browser/prerender/prerender_contents.h
@@ -30,7 +30,6 @@ class ProcessMetrics;
 }
 
 namespace content {
-struct FaviconURL;
 class RenderViewHost;
 class SessionStorageNamespace;
 class WebContents;
@@ -162,7 +161,6 @@ class PrerenderContents : public content::NotificationObserver,
 
   base::string16 title() const { return title_; }
   int32 page_id() const { return page_id_; }
-  GURL icon_url() const { return icon_url_; }
   const GURL& prerender_url() const { return prerender_url_; }
   const content::Referrer& referrer() const { return referrer_; }
   bool has_stopped_loading() const { return has_stopped_loading_; }
@@ -221,8 +219,6 @@ class PrerenderContents : public content::NotificationObserver,
   virtual void DidGetRedirectForResourceRequest(
       content::RenderViewHost* render_view_host,
       const content::ResourceRedirectDetails& details) OVERRIDE;
-  virtual void DidUpdateFaviconURL(int32 page_id,
-      const std::vector<content::FaviconURL>& urls) OVERRIDE;
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
   virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE;
@@ -385,7 +381,6 @@ class PrerenderContents : public content::NotificationObserver,
   base::string16 title_;
   int32 page_id_;
   GURL url_;
-  GURL icon_url_;
   content::NotificationRegistrar notification_registrar_;
 
   // A vector of URLs that this prerendered page matches against.
diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
index 058c233..bee549a 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -23,7 +23,6 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/common/cancelable_request.h"
-#include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/net/chrome_cookie_notification_details.h"
 #include "chrome/browser/predictors/predictor_database.h"
@@ -61,7 +60,6 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/browser/web_contents_view.h"
-#include "content/public/common/favicon_url.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/common/constants.h"
 #include "net/url_request/url_request_context.h"
@@ -606,24 +604,6 @@ WebContents* PrerenderManager::SwapInternal(
                       prerender_contents->has_finished_loading());
   prerender_contents->CommitHistory(new_web_contents);
 
-  GURL icon_url = prerender_contents->icon_url();
-
-  if (!icon_url.is_empty()) {
-#if defined(OS_ANDROID)
-    // Do the delayed icon fetch since we didn't download
-    // the favicon during prerendering on mobile devices.
-    FaviconTabHelper * favicon_tap_helper =
-        FaviconTabHelper::FromWebContents(new_web_contents);
-    favicon_tap_helper->set_should_fetch_icons(true);
-    favicon_tap_helper->FetchFavicon(icon_url);
-#endif  // defined(OS_ANDROID)
-
-    std::vector<content::FaviconURL> urls;
-    urls.push_back(content::FaviconURL(icon_url, content::FaviconURL::FAVICON));
-    FaviconTabHelper::FromWebContents(new_web_contents)->
-        DidUpdateFaviconURL(prerender_contents->page_id(), urls);
-  }
-
   // Update PPLT metrics:
   // If the tab has finished loading, record a PPLT of 0.
   // If the tab is still loading, reset its start time to the current time.
diff --git a/chrome/browser/prerender/prerender_resource_throttle.cc b/chrome/browser/prerender/prerender_resource_throttle.cc
index f115bb0..5636de8 100644
--- a/chrome/browser/prerender/prerender_resource_throttle.cc
+++ b/chrome/browser/prerender/prerender_resource_throttle.cc
@@ -40,8 +40,9 @@ void PrerenderResourceThrottle::WillStartRequest(bool* defer) {
       content::BrowserThread::UI,
       FROM_HERE,
       base::Bind(&PrerenderResourceThrottle::WillStartRequestOnUI,
-                 AsWeakPtr(), request_->method(), info->GetChildID(),
-                 info->GetRenderFrameID(), request_->url()));
+                 AsWeakPtr(), request_->method(), info->GetResourceType(),
+                 info->GetChildID(), info->GetRenderFrameID(),
+                 request_->url()));
 }
 
 void PrerenderResourceThrottle::WillRedirectRequest(const GURL& new_url,
@@ -75,6 +76,7 @@ void PrerenderResourceThrottle::Cancel() {
 void PrerenderResourceThrottle::WillStartRequestOnUI(
     const base::WeakPtr<PrerenderResourceThrottle>& throttle,
     const std::string& method,
+    ResourceType::Type resource_type,
     int render_process_id,
     int render_frame_id,
     const GURL& url) {
@@ -91,6 +93,13 @@ void PrerenderResourceThrottle::WillStartRequestOnUI(
       prerender_contents->Destroy(FINAL_STATUS_UNSUPPORTED_SCHEME);
       ReportUnsupportedPrerenderScheme(url);
       cancel = true;
+#if defined(OS_ANDROID)
+    } else if (resource_type == ResourceType::FAVICON) {
+      // Delay icon fetching until the contents are getting swapped in
+      // to conserve network usage in mobile devices.
+      prerender_contents->AddResourceThrottle(throttle);
+      return;
+#endif
     }
   }
 
diff --git a/chrome/browser/prerender/prerender_resource_throttle.h b/chrome/browser/prerender/prerender_resource_throttle.h
index 0a6ae56..8a691d1 100644
--- a/chrome/browser/prerender/prerender_resource_throttle.h
+++ b/chrome/browser/prerender/prerender_resource_throttle.h
@@ -49,6 +49,7 @@ class PrerenderResourceThrottle
   static void WillStartRequestOnUI(
       const base::WeakPtr<PrerenderResourceThrottle>& throttle,
       const std::string& method,
+      ResourceType::Type resource_type,
       int render_process_id,
       int render_frame_id,
       const GURL& url);