Make image_decoder a Leaky LazyInstance

chrome/browser/image_decoder.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/image_decoder.h"
 
 #include "base/bind.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/common/chrome_utility_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/utility_process_host.h"
 
 using content::BrowserThread;
 using content::UtilityProcessHost;
 
-ImageDecoder::ImageDecoder(Delegate* delegate,
-                           const std::string& image_data,
-                           ImageCodec image_codec)
-    : delegate_(delegate),
-      image_data_(image_data.begin(), image_data.end()),
-      image_codec_(image_codec),
-      task_runner_(NULL),
-      shrink_to_fit_(false) {
+namespace {

[Lei Zhang, 2015/03/25 21:44:46]

nit: add a blank line after this and before the end of the anonymous namespace.
There should be 2 spaces in the ending comment.

[Theresa, 2015/03/25 23:13:15]

Done.

+// static, Leaky to allow access from any thread.
+base::LazyInstance<ImageDecoder>::Leaky g_decoder_ = LAZY_INSTANCE_INITIALIZER;

[Lei Zhang, 2015/03/25 21:44:46]

nit: just |g_decoder| since it's not a member variable.

[Theresa, 2015/03/25 23:13:15]

Done.

+} // namespace
+
+ImageDecoder::ImageDecoder()
+    : image_request_id_counter_(0), last_request_(base::TimeTicks::Now()) {
+  // A single ImageDecoder instance should live for the life of the program.
+  // Explicitly add a reference so the object isn't deleted.
+  AddRef();
 }
 
-ImageDecoder::ImageDecoder(Delegate* delegate,
-                           const std::vector<char>& image_data,
-                           ImageCodec image_codec)
-    : delegate_(delegate),
-      image_data_(image_data.begin(), image_data.end()),
-      image_codec_(image_codec),
-      task_runner_(NULL),
-      shrink_to_fit_(false) {
+ImageDecoder::~ImageDecoder() {
 }
 
-ImageDecoder::~ImageDecoder() {}
-
-void ImageDecoder::Start(scoped_refptr<base::SequencedTaskRunner> task_runner) {
-  task_runner_ = task_runner;
-  BrowserThread::PostTask(
-     BrowserThread::IO, FROM_HERE,
-     base::Bind(&ImageDecoder::DecodeImageInSandbox, this, image_data_));
+ImageDecoder::ImageRequest::ImageRequest(
+    const scoped_refptr<base::SequencedTaskRunner>& task_runner)
+    : task_runner_(task_runner) {
 }
 
-bool ImageDecoder::OnMessageReceived(const IPC::Message& message) {
+ImageDecoder::ImageRequest::~ImageRequest() {
+  ImageDecoder::Cancel(this);
+}
+
+// static
+void ImageDecoder::Start(ImageRequest* image_request,
+                         const std::string& image_data,
+                         ImageCodec image_codec,
+                         bool shrink_to_fit) {
+  DCHECK(image_request);
+  DCHECK(image_request->task_runner());
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(
+          &ImageDecoder::DecodeImageInSandbox,
+          g_decoder_.Pointer(), image_request,
+          std::vector<unsigned char>(image_data.begin(), image_data.end()),
+          image_codec, shrink_to_fit));
+}
+
+// static
+void ImageDecoder::Cancel(ImageRequest* image_request) {
+  DCHECK(image_request);
+  g_decoder_.Pointer()->CancelImpl(image_request);
+}
+
+void ImageDecoder::DecodeImageInSandbox(
+    ImageRequest* image_request,
+    const std::vector<unsigned char>& image_data,
+    ImageCodec image_codec,
+    bool shrink_to_fit) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

[Lei Zhang, 2015/03/25 21:44:46]

DCHECK_CURRENTLY_ON()

[Theresa, 2015/03/25 23:13:15]

Done.

+  if (!utility_process_host_) {
+    StartBatchMode();
+  }
+
+  last_request_ = base::TimeTicks::Now();

[Lei Zhang, 2015/03/25 21:44:46]

If |batch_mode_timer_| is running, can't you use base::Timer::Reset() here,
instead of tracking a separate |last_request_| ?

[Theresa, 2015/03/25 23:13:15]

Yes, Reset() would effectively do the same thing. I chose not to use it, because
for some use cases (e.g. scrolling through bookmarks), hundreds of image
decoding requests may come in at once and calling Reset on the timer each time
would cause more churn than checking if batch mode should be stopped every 5
seconds.

The code for Timer::Reset() doesn't look like it has super high overhead if
there is an existing scheduled task that can be reused, though, so I'm not
opposed to changing this to a OneShotTimer w/ calls to Reset(). 

+  base::AutoLock lock(map_lock_);
+  image_request_id_map_.insert(
+      std::make_pair(image_request_id_counter_, image_request));
+
+  switch (image_codec) {
+    case ROBUST_JPEG_CODEC:
+      utility_process_host_->Send(new ChromeUtilityMsg_RobustJPEGDecodeImage(
+          image_data, image_request_id_counter_));
+      break;
+    case DEFAULT_CODEC:
+      utility_process_host_->Send(new ChromeUtilityMsg_DecodeImage(
+          image_data, shrink_to_fit, image_request_id_counter_));
+      break;
+  }
+
+  ++image_request_id_counter_;
+}
+
+void ImageDecoder::CancelImpl(ImageRequest* image_request) {
+  base::AutoLock lock(map_lock_);
+  for (auto it = image_request_id_map_.begin();
+       it != image_request_id_map_.end();) {
+    if (it->second == image_request) {
+      image_request_id_map_.erase(it++);
+    } else {
+      ++it;
+    }
+  }
+}
+
+void ImageDecoder::StartBatchMode() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  utility_process_host_ =
+      UtilityProcessHost::Create(this, base::MessageLoopProxy::current().get())
+          ->AsWeakPtr();
+  utility_process_host_->StartBatchMode();
+  batch_mode_timer_.Start(
+      FROM_HERE, base::TimeDelta::FromSeconds(kBatchModeTimeoutSeconds),
+      this, &ImageDecoder::StopBatchMode);
+}
+
+void ImageDecoder::StopBatchMode() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if ((base::TimeTicks::Now() - last_request_)
+      < base::TimeDelta::FromSeconds(kBatchModeTimeoutSeconds)) {
+    return;
+  }
+
+  if (utility_process_host_) {
+    utility_process_host_->EndBatchMode();
+    utility_process_host_.reset();
+  }
+  batch_mode_timer_.Stop();
+}
+
+bool ImageDecoder::OnMessageReceived(
+    const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(ImageDecoder, message)
     IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_DecodeImage_Succeeded,
                         OnDecodeImageSucceeded)
     IPC_MESSAGE_HANDLER(ChromeUtilityHostMsg_DecodeImage_Failed,
                         OnDecodeImageFailed)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
-void ImageDecoder::OnDecodeImageSucceeded(const SkBitmap& decoded_image) {
-  DCHECK(task_runner_->RunsTasksOnCurrentThread());
-  if (delegate_)
-    delegate_->OnImageDecoded(this, decoded_image);
+void ImageDecoder::OnDecodeImageSucceeded(
+    const SkBitmap& decoded_image,
+    int id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  base::AutoLock lock(map_lock_);
+  auto it = image_request_id_map_.find(id);
+  if (it != image_request_id_map_.end()) {
+    ImageRequest* image_request = it->second;
+    image_request->task_runner()->PostTask(
+        FROM_HERE, base::Bind(&ImageRequest::OnImageDecoded,
+                              base::Unretained(image_request), decoded_image));
+
+    image_request_id_map_.erase(it);
+  }
 }
 
-void ImageDecoder::OnDecodeImageFailed() {
-  DCHECK(task_runner_->RunsTasksOnCurrentThread());
-  if (delegate_)
-    delegate_->OnDecodeImageFailed(this);
-}
+void ImageDecoder::OnDecodeImageFailed(int id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  base::AutoLock lock(map_lock_);
+  auto it = image_request_id_map_.find(id);
+  if (it != image_request_id_map_.end()) {
+    ImageRequest* image_request = it->second;
+    image_request->task_runner()->PostTask(
+        FROM_HERE, base::Bind(&ImageRequest::OnDecodeImageFailed,
+                              base::Unretained(image_request)));
 
-void ImageDecoder::DecodeImageInSandbox(
-    const std::vector<unsigned char>& image_data) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  UtilityProcessHost* utility_process_host;
-  utility_process_host = UtilityProcessHost::Create(this, task_runner_.get());
-  if (image_codec_ == ROBUST_JPEG_CODEC) {
-    utility_process_host->Send(
-        new ChromeUtilityMsg_RobustJPEGDecodeImage(image_data));
-  } else {
-    utility_process_host->Send(
-        new ChromeUtilityMsg_DecodeImage(image_data, shrink_to_fit_));
+    image_request_id_map_.erase(it);
   }
 }