[devtools] Support different encodings for Page.CaptureScreenshot.

content/browser/renderer_host/render_widget_host_impl.cc

Index: content/browser/renderer_host/render_widget_host_impl.cc
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 8860791b51b005f240538873bcf57ad775c73536..a3cce57d500d7f4830b44c3c505700a965cdab24 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -89,7 +89,6 @@
 #include "ui/gfx/geometry/vector2d_conversions.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/skbitmap_operations.h"
-#include "ui/snapshot/snapshot.h"
 
 #if defined(OS_MACOSX)
 #include "device/power_save_blocker/power_save_blocker.h"
@@ -274,6 +273,7 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate,
       received_paint_after_load_(false),
       latency_tracker_(),
       next_browser_snapshot_id_(1),
+      pending_browser_snapshots_(0),
       owned_by_render_frame_host_(false),
       is_focused_(false),
       hung_renderer_delay_(
@@ -1430,14 +1430,16 @@ void RenderWidgetHostImpl::NotifyScreenInfoChanged() {
 }
 
 void RenderWidgetHostImpl::GetSnapshotFromBrowser(
-    const GetSnapshotFromBrowserCallback& callback) {
+    const GetSnapshotFromBrowserCallback& callback,
+    ui::SnapshotEncoding encoding,
+    ui::SnapshotQuality quality) {
   int id = next_browser_snapshot_id_++;
 
 #if defined(OS_MACOSX)
   // MacOS version of underlying GrabViewSnapshot() blocks while
   // display/GPU are in a power-saving mode, so make sure display
   // does not go to sleep for the duration of reading a snapshot.
-  if (pending_browser_snapshots_.empty()) {
+  if (pending_browser_snapshots_ == 0) {
     DCHECK(!power_save_blocker_);
     power_save_blocker_.reset(new device::PowerSaveBlocker(
         device::PowerSaveBlocker::kPowerSaveBlockPreventDisplaySleep,
@@ -1446,7 +1448,9 @@ void RenderWidgetHostImpl::GetSnapshotFromBrowser(
         BrowserThread::GetTaskRunnerForThread(BrowserThread::FILE)));
   }
 #endif
-  pending_browser_snapshots_.insert(std::make_pair(id, callback));
+  pending_browser_snapshot_params_.insert(
+      std::make_pair(id, PendingSnapshotParams(callback, encoding, quality)));
+  pending_browser_snapshots_++;
   ui::LatencyInfo latency_info;
   latency_info.AddLatencyNumber(ui::WINDOW_SNAPSHOT_FRAME_NUMBER_COMPONENT, 0,
                                 id);
@@ -2354,49 +2358,52 @@ void RenderWidgetHostImpl::WindowSnapshotReachedScreen(int snapshot_id) {
   gfx::Rect view_bounds = GetView()->GetViewBounds();
   gfx::Rect snapshot_bounds(view_bounds.size());
 
-  std::vector<unsigned char> png;
-  if (ui::GrabViewSnapshot(
-      GetView()->GetNativeView(), &png, snapshot_bounds)) {
-    OnSnapshotDataReceived(snapshot_id, &png.front(), png.size());
-    return;
-  }
-
-  ui::GrabViewSnapshotAsync(
-      GetView()->GetNativeView(),
-      snapshot_bounds,
-      base::ThreadTaskRunnerHandle::Get(),
-      base::Bind(&RenderWidgetHostImpl::OnSnapshotDataReceivedAsync,
-                 weak_factory_.GetWeakPtr(),
-                 snapshot_id));
-}
-
-void RenderWidgetHostImpl::OnSnapshotDataReceived(int snapshot_id,
-                                                  const unsigned char* data,
-                                                  size_t size) {
   // Any pending snapshots with a lower ID than the one received are considered
-  // to be implicitly complete, and returned the same snapshot data.
-  PendingSnapshotMap::iterator it = pending_browser_snapshots_.begin();
-  while (it != pending_browser_snapshots_.end()) {
-      if (it->first <= snapshot_id) {
-        it->second.Run(data, size);
-        pending_browser_snapshots_.erase(it++);
+  // to be implicitly complete. Grab snapshots in requested encodings.
+  PendingSnapshotParamsMap::iterator it =
+      pending_browser_snapshot_params_.begin();
+  while (it != pending_browser_snapshot_params_.end()) {
+    if (it->first <= snapshot_id) {
+      std::vector<unsigned char> data;
+      if (ui::GrabViewSnapshot(GetView()->GetNativeView(), &data,

[pfeldman, 2017/01/03 18:42:33]

Why don't we grab it once and then encode here?

[Eric Seckler, 2017/01/03 19:18:09]

What would that look like? I moved the encoding into GrabViewSnapshot since it
is done differently depending on the platform. Would we instead always encode
into PNG in GrabViewSnapshot and then decode and reencode here for JPEG?

I considered the overhead of that de+reencode to be more significant compared to
the (probably unlikely) case that we have multiple screenshots in the queue for
the same frame - is that a bad assumption? :)

[pfeldman, 2017/01/04 00:10:50]

There is nothing wrong with your assumption, but capturing multiple times in a
loop, while knowing that the result will be the same isn't pretty. MacOS is the
only place you need to work around. I was hoping you could do something like
NSImageToSkBitmapWithColorSpace and operate on SkBitmap from there.

[Eric Seckler, 2017/01/06 21:06:01]

I've changed ui::Grab*Snapshot* methods to return gfx::Images that are either
backed by an NSImage (mac) or an SkBitmap (other platforms), and am converting
from either of these two representations in PageHandler, using existing /
extended helper methods. Does this make more sense?

+                               snapshot_bounds, it->second.encoding,
+                               it->second.quality)) {
+        OnSnapshotDataReceived(it->second.callback, &data.front(), data.size());
       } else {
-        ++it;
+        ui::GrabViewSnapshotAsync(
+            GetView()->GetNativeView(), snapshot_bounds,
+            base::ThreadTaskRunnerHandle::Get(),
+            base::Bind(&RenderWidgetHostImpl::OnSnapshotDataReceivedAsync,
+                       weak_factory_.GetWeakPtr(), it->second.callback),
+            it->second.encoding, it->second.quality);
       }
+      pending_browser_snapshot_params_.erase(it++);
+    } else {
+      ++it;
+    }
   }
+}
+
+void RenderWidgetHostImpl::OnSnapshotDataReceived(
+    const GetSnapshotFromBrowserCallback& callback,
+    const unsigned char* data,
+    size_t size) {
+  DCHECK_LT(0, pending_browser_snapshots_);
+  callback.Run(data, size);
+  pending_browser_snapshots_--;
 #if defined(OS_MACOSX)
-  if (pending_browser_snapshots_.empty())
+  if (pending_browser_snapshots_ == 0)
     power_save_blocker_.reset();
 #endif
 }
 
 void RenderWidgetHostImpl::OnSnapshotDataReceivedAsync(
-    int snapshot_id,
+    const GetSnapshotFromBrowserCallback& callback,
     scoped_refptr<base::RefCountedBytes> png_data) {
   if (png_data.get())
-    OnSnapshotDataReceived(snapshot_id, png_data->front(), png_data->size());
+    OnSnapshotDataReceived(callback, png_data->front(), png_data->size());
   else
-    OnSnapshotDataReceived(snapshot_id, NULL, 0);
+    OnSnapshotDataReceived(callback, NULL, 0);
 }
 
 // static
@@ -2518,4 +2525,15 @@ void RenderWidgetHostImpl::GrantFileAccessFromDropData(DropData* drop_data) {
   }
 }
 
+RenderWidgetHostImpl::PendingSnapshotParams::PendingSnapshotParams(
+    const GetSnapshotFromBrowserCallback& callback,
+    ui::SnapshotEncoding encoding,
+    ui::SnapshotQuality quality)
+    : callback(callback), encoding(encoding), quality(quality) {}
+
+RenderWidgetHostImpl::PendingSnapshotParams::PendingSnapshotParams(
+    const PendingSnapshotParams& params) = default;
+
+RenderWidgetHostImpl::PendingSnapshotParams::~PendingSnapshotParams() = default;
+
 }  // namespace content