[Web Intents] WebIntentsPickerController will now fetch icon for suggested extensions.

chrome/browser/ui/intents/web_intent_picker_controller.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_picker_controller.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/intents/web_intents_registry_factory.h"
 #include "chrome/browser/intents/cws_intents_registry_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_navigator.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_wrapper.h"
 #include "chrome/browser/webdata/web_data_service.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "content/browser/intents/intent_injector.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_intents_dispatcher.h"
+#include "content/public/common/url_fetcher.h"
+#include "content/public/common/url_fetcher_delegate.h"
+#include "net/base/load_flags.h"
+#include "skia/ext/image_operations.h"
 #include "ui/gfx/codec/png_codec.h"
+#include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image.h"
 #include "webkit/glue/web_intent_service_data.h"
 
 namespace {
 
 // Gets the favicon service for the profile in |tab_contents|.
 FaviconService* GetFaviconService(TabContentsWrapper* wrapper) {
   return wrapper->profile()->GetFaviconService(Profile::EXPLICIT_ACCESS);
 }
 
@@ skipping 13 matching lines @@
     case webkit_glue::WebIntentServiceData::DISPOSITION_INLINE:
       return WebIntentPickerModel::DISPOSITION_INLINE;
     case webkit_glue::WebIntentServiceData::DISPOSITION_WINDOW:
       return WebIntentPickerModel::DISPOSITION_WINDOW;
     default:
       NOTREACHED();
       return WebIntentPickerModel::DISPOSITION_WINDOW;
   }
 }
 
+class URLFetcherTrampoline : public content::URLFetcherDelegate {
+ public:
+  typedef base::Callback<void(const content::URLFetcher* source)> Callback;
+
+  explicit URLFetcherTrampoline(const Callback& callback)
+      : callback_(callback) {}
+  ~URLFetcherTrampoline() {}
+
+  // content::URLFetcherDelegate implementation.
+  virtual void OnURLFetchComplete(const content::URLFetcher* source) OVERRIDE {
+    callback_.Run(source);
+    delete source;
+    delete this;
+  }
+
+ private:
+  Callback callback_;
+};
+
 }  // namespace
 
 WebIntentPickerController::WebIntentPickerController(
     TabContentsWrapper* wrapper)
     : wrapper_(wrapper),
       picker_(NULL),
       picker_model_(new WebIntentPickerModel()),
       pending_async_count_(0),
       picker_shown_(false),
       intents_dispatcher_(NULL),
@@ skipping 205 matching lines @@
       picker_model_->UpdateFaviconAt(index, icon_image);
       return;
     }
   }
 
   AsyncOperationFinished();
 }
 
 void WebIntentPickerController::OnCWSIntentServicesAvailable(
     const CWSIntentsRegistry::IntentExtensionList& extensions) {
+  net::URLRequestContextGetter* context =
+      wrapper_->profile()->GetRequestContext();
+
   for (size_t i = 0; i < extensions.size(); ++i) {
     const CWSIntentsRegistry::IntentExtensionInfo& info = extensions[i];
     picker_model_->AddSuggestedExtension(
         info.name,
         info.id,
         info.average_rating);
 
-    // TODO(binji): Fetch extension icon.
+    pending_async_count_++;
+    content::URLFetcher* icon_url_fetcher = content::URLFetcher::Create(
+        0,
+        info.icon_url,
+        content::URLFetcher::GET,
+        new URLFetcherTrampoline(
+            base::Bind(
+                &WebIntentPickerController::OnExtensionIconURLFetchComplete,
+                weak_ptr_factory_.GetWeakPtr(), info.id)));
+
+    icon_url_fetcher->SetLoadFlags(
+        net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES);
+    icon_url_fetcher->SetRequestContext(context);
+    icon_url_fetcher->Start();
   }
 
   AsyncOperationFinished();
 }
 
+void WebIntentPickerController::OnExtensionIconURLFetchComplete(

[Greg Billock, 2012/02/23 18:38:34]

This'll get called by the fetcher thread, right? Is it safe to use that thread
to do the icon decoding, or is that a no-no?

[binji, 2012/02/23 19:06:13]

AFAIK this is called back on the UI thread, so it seemed like we shouldn't do
much work on it (even if decoding and resizing a PNG is lightweight).

+    const string16& extension_id, const content::URLFetcher* source) {
+  if (source->GetResponseCode() != 200)
+    return;

[Greg Billock, 2012/02/23 18:38:34]

Need to AsyncOperationFinished here?

[binji, 2012/02/23 19:06:13]

Done.

+
+  scoped_ptr<std::string> response(new std::string());

[Greg Billock, 2012/02/23 18:38:34]

Can kill inner "()"

[binji, 2012/02/23 19:06:13]

Done.

+  if (source->GetResponseAsString(response.get())) {
+    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+    // Decode PNG and resize on worker thread.
+    content::BrowserThread::PostBlockingPoolTask(

[Greg Billock, 2012/02/23 18:38:34]

These cascades are why the frontends ended up using state-machine-driven
callback systems. After a while it gets hard to track the closures and error
states. :-)

[binji, 2012/02/23 19:06:13]

Agreed. Though I think it's not too hard to follow here...

+        FROM_HERE,
+        base::Bind(&DecodeExtensionIconAndResize,
+                   base::Passed(&response),
+                   base::Bind(
+                       &WebIntentPickerController::OnExtensionIconAvailable,
+                       weak_ptr_factory_.GetWeakPtr(),
+                       extension_id),
+                   base::Bind(
+                       &WebIntentPickerController::OnExtensionIconUnavailable,
+                       weak_ptr_factory_.GetWeakPtr(),
+                       extension_id)));
+  } else {
+    AsyncOperationFinished();
+  }
+}
+
+// static
+void WebIntentPickerController::DecodeExtensionIconAndResize(
+    scoped_ptr<std::string> icon_response,
+    const ExtensionIconAvailableCallback& callback,
+    const base::Closure& unavailable_callback) {
+  SkBitmap icon_bitmap;
+  if (gfx::PNGCodec::Decode(
+        reinterpret_cast<const unsigned char*>(icon_response->data()),
+        icon_response->length(),
+        &icon_bitmap)) {
+    SkBitmap resized_icon = skia::ImageOperations::Resize(

[Greg Billock, 2012/02/23 18:38:34]

Is this doing an extra image copy?

[binji, 2012/02/23 19:06:13]

Yes, it doesn't resize in-place.

+        icon_bitmap,
+        skia::ImageOperations::RESIZE_BEST,
+        gfx::kFaviconSize, gfx::kFaviconSize);
+    gfx::Image icon_image(new SkBitmap(resized_icon));
+
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(callback, icon_image));

[Greg Billock, 2012/02/23 18:38:34]

So gfx::Image is pointer-ish and cool to pass like this?

[binji, 2012/02/23 19:06:13]

Yes, gfx::Image has internal refcounted storage.

+  } else {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI,
+        FROM_HERE,
+        unavailable_callback);
+  }
+}
+
+void WebIntentPickerController::OnExtensionIconAvailable(
+    const string16& extension_id,
+    const gfx::Image& icon_image) {
+  picker_model_->SetSuggestedExtensionIconWithId(extension_id, icon_image);
+  AsyncOperationFinished();
+}
+
+void WebIntentPickerController::OnExtensionIconUnavailable(
+    const string16& extension_id) {
+  AsyncOperationFinished();
+}
+
 void WebIntentPickerController::AsyncOperationFinished() {
   if (--pending_async_count_ == 0) {

[Greg Billock, 2012/02/23 18:38:34]

So we don't need a mutex here because Decode() is posting the callbacks back to
the browser thread? Would it just be easier to have them call directly? There's
very little contention risk... Or would the model freak out to be called
multi-threaded?

[binji, 2012/02/23 19:06:13]

This callback is only used to notify a test that all operations are finished.
Having it call multi-threaded makes those tests more complicated, I think.

I'll add a thread check for this function to make it a little clearer.

     picker_->OnPendingAsyncCompleted();
   }
 }
 
 void WebIntentPickerController::ClosePicker() {
   if (picker_) {
     picker_->Close();
   }
 }