Avoid transcode by getting JPEG data from ThumbnailImage

This avoids re-encoding thumbnail images by requesting compressed image
data directly from ThumbnailImage.

Additionally, this changes the data URI generating code to reduce usage
of temporary std::strings.

Bug: 991393
Change-Id: Ia5cc679c383e5ef4a084c5b220492c824124b7d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1836146
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705166}

chrome/browser/ui/webui/tab_strip/tab_strip_ui.cc

@@ -11,6 +11,7 @@
 
 #include "base/base64.h"
 #include "base/bind.h"
+#include "base/strings/string_piece.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/profiles/profile.h"
@@ -45,8 +46,6 @@ namespace {
 
 // Writes bytes to a std::vector that can be fetched. This is used to record the
 // output of skia image encoding.
-//
-// TODO(crbug.com/991393): remove this when we no longer transcode images here.
 class BufferWStream : public SkWStream {
  public:
   BufferWStream() = default;
@@ -68,26 +67,23 @@ class BufferWStream : public SkWStream {
   std::vector<unsigned char> result_;
 };
 
-std::string EncodeImage(gfx::ImageSkia image,
-                        SkEncodedImageFormat format,
-                        float scale_factor) {
+std::string MakeDataURIForImage(base::span<const uint8_t> image_data,
+                                base::StringPiece mime_subtype) {
+  std::string result = "data:image/";
+  result.append(mime_subtype.begin(), mime_subtype.end());
+  result += ";base64,";
+  result += base::Base64Encode(image_data);
+  return result;
+}
+
+std::string EncodePNGAndMakeDataURI(gfx::ImageSkia image, float scale_factor) {
   const SkBitmap& bitmap = image.GetRepresentation(scale_factor).GetBitmap();
   BufferWStream stream;
-  const bool encoding_succeeded = SkEncodeImage(&stream, bitmap, format, 100);
+  const bool encoding_succeeded =
+      SkEncodeImage(&stream, bitmap, SkEncodedImageFormat::kPNG, 100);
   DCHECK(encoding_succeeded);
-  const std::vector<unsigned char> image_data = stream.GetBuffer();
-
-  std::string mime_subtype;
-  if (format == SkEncodedImageFormat::kJPEG) {
-    mime_subtype = "jpeg";
-  } else if (format == SkEncodedImageFormat::kPNG) {
-    mime_subtype = "png";
-  } else {
-    NOTREACHED();
-  }
-
-  return "data:image/" + mime_subtype + ";base64," +
-         base::Base64Encode(base::as_bytes(base::make_span(image_data)));
+  return MakeDataURIForImage(
+      base::as_bytes(base::make_span(stream.GetBuffer())), "png");
 }
 
 class WebUITabContextMenu : public ui::SimpleMenuModel::Delegate,
@@ -224,10 +220,9 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
     tab_data.SetString("url", tab_renderer_data.visible_url.GetContent());
 
     if (!tab_renderer_data.favicon.isNull()) {
-      tab_data.SetString(
-          "favIconUrl",
-          EncodeImage(tab_renderer_data.favicon, SkEncodedImageFormat::kPNG,
-                      web_ui()->GetDeviceScaleFactor()));
+      tab_data.SetString("favIconUrl", EncodePNGAndMakeDataURI(
+                                           tab_renderer_data.favicon,
+                                           web_ui()->GetDeviceScaleFactor()));
     }
 
     tab_data.SetInteger("networkState",
@@ -349,24 +344,15 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
 
   // Callback passed to |thumbnail_tracker_|. Called when a tab's thumbnail
   // changes, or when we start watching the tab.
-  void HandleThumbnailUpdate(content::WebContents* tab, gfx::ImageSkia image) {
+  void HandleThumbnailUpdate(content::WebContents* tab,
+                             ThumbnailTracker::CompressedThumbnailData image) {
     // Send base-64 encoded image to JS side.
-    //
-    // TODO(crbug.com/991393): streamline the process from tab capture to
-    // sending the image. Currently, a frame is captured, sent to
-    // ThumbnailImage, encoded to JPEG (w/ a copy), decoded to a raw bitmap
-    // (another copy), and sent to observers. Here, we then re-encode the image
-    // to a JPEG (another copy), encode to base64 (another copy), append the
-    // base64 header (another copy), and send it (another copy). This is 6
-    // copies of essentially the same image, and it is de-encoded and re-encoded
-    // to the same format. We can reduce the number of copies and avoid the
-    // redundant encoding.
-    std::string encoded_image = EncodeImage(image, SkEncodedImageFormat::kJPEG,
-                                            web_ui()->GetDeviceScaleFactor());
+    std::string data_uri =
+        MakeDataURIForImage(base::make_span(image->data), "jpeg");
 
     const int tab_id = extensions::ExtensionTabUtil::GetTabId(tab);
     FireWebUIListener("tab-thumbnail-updated", base::Value(tab_id),
-                      base::Value(encoded_image));
+                      base::Value(data_uri));
   }
 
   Browser* const browser_;

chrome/browser/ui/webui/tab_strip/tab_strip_ui.h

@@ -45,7 +45,8 @@ class TabStripUI : public content::WebUIController {
   void Initialize(Browser* browser, Embedder* embedder);
 
  private:
-  void HandleThumbnailUpdate(int extension_tab_id, gfx::ImageSkia image);
+  void HandleThumbnailUpdate(int extension_tab_id,
+                             ThumbnailTracker::CompressedThumbnailData image);
 
   DISALLOW_COPY_AND_ASSIGN(TabStripUI);
 };

chrome/browser/ui/webui/tab_strip/thumbnail_tracker.cc

@@ -10,7 +10,6 @@
 #include "base/macros.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/thumbnails/thumbnail_image.h"
 #include "chrome/browser/ui/thumbnails/thumbnail_tab_helper.h"
 #include "content/public/browser/web_contents_observer.h"
 
@@ -28,7 +27,7 @@ class ThumbnailTracker::ContentsData : public content::WebContentsObserver,
 
   void RequestThumbnail() {
     if (thumbnail_)
-      thumbnail_->RequestThumbnailImage();
+      thumbnail_->RequestCompressedThumbnailData();
   }
 
   // content::WebContents:
@@ -45,7 +44,8 @@ class ThumbnailTracker::ContentsData : public content::WebContentsObserver,
   }
 
   // ThumbnailImage::Observer:
-  void OnThumbnailImageAvailable(gfx::ImageSkia thumbnail_image) override {
+  void OnCompressedThumbnailDataAvailable(
+      CompressedThumbnailData thumbnail_image) override {
     parent_->ThumbnailUpdated(web_contents(), thumbnail_image);
   }
 
@@ -79,7 +79,7 @@ void ThumbnailTracker::WatchTab(content::WebContents* contents) {
 }
 
 void ThumbnailTracker::ThumbnailUpdated(content::WebContents* contents,
-                                        gfx::ImageSkia image) {
+                                        CompressedThumbnailData image) {
   callback_.Run(contents, image);
 }
 

chrome/browser/ui/webui/tab_strip/thumbnail_tracker.h

@@ -9,19 +9,20 @@
 
 #include "base/containers/flat_map.h"
 #include "base/memory/scoped_refptr.h"
+#include "chrome/browser/ui/thumbnails/thumbnail_image.h"
 #include "ui/gfx/image/image_skia.h"
 
 namespace content {
 class WebContents;
 }
 
-class ThumbnailImage;
-
 // Tracks the thumbnails of a set of WebContentses. This set is dynamically
 // managed, and WebContents closure is handled gracefully. The user is notified
 // of any thumbnail change via a callback specified on construction.
 class ThumbnailTracker {
  public:
+  using CompressedThumbnailData = ThumbnailImage::CompressedThumbnailData;
+
   // Should return the ThumbnailImage instance for a WebContents.
   using GetThumbnailCallback =
       base::RepeatingCallback<scoped_refptr<ThumbnailImage>(
@@ -29,7 +30,8 @@ class ThumbnailTracker {
 
   // Handles a thumbnail update for a tab.
   using ThumbnailUpdatedCallback =
-      base::RepeatingCallback<void(content::WebContents*, gfx::ImageSkia)>;
+      base::RepeatingCallback<void(content::WebContents*,
+                                   CompressedThumbnailData)>;
 
   explicit ThumbnailTracker(ThumbnailUpdatedCallback callback);
   // Specifies how to get a ThumbnailImage for a WebContents. This is intended
@@ -44,7 +46,8 @@ class ThumbnailTracker {
   void WatchTab(content::WebContents* contents);
 
  private:
-  void ThumbnailUpdated(content::WebContents* contents, gfx::ImageSkia image);
+  void ThumbnailUpdated(content::WebContents* contents,
+                        CompressedThumbnailData image);
   void ContentsClosed(content::WebContents* contents);
 
   // The default |GetThumbnailCallback| implementation.

chrome/browser/ui/webui/tab_strip/thumbnail_tracker_unittest.cc

@@ -99,11 +99,9 @@ TEST_F(ThumbnailTrackerTest, WatchTabGetsCurrentThumbnail) {
   // Verify that WatchTab() gets the current image, waiting for the decoding to
   // happen.
   EXPECT_CALL(thumbnail_updated_callback_, Run(contents.get(), _)).Times(1);
-  base::RunLoop notify_loop;
   thumbnail->set_async_operation_finished_callback_for_testing(
-      notify_loop.QuitClosure());
+      base::RepeatingClosure());
   thumbnail_tracker_.WatchTab(contents.get());
-  notify_loop.Run();
 }
 
 TEST_F(ThumbnailTrackerTest, PropagatesThumbnailUpdate) {