[Download Home] Add image thumbnail support

chrome/browser/android/download/ui/thumbnail_provider.cc

+// Copyright 2016 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/android/download/ui/thumbnail_provider.h"
+
+#include "base/android/jni_string.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/image_decoder.h"
+#include "content/public/browser/browser_thread.h"
+#include "jni/ThumbnailProviderImpl_jni.h"
+#include "skia/ext/image_operations.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/android/java_bitmap.h"
+#include "ui/gfx/skbitmap_operations.h"
+
+class SkBitmap;
+
+using base::android::JavaParamRef;
+
+namespace {
+
+// Ignore image files that are too large to avoid long delays.
+const int64_t kMaxImageSize = 10 * 1024 * 1024;  // 10 MB
+
+class ImageThumbnailRequest : public ImageDecoder::ImageRequest {
+ public:
+  ImageThumbnailRequest(int icon_size,
+                        const std::string& file_path,
+                        base::WeakPtr<ThumbnailProvider> weak_provider)
+      : icon_size_(icon_size),
+        file_path_(file_path),
+        weak_provider_(weak_provider) {}
+
+  void Start() {
+    // Confirm that the file's size is within our threshold.
+    int64_t file_size;
+    base::FilePath path = base::FilePath::FromUTF8Unsafe(file_path_);
+    if (!base::GetFileSize(path, &file_size) || file_size > kMaxImageSize) {
+      LOG(ERROR) << "Unexpected file size: " << file_path_ << ", " << file_size;
+      FinishRequest(SkBitmap());
+    }
+
+    // Make sure the file isn't empty.
+    std::string data;
+    bool success = base::ReadFileToString(path, &data);
+    if (!success || data.empty()) {
+      LOG(ERROR) << "Failed to read file: " << file_path_;
+      FinishRequest(SkBitmap());
+    }
+
+    ImageDecoder::Start(this, data);
+  }
+
+  void OnImageDecoded(const SkBitmap& decoded_image) override {
+    SkBitmap thumbnail = decoded_image;
+    if (!decoded_image.drawsNothing()) {
+      // Shrink the image down so that its smallest dimension is equal to or
+      // smaller than the requested size.
+      int min_dimension =
+          std::min(decoded_image.width(), decoded_image.height());
+
+      if (min_dimension > icon_size_) {
+        uint64_t width = static_cast<uint64_t>(decoded_image.width());
+        uint64_t height = static_cast<uint64_t>(decoded_image.height());
+        thumbnail = skia::ImageOperations::Resize(
+            decoded_image, skia::ImageOperations::RESIZE_BEST,
+            width * icon_size_ / min_dimension,
+            height * icon_size_ / min_dimension);
+      }
+    }
+
+    FinishRequest(thumbnail);
+  }
+
+  void OnDecodeImageFailed() override {
+    LOG(ERROR) << "Failed to decode image: " << file_path_;
+    FinishRequest(SkBitmap());
+  }
+
+ private:
+  void FinishRequest(const SkBitmap& thumbnail) {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(&ImageThumbnailRequest::FinishRequestOnUIThread,
+                   base::Unretained(this), thumbnail));

[Ted C, 2016/08/31 20:14:40]

same question as below, should you be passing the weak_ptr here instead of a
reference to this?

Could be a static as well.

Could this also just post to ThumbnailProvider::OnThumbnailRetrieved?  I think
Bind would handle the case where the weak_ptr has gone away.  But I guess that
depends on whether you need DeleteSoon to be called on the UI thread.

[Ian Wen, 2016/08/31 20:43:30]

IIRC, weak_ptr is only better than Unretained() if we are posting on to the same
thread. If on the same thread, weak_ptr will cancel the task itself, rather than
only cancelling the callback.

Yet since this is a multi-threaded case, Unretained(this) should be okay, as
they both only cancel the callback, should "this" is deleted?

https://www.chromium.org/developers/design-documents/threading#base_WeakPtr_a...

[gone, 2016/08/31 22:10:53]

This should be fine because the ImageThumbnailRequest in the current patch
continues to handle its own deletion.

+  }
+
+  void FinishRequestOnUIThread(const SkBitmap& thumbnail) {
+    if (weak_provider_)
+      weak_provider_->OnThumbnailRetrieved(file_path_, thumbnail);
+    task_runner()->DeleteSoon(FROM_HERE, this);
+  }
+
+  const int icon_size_;
+  std::string file_path_;
+  base::WeakPtr<ThumbnailProvider> weak_provider_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(ImageThumbnailRequest);
+};
+
+}  // namespace
+
+ThumbnailProvider::ThumbnailProvider(const JavaParamRef<jobject>& jobj,
+                                     int icon_size)

[Ted C, 2016/08/31 20:14:41]

icon_size isn't used

[gone, 2016/08/31 22:10:53]

Whoops.  Removed.

+    : java_delegate_(jobj), weak_factory_(this) {}
+
+ThumbnailProvider::~ThumbnailProvider() {
+  java_delegate_.Reset();
+}
+
+void ThumbnailProvider::Destroy(JNIEnv* env,
+                                const JavaParamRef<jobject>& jobj) {
+  delete this;
+}
+
+void ThumbnailProvider::OnThumbnailRetrieved(const std::string& file_path,
+                                             const SkBitmap& thumbnail) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (java_delegate_.is_null())
+    return;
+
+  // Send the bitmap back to Java-land.
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_ThumbnailProviderImpl_onThumbnailRetrieved(
+      env, java_delegate_,
+      base::android::ConvertUTF8ToJavaString(env, file_path),
+      gfx::ConvertToJavaBitmap(&thumbnail));
+}
+
+void ThumbnailProvider::RetrieveThumbnail(JNIEnv* env,

[Ted C, 2016/08/31 20:14:41]

What is the longer term plan for this?  Should it handle thumbnails for all file
types or just images?  If the former, then this is good, if the latter I wonder
if this should be called ImageThumbnailProvider (vs VideoThumbnailProvider,
OfflineThumbnailProvider)

[gone, 2016/08/31 22:10:53]

I'm planning on having this class delegate to different retrieval classes.  Main
reason why there's a separate ImageThumbnailRequest class above.

+                                          const JavaParamRef<jobject>& jobj,
+                                          jstring jfile_path,
+                                          jint icon_size) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // The ImageThumbnailRequest deletes itself on completion.
+  std::string path = base::android::ConvertJavaStringToUTF8(env, jfile_path);
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ThumbnailProvider::RetrieveThumbnailOnFileThread,
+                 base::Unretained(this), path, icon_size));

[Ted C, 2016/08/31 20:14:40]

I think you need to pass the weak_ptr here.  You are accessing weak_factory_ in
the FILE thread below and it might have been deleted on the UI thread by
Destroy().

[gone, 2016/08/31 22:10:53]

Blarghhh.  Ian and I walked through the wonkiness here and found a way to keep
the ThumbnailProvider entirely on the UI thread while keeping the
ImageThumbnailRequest on the FILE thread as much as possible.

+}
+
+void ThumbnailProvider::RetrieveThumbnailOnFileThread(
+    const std::string& file_path,
+    int icon_size) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
+
+  ImageThumbnailRequest* request = new ImageThumbnailRequest(
+      icon_size, file_path, weak_factory_.GetWeakPtr());
+  request->Start();
+}
+
+// static
+bool ThumbnailProvider::RegisterJNI(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+// static
+static jlong Init(JNIEnv* env,
+                  const JavaParamRef<jobject>& jobj,
+                  int icon_size) {
+  return reinterpret_cast<intptr_t>(new ThumbnailProvider(jobj, icon_size));
+}