[Large Icon Service] Move icon resizing into worker thread.

components/favicon/core/large_icon_service.cc

 // Copyright 2015 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 "components/favicon/core/large_icon_service.h"
 
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/task_runner.h"
+#include "base/threading/sequenced_worker_pool.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon_base/fallback_icon_style.h"
 #include "components/favicon_base/favicon_types.h"
+#include "content/public/browser/browser_thread.h"
 #include "skia/ext/image_operations.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace favicon {
 
 LargeIconService::LargeIconService(FaviconService* favicon_service)
     : favicon_service_(favicon_service) {
   large_icon_types_.push_back(favicon_base::IconType::FAVICON);
   large_icon_types_.push_back(favicon_base::IconType::TOUCH_ICON);
   large_icon_types_.push_back(favicon_base::IconType::TOUCH_PRECOMPOSED_ICON);
 }
 
 LargeIconService::~LargeIconService() {
 }
 
+// Main entry point, runs on owner thread.
 base::CancelableTaskTracker::TaskId
     LargeIconService::GetLargeIconOrFallbackStyle(
         const GURL& page_url,
         int desired_size_in_pixel,
         const favicon_base::LargeIconCallback& callback,
         base::CancelableTaskTracker* tracker) {
   // TODO(beaudoin): For now this is just a wrapper around
   //   GetLargestRawFaviconForPageURL. Add the logic required to select the best
   //   possible large icon. Also add logic to fetch-on-demand when the URL of
   //   a large icon is known but its bitmap is not available.
   return favicon_service_->GetLargestRawFaviconForPageURL(
       page_url,
       large_icon_types_,
       desired_size_in_pixel,
       base::Bind(&LargeIconService::RunLargeIconCallback,
-                 base::Unretained(this), callback, desired_size_in_pixel),
+                 base::Unretained(this),
+                 callback,
+                 desired_size_in_pixel,
+                 tracker),
       tracker);
 }
 
+// Runs on background thread.
 bool LargeIconService::ResizeLargeIconIfValid(
     int desired_size_in_pixel,
     const favicon_base::FaviconRawBitmapResult& bitmap_result,
     favicon_base::FaviconRawBitmapResult* resized_bitmap_result) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   // Require bitmap to be valid and square.
   if (!bitmap_result.is_valid() ||
       bitmap_result.pixel_size.width() != bitmap_result.pixel_size.height())
     return false;
 
   // Require bitmap to be large enough. It's square, so just check width.
   if (bitmap_result.pixel_size.width() < desired_size_in_pixel) {
     // TODO(beaudoin): Potentially relax this, and allow icon to be scaled up.
     return false;
   }
@@ skipping 18 matching lines @@
   if (!gfx::PNGCodec::EncodeBGRASkBitmap(resized_bitmap, false, &bitmap_data))
     return false;
 
   resized_bitmap_result->pixel_size =
       gfx::Size(desired_size_in_pixel, desired_size_in_pixel);
   resized_bitmap_result->bitmap_data =
       base::RefCountedBytes::TakeVector(&bitmap_data);
   return true;
 }
 
-void LargeIconService::RunLargeIconCallback(
+// Runs on background thread.
+void LargeIconService::RunLargeIconCallbackOnBackgroundThread(
     const favicon_base::LargeIconCallback& callback,
     int desired_size_in_pixel,
     const favicon_base::FaviconRawBitmapResult& bitmap_result) {
   favicon_base::FaviconRawBitmapResult resized_bitmap_result;
   if (ResizeLargeIconIfValid(desired_size_in_pixel, bitmap_result,
                              &resized_bitmap_result)) {
     callback.Run(favicon_base::LargeIconResult(resized_bitmap_result));
     return;
   }
 
   // Failed to resize |bitmap_result|, so compute fallback icon style.
   favicon_base::LargeIconResult result(new favicon_base::FallbackIconStyle());
   if (bitmap_result.is_valid()) {
     favicon_base::SetDominantColorAsBackground(
         bitmap_result.bitmap_data, result.fallback_icon_style.get());
   }
   callback.Run(result);

[beaudoin, 2015/05/05 00:00:52]

Shouldn't that callback go back to the initial thread? The invoker may not be
thread safe or may expect to be on the UI thread. (Not entirely sure how to go
back to the owner's thread...)

[huangs, 2015/05/05 15:22:13]

Done.

 }
 
+// Runs on owner thread.
+void LargeIconService::RunLargeIconCallback(
+    const favicon_base::LargeIconCallback& callback,
+    int desired_size_in_pixel,
+    base::CancelableTaskTracker* tracker,
+    const favicon_base::FaviconRawBitmapResult& bitmap_result) {
+  DCHECK(thread_checker_.CalledOnValidThread());

[beaudoin, 2015/05/05 00:00:52]

From reading CalledOnValidThread it looks like this first call will take a
reference to the current thread ID which will later be checked against the
ThreadID in ResizeLargeIconIfValid. As a result I would expect the second call
to always fail since it's in a different thread.

I wonder if there is a way to check that the threads are actually different, or
if DCHECKing this is actually desirable.

[huangs, 2015/05/05 15:22:12]

Using a different way to check.

+  tracker->PostTask(
+      GetBackgroundTaskRunner().get(),
+      FROM_HERE,
+      base::Bind(&LargeIconService::RunLargeIconCallbackOnBackgroundThread,
+                 base::Unretained(this),
+                 callback,
+                 desired_size_in_pixel,
+                 bitmap_result));
+}
+
+scoped_refptr<base::TaskRunner> LargeIconService::GetBackgroundTaskRunner() {
+  return content::BrowserThread::GetBlockingPool()->
+      GetTaskRunnerWithShutdownBehavior(
+          base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
+}
+
 }  // namespace favicon