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

components/favicon/core/large_icon_service.cc

Index: components/favicon/core/large_icon_service.cc
diff --git a/components/favicon/core/large_icon_service.cc b/components/favicon/core/large_icon_service.cc
index b411d4abe64f4d2d4f01cee0415e322fa1642ff9..99784d07e3c910d61eec30a4ffce911ce671cd4e 100644
--- a/components/favicon/core/large_icon_service.cc
+++ b/components/favicon/core/large_icon_service.cc
@@ -4,9 +4,14 @@
 
 #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"
@@ -23,6 +28,7 @@ LargeIconService::LargeIconService(FaviconService* favicon_service)
 LargeIconService::~LargeIconService() {
 }
 
+// Main entry point, runs on owner thread.
 base::CancelableTaskTracker::TaskId
     LargeIconService::GetLargeIconOrFallbackStyle(
         const GURL& page_url,
@@ -38,14 +44,19 @@ base::CancelableTaskTracker::TaskId
       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())
@@ -84,7 +95,8 @@ bool LargeIconService::ResizeLargeIconIfValid(
   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) {
@@ -104,4 +116,27 @@ void LargeIconService::RunLargeIconCallback(
   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