Factoring out of FaviconLoader

TBR=jhawkins@chromium.org
R=gbillock@chromium.org

BUG=none

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

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

chrome/browser/ui/intents/web_intent_icon_loader.cc

+// 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.
+
+#include "chrome/browser/ui/intents/web_intent_icon_loader.h"
+
+#include "chrome/browser/favicon/favicon_service.h"
+#include "chrome/browser/favicon/favicon_service_factory.h"
+#include "chrome/browser/ui/intents/web_intent_picker_model.h"
+#include "ui/gfx/favicon_size.h"
+
+namespace web_intents {
+
+IconLoader::IconLoader(Profile* profile,
+                       WebIntentPickerModel* model)
+    : profile_(profile),
+      model_(model),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+}
+
+IconLoader::~IconLoader() {}
+
+void IconLoader::LoadFavicon(const GURL& url) {
+  FaviconService* favicon_service =
+      FaviconServiceFactory::GetForProfile(profile_,
+                                           Profile::EXPLICIT_ACCESS);
+
+  favicon_service->GetFaviconImageForURL(
+      FaviconService::FaviconForURLParams(
+          profile_,
+          url,
+          history::FAVICON,
+          gfx::kFaviconSize,
+          &favicon_consumer_),
+      base::Bind(
+          &IconLoader::OnFaviconDataAvailable,
+          weak_ptr_factory_.GetWeakPtr(),
+          url));
+}
+
+void IconLoader::OnFaviconDataAvailable(
+    const GURL& url,
+    FaviconService::Handle,
+    const history::FaviconImageResult& image_result) {
+  if (!image_result.image.IsEmpty())
+    model_->UpdateFaviconForServiceWithURL(url, image_result.image);
+}
+
+}  // namespace web_intents

chrome/browser/ui/intents/web_intent_icon_loader.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_UI_INTENTS_WEB_INTENT_ICON_LOADER_H_
+#define CHROME_BROWSER_UI_INTENTS_WEB_INTENT_ICON_LOADER_H_
+
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/common/cancelable_request.h"
+#include "chrome/browser/favicon/favicon_service.h"
+
+class Profile;
+class WebIntentPickerModel;
+
+namespace web_intents {
+
+class IconLoader {
+ public:
+  IconLoader(Profile* profile, WebIntentPickerModel* model);
+  ~IconLoader();
+
+  // Load the favicon associated with |url|.
+  void LoadFavicon(const GURL& url);
+
+ private:
+  // Called when a favicon is returned from the FaviconService.
+  void OnFaviconDataAvailable(
+      const GURL& url,
+      FaviconService::Handle handle,
+      const history::FaviconImageResult& image_result);
+
+  // A weak pointer to the profile for the web contents.
+  Profile* profile_;
+
+  // A weak pointer to the model being updated.
+  WebIntentPickerModel* model_;
+
+  // Request consumer used when asynchronously loading favicons.
+  CancelableRequestConsumer favicon_consumer_;
+
+  // Factory for weak pointers used in callbacks for async calls to load icon.
+  base::WeakPtrFactory<IconLoader> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(IconLoader);
+};
+
+}  // namespace web_intents;
+
+#endif  // CHROME_BROWSER_UI_INTENTS_WEB_INTENT_ICON_LOADER_H_

chrome/browser/ui/intents/web_intent_picker_controller.cc

@@ -16,8 +16,6 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/platform_app_launcher.h"
 #include "chrome/browser/extensions/webstore_installer.h"
-#include "chrome/browser/favicon/favicon_service.h"
-#include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/intents/cws_intents_registry_factory.h"
 #include "chrome/browser/intents/default_web_intent_service.h"
 #include "chrome/browser/intents/intent_service_host.h"
@@ -33,6 +31,7 @@
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/constrained_window_tab_helper.h"
+#include "chrome/browser/ui/intents/web_intent_icon_loader.h"
 #include "chrome/browser/ui/intents/web_intent_picker.h"
 #include "chrome/browser/ui/intents/web_intent_picker_model.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
@@ -65,13 +64,6 @@ const int kMaxHiddenSetupTimeMs = 200;
 // Minimum amount of time to show waiting dialog, if it is shown.
 const int kMinThrobberDisplayTimeMs = 800;
 
-
-// Gets the favicon service for the specified profile.
-FaviconService* GetFaviconService(Profile* profile) {
-  return FaviconServiceFactory::GetForProfile(profile,
-                                              Profile::EXPLICIT_ACCESS);
-}
-
 // Gets the web intents registry for the specified profile.
 WebIntentsRegistry* GetWebIntentsRegistry(Profile* profile) {
   return WebIntentsRegistryFactory::GetForProfile(profile);
@@ -113,7 +105,6 @@ void URLFetcherTrampoline::OnURLFetchComplete(
   delete source;
   delete this;
 }
-
 class SourceWindowObserver : content::WebContentsObserver {
  public:
   SourceWindowObserver(content::WebContents* web_contents,
@@ -187,6 +178,7 @@ WebIntentPickerController::WebIntentPickerController(
       source_intents_dispatcher_(NULL),
       intents_dispatcher_(NULL),
       service_tab_(NULL),
+      icon_loader_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(timer_factory_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(dispatcher_factory_(this)) {
@@ -194,6 +186,8 @@ WebIntentPickerController::WebIntentPickerController(
 #if defined(TOOLKIT_VIEWS)
   cancelled_ = true;
 #endif
+  icon_loader_.reset(
+      new web_intents::IconLoader(profile_, picker_model_.get()));
 }
 
 WebIntentPickerController::~WebIntentPickerController() {
@@ -613,26 +607,13 @@ void WebIntentPickerController::OnSendReturnMessage(
 
 void WebIntentPickerController::AddServiceToModel(
     const webkit_glue::WebIntentServiceData& service) {
-  FaviconService* favicon_service = GetFaviconService(profile_);
 
   picker_model_->AddInstalledService(
       service.title,
       service.service_url,
       service.disposition);
 
-  pending_async_count_++;
-  FaviconService::Handle handle = favicon_service->GetFaviconImageForURL(
-      FaviconService::FaviconForURLParams(
-          profile_,
-          service.service_url,
-          history::FAVICON,
-          gfx::kFaviconSize,
-          &favicon_consumer_),
-      base::Bind(
-          &WebIntentPickerController::OnFaviconDataAvailable,
-          weak_ptr_factory_.GetWeakPtr()));
-  favicon_consumer_.SetClientData(
-      favicon_service, handle, picker_model_->GetInstalledServiceCount() - 1);
+  icon_loader_->LoadFavicon(service.service_url);
 }
 
 void WebIntentPickerController::OnWebIntentServicesAvailable(
@@ -697,18 +678,6 @@ void WebIntentPickerController::RegistryCallsCompleted() {
   OnIntentDataArrived();
 }
 
-void WebIntentPickerController::OnFaviconDataAvailable(
-    FaviconService::Handle handle,
-    const history::FaviconImageResult& image_result) {
-  size_t index = favicon_consumer_.GetClientDataForCurrentRequest();
-  if (!image_result.image.IsEmpty()) {
-    picker_model_->UpdateFaviconAt(index, image_result.image);
-    return;
-  }
-
-  AsyncOperationFinished();
-}
-
 void WebIntentPickerController::OnCWSIntentServicesAvailable(
     const CWSIntentsRegistry::IntentExtensionList& extensions) {
   ExtensionServiceInterface* extension_service =
@@ -812,7 +781,11 @@ void WebIntentPickerController::Reset() {
   pending_cws_request_ = false;
 
   // Reset picker.
+  icon_loader_.reset();
   picker_model_.reset(new WebIntentPickerModel());
+  icon_loader_.reset(
+      new web_intents::IconLoader(profile_, picker_model_.get()));
+
   picker_shown_ = false;
 
   DCHECK(web_contents_);

chrome/browser/ui/intents/web_intent_picker_controller.h

@@ -13,7 +13,6 @@
 #include "base/memory/weak_ptr.h"
 #include "base/string16.h"
 #include "chrome/browser/extensions/webstore_installer.h"
-#include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/intents/cws_intents_registry.h"
 #include "chrome/browser/intents/web_intents_registry.h"
 #include "chrome/browser/intents/web_intents_reporting.h"
@@ -38,6 +37,7 @@ class WebIntentsDispatcher;
 
 namespace web_intents {
 class NativeServiceFactory;
+class IconLoader;
 }
 
 namespace webkit_glue {
@@ -207,11 +207,6 @@ class WebIntentPickerController
   void OnWebIntentServicesAvailableForExplicitIntent(
       const std::vector<webkit_glue::WebIntentServiceData>& services);
 
-  // Called when a favicon is returned from the FaviconService.
-  void OnFaviconDataAvailable(
-      FaviconService::Handle handle,
-      const history::FaviconImageResult& image_result);
-
   // Called when IntentExtensionInfo is returned from the CWSIntentsRegistry.
   void OnCWSIntentServicesAvailable(
       const CWSIntentsRegistry::IntentExtensionList& extensions);
@@ -341,8 +336,8 @@ class WebIntentPickerController
   // close it when a reply is sent.
   content::WebContents* service_tab_;
 
-  // Request consumer used when asynchronously loading favicons.
-  CancelableRequestConsumerTSimple<size_t> favicon_consumer_;
+  // Object managing the details of icon loading.
+  scoped_ptr<web_intents::IconLoader> icon_loader_;
 
   // Factory for weak pointers used in callbacks for async calls to load the
   // picker model.

chrome/browser/ui/intents/web_intent_picker_model.cc

@@ -91,12 +91,18 @@ size_t WebIntentPickerModel::GetInstalledServiceCount() const {
   return installed_services_.size();
 }
 
-void WebIntentPickerModel::UpdateFaviconAt(size_t index,
-                                           const gfx::Image& image) {
-  DCHECK_LT(index, installed_services_.size());
-  installed_services_[index]->favicon = image;
-  if (observer_)
-    observer_->OnFaviconChanged(this, index);
+void WebIntentPickerModel::UpdateFaviconForServiceWithURL(
+    const GURL& url, const gfx::Image& image) {
+  for (size_t i = 0; i < installed_services_.size(); ++i) {
+    InstalledService* service = installed_services_[i];
+    if (service->url == url) {
+      service->favicon = image;
+      if (observer_)
+        observer_->OnFaviconChanged(this, i);
+      return;
+    }
+  }
+  NOTREACHED();  // Calling this with an invalid URL is not allowed.
 }
 
 void WebIntentPickerModel::AddSuggestedExtensions(

chrome/browser/ui/intents/web_intent_picker_model.h

@@ -106,8 +106,8 @@ class WebIntentPickerModel {
   // Return the number of intent services in the picker.
   size_t GetInstalledServiceCount() const;
 
-  // Update the favicon for the intent service at |index| to |image|.
-  void UpdateFaviconAt(size_t index, const gfx::Image& image);
+  // Update favicon for the intent service with service URL |url| to |image|.
+  void UpdateFaviconForServiceWithURL(const GURL& url, const gfx::Image& image);
 
   // Add a list of suggested extensions to the model.
   void AddSuggestedExtensions(

chrome/browser/ui/intents/web_intent_picker_model_unittest.cc

@@ -134,14 +134,14 @@ TEST_F(WebIntentPickerModelTest, GetInstalledServiceWithURL) {
   EXPECT_EQ(NULL, model_.GetInstalledServiceWithURL(kUrl3));
 }
 
-TEST_F(WebIntentPickerModelTest, UpdateFaviconAt) {
+TEST_F(WebIntentPickerModelTest, UpdateFaviconForServiceWithURL) {
   EXPECT_CALL(observer_, OnModelChanged(&model_)).Times(2);
   EXPECT_CALL(observer_, OnFaviconChanged(&model_, 1U)).Times(1);
 
   model_.AddInstalledService(kTitle1, kUrl1, kWindowDisposition);
   model_.AddInstalledService(kTitle2, kUrl2, kWindowDisposition);
   gfx::Image image(gfx::test::CreateImage());
-  model_.UpdateFaviconAt(1U, image);
+  model_.UpdateFaviconForServiceWithURL(kUrl2, image);
 
   EXPECT_FALSE(gfx::test::IsEqual(
       image, model_.GetInstalledServiceAt(0).favicon));

chrome/chrome_browser_ui.gypi

@@ -1089,6 +1089,8 @@
         'browser/ui/gtk/zoom_bubble_gtk.h',
         'browser/ui/hung_plugin_tab_helper.cc',
         'browser/ui/hung_plugin_tab_helper.h',
+        'browser/ui/intents/web_intent_icon_loader.h',
+        'browser/ui/intents/web_intent_icon_loader.cc',
         'browser/ui/intents/native_file_picker_service.cc',
         'browser/ui/intents/web_intent_inline_disposition_delegate.cc',
         'browser/ui/intents/web_intent_inline_disposition_delegate.h',