Media Remoting: Draw remoting interstitial on poster image.

content/renderer/image_downloader/image_downloader_core.cc

Index: content/renderer/image_downloader/image_downloader_core.cc
diff --git a/content/renderer/image_downloader/image_downloader_impl.cc b/content/renderer/image_downloader/image_downloader_core.cc
similarity index 34%
copy from content/renderer/image_downloader/image_downloader_impl.cc
copy to content/renderer/image_downloader/image_downloader_core.cc
index a7679900966e61f80eedefda12fd1f9fe4c42cdd..6274008d726155e17439ef46e9f81bf68565607e 100644
--- a/content/renderer/image_downloader/image_downloader_impl.cc
+++ b/content/renderer/image_downloader/image_downloader_core.cc
@@ -1,36 +1,28 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// 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 "content/renderer/image_downloader/image_downloader_impl.h"
+#include "content/renderer/image_downloader/image_downloader_core.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
-#include "base/single_thread_task_runner.h"
-#include "base/threading/thread_task_runner_handle.h"
 #include "content/child/image_decoder.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/renderer/fetchers/multi_resolution_image_resource_fetcher.h"
 #include "net/base/data_url.h"
-#include "skia/ext/image_operations.h"
 #include "third_party/WebKit/public/platform/WebCachePolicy.h"
 #include "third_party/WebKit/public/platform/WebURLRequest.h"
-#include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
-#include "third_party/WebKit/public/web/WebView.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/geometry/size.h"
-#include "ui/gfx/skbitmap_operations.h"
 #include "url/url_constants.h"
 
 using blink::WebCachePolicy;
 using blink::WebFrame;
-using blink::WebVector;
-using blink::WebURL;
 using blink::WebURLRequest;
 
 namespace {
@@ -50,148 +42,54 @@ SkBitmap ImageFromDataUrl(const GURL& url) {
   return SkBitmap();
 }
 
-//  Proportionally resizes the |image| to fit in a box of size
-// |max_image_size|.
-SkBitmap ResizeImage(const SkBitmap& image, uint32_t max_image_size) {
-  if (max_image_size == 0)
-    return image;
-  uint32_t max_dimension = std::max(image.width(), image.height());
-  if (max_dimension <= max_image_size)
-    return image;
-  // Proportionally resize the minimal image to fit in a box of size
-  // max_image_size.
-  return skia::ImageOperations::Resize(
-      image, skia::ImageOperations::RESIZE_BEST,
-      static_cast<uint64_t>(image.width()) * max_image_size / max_dimension,
-      static_cast<uint64_t>(image.height()) * max_image_size / max_dimension);
-}
-
-// Filters the array of bitmaps, removing all images that do not fit in a box of
-// size |max_image_size|. Returns the result if it is not empty. Otherwise,
-// find the smallest image in the array and resize it proportionally to fit
-// in a box of size |max_image_size|.
-// Sets |original_image_sizes| to the sizes of |images| before resizing.
-void FilterAndResizeImagesForMaximalSize(
-    const std::vector<SkBitmap>& unfiltered,
-    uint32_t max_image_size,
-    std::vector<SkBitmap>* images,
-    std::vector<gfx::Size>* original_image_sizes) {
-  images->clear();
-  original_image_sizes->clear();
-
-  if (unfiltered.empty())
-    return;
-
-  if (max_image_size == 0)
-    max_image_size = std::numeric_limits<uint32_t>::max();
-
-  const SkBitmap* min_image = NULL;
-  uint32_t min_image_size = std::numeric_limits<uint32_t>::max();
-  // Filter the images by |max_image_size|, and also identify the smallest image
-  // in case all the images are bigger than |max_image_size|.
-  for (std::vector<SkBitmap>::const_iterator it = unfiltered.begin();
-       it != unfiltered.end(); ++it) {
-    const SkBitmap& image = *it;
-    uint32_t current_size = std::max(it->width(), it->height());
-    if (current_size < min_image_size) {
-      min_image = &image;
-      min_image_size = current_size;
-    }
-    if (static_cast<uint32_t>(image.width()) <= max_image_size &&
-        static_cast<uint32_t>(image.height()) <= max_image_size) {
-      images->push_back(image);
-      original_image_sizes->push_back(gfx::Size(image.width(), image.height()));
-    }
-  }
-  DCHECK(min_image);
-  if (images->size())
-    return;
-  // Proportionally resize the minimal image to fit in a box of size
-  // |max_image_size|.
-  SkBitmap resized = ResizeImage(*min_image, max_image_size);
-  // Drop null or empty SkBitmap.
-  if (resized.drawsNothing())
-    return;
-  images->push_back(resized);
-  original_image_sizes->push_back(
-      gfx::Size(min_image->width(), min_image->height()));
-}
-
 }  // namespace
 
 namespace content {
 
-ImageDownloaderImpl::ImageDownloaderImpl(
-    RenderFrame* render_frame,
-    mojom::ImageDownloaderRequest request)
-    : RenderFrameObserver(render_frame),
-      binding_(this, std::move(request)) {
-  DCHECK(render_frame);
+ImageDownloaderCore::ImageDownloaderCore(RenderFrame* render_frame)
+    : render_frame_(render_frame) {

[miu, 2016/12/03 01:05:30]

It makes sense to use the RenderFrameObserver in this base class and override
OnDestruct() so that it knows when the |render_frame_| pointer is no longer
valid. This could happen, for example, if this class is constructed, but
FetchImage() is called at some later point after the RenderFrame is destroyed.
Also, unlike the original impl, OnDestruct() would *not* "delete this" since
ownership semantics are the responsibility of the subclasses.

[xjz, 2016/12/06 19:50:55]

Done. I do "delete this" when OnDestruct() is called to prevent FetchImage() is
further called. This will not cause the "double delete". Please see my reply to
the other comment.

[xjz, 2016/12/06 23:58:17]

Now removed "delete this" from OnDestruct(), and call callbacks with empty image
vector instead. Note: I don't want to keep a copy of all callbacks in this base
class, so now this works as follows: when OnDestruct() is called, all fetchers
(MultiResolutionImageResourceFetcher) will get notified, and they will call
callback here (ImageDownloaderBase::DidFetchImage).

   RenderThread::Get()->AddObserver(this);
-  binding_.set_connection_error_handler(
-      base::Bind(&ImageDownloaderImpl::OnDestruct, base::Unretained(this)));
 }
 
-ImageDownloaderImpl::~ImageDownloaderImpl() {
+ImageDownloaderCore::~ImageDownloaderCore() {
   RenderThread* thread = RenderThread::Get();
-  // As ImageDownloaderImpl is a strong binding with message pipe, the
-  // destructor may run after message loop shutdown, so we need to check whether
-  // RenderThread is null.
+  // The destructor may run after message loop shutdown, so we need to check
+  // whether RenderThread is null.
   if (thread)
     thread->RemoveObserver(this);
 }
 
-// static
-void ImageDownloaderImpl::CreateMojoService(
-    RenderFrame* render_frame,
-    mojom::ImageDownloaderRequest request) {
-  DVLOG(1) << "ImageDownloaderImpl::CreateMojoService";
-  DCHECK(render_frame);
-
-  // Owns itself.
-  new ImageDownloaderImpl(render_frame, std::move(request));
-}
-
 // Ensure all loaders cleared before calling blink::shutdown.
-void ImageDownloaderImpl::OnRenderProcessShutdown() {
+void ImageDownloaderCore::OnRenderProcessShutdown() {
   image_fetchers_.clear();
 }
 
-// ImageDownloader methods:
-void ImageDownloaderImpl::DownloadImage(const GURL& image_url,
+void ImageDownloaderCore::DownloadImage(const GURL& image_url,
                                         bool is_favicon,
-                                        uint32_t max_bitmap_size,
                                         bool bypass_cache,
-                                        const DownloadImageCallback& callback) {
+                                        const DownloadCallback& callback) {
   std::vector<SkBitmap> result_images;
-  std::vector<gfx::Size> result_original_image_sizes;
 
   if (image_url.SchemeIs(url::kDataScheme)) {
     SkBitmap data_image = ImageFromDataUrl(image_url);
-    SkBitmap resized = ResizeImage(data_image, max_bitmap_size);
     // Drop null or empty SkBitmap.
-    if (!resized.drawsNothing()) {
-      result_images.push_back(resized);
-      result_original_image_sizes.push_back(
-          gfx::Size(data_image.width(), data_image.height()));
-    }
+    if (!data_image.drawsNothing())
+      result_images.push_back(data_image);
   } else {
-    if (FetchImage(image_url, is_favicon, max_bitmap_size, bypass_cache,
-                   callback)) {
-      // Will complete asynchronously via ImageDownloaderImpl::DidFetchImage
+    if (FetchImage(image_url, is_favicon, bypass_cache, callback)) {
+      // Will complete asynchronously via ImageDownloaderCore::DidFetchImage
       return;
     }
   }
 
-  ReplyDownloadResult(0, result_images, result_original_image_sizes, callback);
+  callback.Run(0, result_images);
 }
 
-bool ImageDownloaderImpl::FetchImage(const GURL& image_url,
+bool ImageDownloaderCore::FetchImage(const GURL& image_url,
                                      bool is_favicon,
-                                     uint32_t max_image_size,
                                      bool bypass_cache,
-                                     const DownloadImageCallback& callback) {
-  blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
+                                     const DownloadCallback& callback) {
+  blink::WebLocalFrame* frame = render_frame_->GetWebFrame();
   DCHECK(frame);
 
   // Create an image resource fetcher and assign it with a call back object.
@@ -200,23 +98,16 @@ bool ImageDownloaderImpl::FetchImage(const GURL& image_url,
                                       : WebURLRequest::RequestContextImage,
       bypass_cache ? WebCachePolicy::BypassingCache
                    : WebCachePolicy::UseProtocolCachePolicy,
-      base::Bind(&ImageDownloaderImpl::DidFetchImage, base::Unretained(this),
-                 max_image_size, callback)));
+      base::Bind(&ImageDownloaderCore::DidFetchImage, base::Unretained(this),
+                 callback)));
   return true;
 }
 
-void ImageDownloaderImpl::DidFetchImage(
-    uint32_t max_image_size,
-    const DownloadImageCallback& callback,
+void ImageDownloaderCore::DidFetchImage(
+    const DownloadCallback& callback,
     MultiResolutionImageResourceFetcher* fetcher,
     const std::vector<SkBitmap>& images) {
-  std::vector<SkBitmap> result_images;
-  std::vector<gfx::Size> result_original_image_sizes;
-  FilterAndResizeImagesForMaximalSize(images, max_image_size, &result_images,
-                                      &result_original_image_sizes);
-
-  ReplyDownloadResult(fetcher->http_status_code(), result_images,
-                      result_original_image_sizes, callback);
+  int32_t http_status_code = fetcher->http_status_code();
 
   // Remove the image fetcher from our pending list. We're in the callback from
   // MultiResolutionImageResourceFetcher, best to delay deletion.
@@ -226,18 +117,9 @@ void ImageDownloaderImpl::DidFetchImage(
     image_fetchers_.weak_erase(iter);
     base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, fetcher);
   }
-}
-
-void ImageDownloaderImpl::ReplyDownloadResult(
-    int32_t http_status_code,
-    const std::vector<SkBitmap>& result_images,
-    const std::vector<gfx::Size>& result_original_image_sizes,
-    const DownloadImageCallback& callback) {
-  callback.Run(http_status_code, result_images, result_original_image_sizes);
-}
 
-void ImageDownloaderImpl::OnDestruct() {
-  delete this;
+  // |this| may be destructed after callback is run.
+  callback.Run(http_status_code, images);
 }
 
 }  // namespace content