lacros: Rename crosapi::WindowSnapshot to crosapi::Bitmap

The structure is just a representation of a bitmap. I'd like to use it
in another API for an icon.

* Extract the mojom struct into its own file
* Rename the C++ class and mojo traits serialization helpers
* Rename the |bitmap| member to |pixels| so we don't have to type
  bitmap.bitmap

None of the implementation changed. Gerrit shows file add/deletes
because the renames touched most lines.

Bug: 1113889
Change-Id: Ie0d9aa30d913089adab52d0f4bd3ac050575e96c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352645Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Auto-Submit: James Cook <jamescook@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797510}

chrome/browser/chromeos/crosapi/screen_manager_crosapi.cc

@@ -13,7 +13,7 @@
 #include "base/numerics/checked_math.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chromeos/crosapi/cpp/window_snapshot.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
 #include "ui/snapshot/snapshot.h"
 
 ScreenManagerCrosapi::ScreenManagerCrosapi() = default;
@@ -93,7 +93,7 @@ void ScreenManagerCrosapi::TakeWindowSnapshot(
     TakeWindowSnapshotCallback callback) {
   auto it = id_to_window_.find(id);
   if (it == id_to_window_.end()) {
-    std::move(callback).Run(/*success=*/false, crosapi::WindowSnapshot());
+    std::move(callback).Run(/*success=*/false, crosapi::Bitmap());
     return;
   }
 
@@ -132,9 +132,9 @@ void ScreenManagerCrosapi::DidTakeSnapshot(SnapshotCallback callback,
   uint8_t* base = static_cast<uint8_t*>(bitmap.getPixels());
   std::vector<uint8_t> bytes(base, base + bitmap.computeByteSize());
 
-  crosapi::WindowSnapshot snapshot;
+  crosapi::Bitmap snapshot;
   snapshot.width = bitmap.width();
   snapshot.height = bitmap.height();
-  snapshot.bitmap.swap(bytes);
+  snapshot.pixels.swap(bytes);
   std::move(callback).Run(std::move(snapshot));
 }

chrome/browser/chromeos/crosapi/screen_manager_crosapi.h

@@ -16,6 +16,10 @@
 #include "ui/aura/window_observer.h"
 #include "ui/gfx/image/image.h"
 
+namespace crosapi {
+struct Bitmap;
+}  // namespace crosapi
+
 // This class is the ash-chrome implementation of the ScreenManager interface.
 // This class must only be used from the main thread.
 class ScreenManagerCrosapi : public crosapi::mojom::ScreenManager,
@@ -42,8 +46,7 @@ class ScreenManagerCrosapi : public crosapi::mojom::ScreenManager,
   void OnWindowDestroying(aura::Window* window) final;
 
  private:
-  using SnapshotCallback =
-      base::OnceCallback<void(const crosapi::WindowSnapshot&)>;
+  using SnapshotCallback = base::OnceCallback<void(const crosapi::Bitmap&)>;
   void DidTakeSnapshot(SnapshotCallback callback, gfx::Image image);
 
   // This class generates unique, non-reused IDs for windows on demand. The IDs

chrome/browser/chromeos/crosapi/screen_manager_crosapi_browsertest.cc

@@ -11,7 +11,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/test/base/in_process_browser_test.h"
-#include "chromeos/crosapi/cpp/window_snapshot.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
 #include "chromeos/crosapi/mojom/screen_manager.mojom.h"
 #include "content/public/test/browser_test.h"
 #include "mojo/public/cpp/bindings/remote.h"
@@ -82,12 +82,12 @@ class ScreenManagerCrosapiBrowserTest : public InProcessBrowserTest {
 // Tests that taking a screen snapshot works.
 IN_PROC_BROWSER_TEST_F(ScreenManagerCrosapiBrowserTest, TakeScreenSnapshot) {
   base::RunLoop run_loop;
-  crosapi::WindowSnapshot snapshot;
+  crosapi::Bitmap snapshot;
 
   // Take a snapshot on a background sequence. The call is blocking, so when it
   // finishes, we can also unblock the main thread.
   auto take_snapshot_background = base::BindOnce(
-      [](SMRemote* remote, crosapi::WindowSnapshot* snapshot) {
+      [](SMRemote* remote, crosapi::Bitmap* snapshot) {
         mojo::ScopedAllowSyncCallForTesting allow_sync;
         (*remote)->TakeScreenSnapshot(snapshot);
       },
@@ -106,12 +106,12 @@ IN_PROC_BROWSER_TEST_F(ScreenManagerCrosapiBrowserTest, TakeScreenSnapshot) {
 IN_PROC_BROWSER_TEST_F(ScreenManagerCrosapiBrowserTest, TakeWindowSnapshot) {
   base::RunLoop run_loop;
   bool success;
-  crosapi::WindowSnapshot snapshot;
+  crosapi::Bitmap snapshot;
 
   // Take a snapshot on a background sequence. The call is blocking, so when it
   // finishes, we can also unblock the main thread.
   auto take_snapshot_background = base::BindOnce(
-      [](SMRemote* remote, bool* success, crosapi::WindowSnapshot* snapshot) {
+      [](SMRemote* remote, bool* success, crosapi::Bitmap* snapshot) {
         mojo::ScopedAllowSyncCallForTesting allow_sync;
         std::vector<crosapi::mojom::WindowDetailsPtr> windows;
         (*remote)->ListWindows(&windows);

chromeos/crosapi/cpp/BUILD.gn

@@ -10,8 +10,8 @@ config("crosapi_implementation") {
 component("cpp") {
   output_name = "crosapi_public_cpp"
   sources = [
-    "window_snapshot.cc",
-    "window_snapshot.h",
+    "bitmap.cc",
+    "bitmap.h",
   ]
   configs += [ ":crosapi_implementation" ]
   deps = [ "//base" ]

chromeos/crosapi/cpp/window_snapshot.cc → chromeos/crosapi/cpp/bitmap.cc

@@ -2,11 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chromeos/crosapi/cpp/window_snapshot.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
 
 namespace crosapi {
 
-WindowSnapshot::WindowSnapshot() = default;
-WindowSnapshot::~WindowSnapshot() = default;
+Bitmap::Bitmap() = default;
+Bitmap::~Bitmap() = default;
 
 }  // namespace crosapi

chromeos/crosapi/cpp/window_snapshot.h → chromeos/crosapi/cpp/bitmap.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROMEOS_CROSAPI_CPP_WINDOW_SNAPSHOT_H_
-#define CHROMEOS_CROSAPI_CPP_WINDOW_SNAPSHOT_H_
+#ifndef CHROMEOS_CROSAPI_CPP_BITMAP_H_
+#define CHROMEOS_CROSAPI_CPP_BITMAP_H_
 
 #include <stdint.h>
 
@@ -13,16 +13,16 @@
 
 namespace crosapi {
 
-// bitmap is a 4-byte RGBA bitmap representation of the window. Its size must
-// be exactly equal to width * height * 4.
-struct COMPONENT_EXPORT(CROSAPI) WindowSnapshot {
-  WindowSnapshot();
-  ~WindowSnapshot();
+// A 4-byte RGBA bitmap representation. Its size must be exactly equal to
+// width * height * 4.
+struct COMPONENT_EXPORT(CROSAPI) Bitmap {
+  Bitmap();
+  ~Bitmap();
   uint32_t width = 0;
   uint32_t height = 0;
-  std::vector<uint8_t> bitmap;
+  std::vector<uint8_t> pixels;
 };
 
 }  // namespace crosapi
 
-#endif  // CHROMEOS_CROSAPI_CPP_WINDOW_SNAPSHOT_H_
+#endif  // CHROMEOS_CROSAPI_CPP_BITMAP_H_

chromeos/crosapi/mojom/BUILD.gn

@@ -7,6 +7,7 @@ import("//mojo/public/tools/bindings/mojom.gni")
 mojom("mojom") {
   sources = [
     "attestation.mojom",
+    "bitmap.mojom",
     "crosapi.mojom",
     "screen_manager.mojom",
     "select_file.mojom",
@@ -16,12 +17,11 @@ mojom("mojom") {
     {
       types = [
         {
-          mojom = "crosapi.mojom.WindowSnapshot"
-          cpp = "crosapi::WindowSnapshot"
+          mojom = "crosapi.mojom.Bitmap"
+          cpp = "crosapi::Bitmap"
         },
       ]
-      traits_headers =
-          [ "//chromeos/crosapi/mojom/window_snapshot_mojom_traits.h" ]
+      traits_headers = [ "//chromeos/crosapi/mojom/bitmap_mojom_traits.h" ]
       traits_public_deps = [
         ":mojom_traits",
         "//chromeos/crosapi/cpp",
@@ -39,8 +39,8 @@ component("mojom_traits") {
   output_name = "crosapi_mojom_traits"
 
   sources = [
-    "window_snapshot_mojom_traits.cc",
-    "window_snapshot_mojom_traits.h",
+    "bitmap_mojom_traits.cc",
+    "bitmap_mojom_traits.h",
   ]
 
   defines = [ "IS_CROSAPI_MOJOM_TRAITS_IMPL" ]

chromeos/crosapi/mojom/bitmap.mojom

+// Copyright 2020 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.
+
+module crosapi.mojom;
+
+// Each pixel is represented by 4 bytes [RGBA].
+// TODO(https://crbug.com/1094460): Use a more performant transport mechanism.
+struct Bitmap {
+  uint32 width;
+  uint32 height;
+  array<uint8> pixels;
+};

chromeos/crosapi/mojom/window_snapshot_mojom_traits.cc → chromeos/crosapi/mojom/bitmap_mojom_traits.cc

@@ -2,32 +2,31 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chromeos/crosapi/mojom/window_snapshot_mojom_traits.h"
+#include "chromeos/crosapi/mojom/bitmap_mojom_traits.h"
 
 #include "base/numerics/checked_math.h"
 
 namespace mojo {
 
 // static
-bool StructTraits<
-    crosapi::mojom::WindowSnapshotDataView,
-    crosapi::WindowSnapshot>::Read(crosapi::mojom::WindowSnapshotDataView data,
-                                   crosapi::WindowSnapshot* out) {
+bool StructTraits<crosapi::mojom::BitmapDataView, crosapi::Bitmap>::Read(
+    crosapi::mojom::BitmapDataView data,
+    crosapi::Bitmap* out) {
   out->width = data.width();
   out->height = data.height();
 
-  ArrayDataView<uint8_t> bitmap;
-  data.GetBitmapDataView(&bitmap);
+  ArrayDataView<uint8_t> pixels;
+  data.GetPixelsDataView(&pixels);
 
   uint32_t size;
   size = base::CheckMul(out->width, out->height).ValueOrDie();
   size = base::CheckMul(size, 4).ValueOrDie();
 
-  if (bitmap.size() != base::checked_cast<size_t>(size))
+  if (pixels.size() != base::checked_cast<size_t>(size))
     return false;
 
-  const uint8_t* base = bitmap.data();
-  out->bitmap.assign(base, base + bitmap.size());
+  const uint8_t* base = pixels.data();
+  out->pixels.assign(base, base + pixels.size());
   return true;
 }
 

chromeos/crosapi/mojom/window_snapshot_mojom_traits.h → chromeos/crosapi/mojom/bitmap_mojom_traits.h

@@ -2,34 +2,29 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROMEOS_CROSAPI_MOJOM_WINDOW_SNAPSHOT_MOJOM_TRAITS_H_
-#define CHROMEOS_CROSAPI_MOJOM_WINDOW_SNAPSHOT_MOJOM_TRAITS_H_
+#ifndef CHROMEOS_CROSAPI_MOJOM_BITMAP_MOJOM_TRAITS_H_
+#define CHROMEOS_CROSAPI_MOJOM_BITMAP_MOJOM_TRAITS_H_
 
 #include "base/component_export.h"
-#include "chromeos/crosapi/cpp/window_snapshot.h"
-#include "chromeos/crosapi/mojom/screen_manager.mojom-shared.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
+#include "chromeos/crosapi/mojom/bitmap.mojom-shared.h"
 #include "mojo/public/cpp/bindings/struct_traits.h"
 
 namespace mojo {
 
 template <>
 struct COMPONENT_EXPORT(CROSAPI_MOJOM_TRAITS)
-    StructTraits<crosapi::mojom::WindowSnapshotDataView,
-                 crosapi::WindowSnapshot> {
-  static uint32_t width(const crosapi::WindowSnapshot& snapshot) {
-    return snapshot.width;
+    StructTraits<crosapi::mojom::BitmapDataView, crosapi::Bitmap> {
+  static uint32_t width(const crosapi::Bitmap& bitmap) { return bitmap.width; }
+  static uint32_t height(const crosapi::Bitmap& bitmap) {
+    return bitmap.height;
   }
-  static uint32_t height(const crosapi::WindowSnapshot& snapshot) {
-    return snapshot.height;
+  static const std::vector<uint8_t>& pixels(const crosapi::Bitmap& bitmap) {
+    return bitmap.pixels;
   }
-  static const std::vector<uint8_t>& bitmap(
-      const crosapi::WindowSnapshot& snapshot) {
-    return snapshot.bitmap;
-  }
-  static bool Read(crosapi::mojom::WindowSnapshotDataView,
-                   crosapi::WindowSnapshot* out);
+  static bool Read(crosapi::mojom::BitmapDataView, crosapi::Bitmap* out);
 };
 
 }  // namespace mojo
 
-#endif  // CHROMEOS_CROSAPI_MOJOM_WINDOW_SNAPSHOT_MOJOM_TRAITS_H_
+#endif  // CHROMEOS_CROSAPI_MOJOM_BITMAP_MOJOM_TRAITS_H_

chromeos/crosapi/mojom/screen_manager.mojom

@@ -4,6 +4,8 @@
 
 module crosapi.mojom;
 
+import "chromeos/crosapi/mojom/bitmap.mojom";
+
 // A unique identifier and title for a window.
 struct WindowDetails {
   // Guaranteed to be unique and never reused.
@@ -13,14 +15,6 @@ struct WindowDetails {
   string title;
 };
 
-// A bitmap representation of the current contents of a window or screen. Each
-// pixel is represented by 4 bytes [RGBA].
-struct WindowSnapshot {
-  uint32 width;
-  uint32 height;
-  array<uint8> bitmap;
-};
-
 // This interface is implemented by ash-chrome. It allows lacros-chrome to query
 // information about existing windows, screens, and displays.
 //
@@ -45,7 +39,7 @@ interface ScreenManager {
   // The media screen capture implementation assumes that every platform
   // provides a synchronous method to take a snapshot of the screen.
   [Sync]
-  TakeScreenSnapshot@0() => (WindowSnapshot snapshot);
+  TakeScreenSnapshot@0() => (Bitmap snapshot);
 
   // Synchronously returns a list of visible, focusable windows. This method
   // needs to be synchronous to support cross-platform code that relies on the
@@ -56,6 +50,5 @@ interface ScreenManager {
   // Take a screenshot of a window with an id from ListWindows. If |success|
   // is false, then the window no longer exists.
   [Sync]
-  TakeWindowSnapshot@2(uint64 id) => (bool success, WindowSnapshot snapshot);
+  TakeWindowSnapshot@2(uint64 id) => (bool success, Bitmap snapshot);
 };
-

content/browser/media/capture/desktop_capturer_lacros.cc

@@ -7,7 +7,7 @@
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
 
-#include "chromeos/crosapi/cpp/window_snapshot.h"
+#include "chromeos/crosapi/cpp/bitmap.h"
 #include "chromeos/lacros/lacros_chrome_service_impl.h"
 
 namespace content {
@@ -75,7 +75,7 @@ void DesktopCapturerLacros::Start(Callback* callback) {
 
 void DesktopCapturerLacros::CaptureFrame() {
   if (capture_type_ == kScreen) {
-    crosapi::WindowSnapshot snapshot;
+    crosapi::Bitmap snapshot;
     {
       // lacros-chrome is allowed to make sync calls to ash-chrome.
       mojo::SyncCallRestrictions::ScopedAllowSyncCall allow_sync_call;
@@ -84,7 +84,7 @@ void DesktopCapturerLacros::CaptureFrame() {
     DidTakeSnapshot(/*success=*/true, snapshot);
   } else {
     bool success;
-    crosapi::WindowSnapshot snapshot;
+    crosapi::Bitmap snapshot;
     {
       // lacros-chrome is allowed to make sync calls to ash-chrome.
       mojo::SyncCallRestrictions::ScopedAllowSyncCall allow_sync_call;
@@ -104,9 +104,8 @@ void DesktopCapturerLacros::SetSharedMemoryFactory(
 
 void DesktopCapturerLacros::SetExcludedWindow(webrtc::WindowId window) {}
 
-void DesktopCapturerLacros::DidTakeSnapshot(
-    bool success,
-    const crosapi::WindowSnapshot& snapshot) {
+void DesktopCapturerLacros::DidTakeSnapshot(bool success,
+                                            const crosapi::Bitmap& snapshot) {
   if (!success) {
     callback_->OnCaptureResult(Result::ERROR_PERMANENT,
                                std::unique_ptr<webrtc::DesktopFrame>());
@@ -120,7 +119,7 @@ void DesktopCapturerLacros::DidTakeSnapshot(
   // This code assumes that the stride is 4 * width. This relies on the
   // assumption that there's no padding and each pixel is 4 bytes.
   frame->CopyPixelsFrom(
-      snapshot.bitmap.data(), 4 * snapshot.width,
+      snapshot.pixels.data(), 4 * snapshot.width,
       webrtc::DesktopRect::MakeWH(snapshot.width, snapshot.height));
 
   callback_->OnCaptureResult(Result::SUCCESS, std::move(frame));

content/browser/media/capture/desktop_capturer_lacros.h

@@ -12,7 +12,7 @@
 #include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h"
 
 namespace crosapi {
-struct WindowSnapshot;
+struct Bitmap;
 }  // namespace crosapi
 
 namespace content {
@@ -46,7 +46,7 @@ class DesktopCapturerLacros : public webrtc::DesktopCapturer {
  private:
   // Callback for when ash-chrome returns a snapshot of the screen or window as
   // a bitmap.
-  void DidTakeSnapshot(bool success, const crosapi::WindowSnapshot& snapshot);
+  void DidTakeSnapshot(bool success, const crosapi::Bitmap& snapshot);
 
   // Whether this object is capturing screens or windows.
   const CaptureType capture_type_;