[Lacros] Improve desktop screen/window capture efficiency

Changes the interface to use SkBitmap, which is passed over Mojo IPC
using BigBuffer (shared memory). This is then wrapped using a special
subclass of webrtc::DesktopFrame (one less copy).

There are still more copies than would be desirable here. We have the
initial copy created by Ash. That is translated into a BigBuffer for
transport over Mojo. That is then copied back to a SkBitmap. Avoiding
those copies and even reusing the transport buffer are an exercise for
a later CL.

The interface design also unifies screen and window capture in a way
that is consistent with webrtc::DesktopCapturer. This also enables a
path to adding support for multiple screens in a future CL.

There's a good deal of complexity in this CL to handle version skew.
QueryVersion doesn't work synchronously, so a nested event loop is
used on the background thread where DesktopCapturerLacros runs. That
is safe, since this is a dedicated thread, but not ideal.

The deprecated methods are re-implemented in terms of the newer
interface.

Bug: 1094460
Change-Id: I2b2f86df08612b79faae7da547ff1242c05ac467
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2443404Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Darin Fisher <darin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814382}

chrome/browser/chromeos/crosapi/screen_manager_ash.h

@@ -7,22 +7,16 @@
 
 #include <stdint.h>
 
-#include <map>
-
-#include "base/memory/weak_ptr.h"
 #include "chromeos/crosapi/mojom/screen_manager.mojom.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
-#include "ui/aura/window_observer.h"
 #include "ui/gfx/image/image.h"
 
 namespace crosapi {
 
-struct Bitmap;
-
 // This class is the ash-chrome implementation of the ScreenManager interface.
 // This class must only be used from the main thread.
-class ScreenManagerAsh : public mojom::ScreenManager, aura::WindowObserver {
+class ScreenManagerAsh : public mojom::ScreenManager {
  public:
   ScreenManagerAsh();
   ScreenManagerAsh(const ScreenManagerAsh&) = delete;
@@ -32,38 +26,31 @@ class ScreenManagerAsh : public mojom::ScreenManager, aura::WindowObserver {
   void BindReceiver(mojo::PendingReceiver<mojom::ScreenManager> receiver);
 
   // crosapi::mojom::ScreenManager:
-  void TakeScreenSnapshot(TakeScreenSnapshotCallback callback) override;
-  void ListWindows(ListWindowsCallback callback) override;
-  void TakeWindowSnapshot(uint64_t id,
-                          TakeWindowSnapshotCallback callback) override;
-
-  // aura::WindowObserver
-  // This method is overridden purely to remove dead windows from
-  // |id_to_window_| and |window_to_id_|. This ensures that if the pointer is
-  // reused for a new window, it does not get confused with a previous window.
-  void OnWindowDestroying(aura::Window* window) final;
+  void DeprecatedTakeScreenSnapshot(
+      DeprecatedTakeScreenSnapshotCallback callback) override;
+  void DeprecatedListWindows(DeprecatedListWindowsCallback callback) override;
+  void DeprecatedTakeWindowSnapshot(
+      uint64_t id,
+      DeprecatedTakeWindowSnapshotCallback callback) override;
+  void GetScreenCapturer(
+      mojo::PendingReceiver<mojom::SnapshotCapturer> receiver) override;
+  void GetWindowCapturer(
+      mojo::PendingReceiver<mojom::SnapshotCapturer> receiver) override;
 
  private:
-  using SnapshotCallback = base::OnceCallback<void(Bitmap)>;
-  void DidTakeSnapshot(SnapshotCallback callback, gfx::Image image);
+  class ScreenCapturerImpl;
+  class WindowCapturerImpl;
 
-  // This class generates unique, non-reused IDs for windows on demand. The IDs
-  // are monotonically increasing 64-bit integers. Once an ID is assigned to a
-  // window, this class listens for the destruction of the window in order to
-  // remove dead windows from the map.
-  //
-  // The members |id_to_window_| and |window_to_id_| must be kept in sync. The
-  // members exist to allow fast lookup in both directions.
-  std::map<uint64_t, aura::Window*> id_to_window_;
-  std::map<aura::Window*, uint64_t> window_to_id_;
-  uint64_t next_window_id_ = 0;
+  ScreenCapturerImpl* GetScreenCapturerImpl();
+  WindowCapturerImpl* GetWindowCapturerImpl();
+
+  std::unique_ptr<ScreenCapturerImpl> screen_capturer_impl_;
+  std::unique_ptr<WindowCapturerImpl> window_capturer_impl_;
 
   // This class supports any number of connections. This allows the client to
   // have multiple, potentially thread-affine, remotes. This is needed by
   // WebRTC.
   mojo::ReceiverSet<mojom::ScreenManager> receivers_;
-
-  base::WeakPtrFactory<ScreenManagerAsh> weak_factory_{this};
 };
 
 }  // namespace crosapi

chrome/browser/chromeos/crosapi/screen_manager_ash_browsertest.cc

@@ -79,7 +79,8 @@ class ScreenManagerAshBrowserTest : public InProcessBrowserTest {
 };
 
 // Tests that taking a screen snapshot works.
-IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest, TakeScreenSnapshot) {
+IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest,
+                       DeprecatedTakeScreenSnapshot) {
   base::RunLoop run_loop;
   Bitmap snapshot;
 
@@ -88,7 +89,7 @@ IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest, TakeScreenSnapshot) {
   auto take_snapshot_background = base::BindOnce(
       [](SMRemote* remote, Bitmap* snapshot) {
         mojo::ScopedAllowSyncCallForTesting allow_sync;
-        (*remote)->TakeScreenSnapshot(snapshot);
+        (*remote)->DeprecatedTakeScreenSnapshot(snapshot);
       },
       screen_manager_remote_.get(), &snapshot);
   background_sequence_->PostTaskAndReply(
@@ -102,7 +103,8 @@ IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest, TakeScreenSnapshot) {
   EXPECT_EQ(int{snapshot.height}, primary_window->bounds().height());
 }
 
-IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest, TakeWindowSnapshot) {
+IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest,
+                       DeprecatedTakeWindowSnapshot) {
   base::RunLoop run_loop;
   bool success;
   Bitmap snapshot;
@@ -112,13 +114,14 @@ IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest, TakeWindowSnapshot) {
   auto take_snapshot_background = base::BindOnce(
       [](SMRemote* remote, bool* success, Bitmap* snapshot) {
         mojo::ScopedAllowSyncCallForTesting allow_sync;
-        std::vector<mojom::WindowDetailsPtr> windows;
-        (*remote)->ListWindows(&windows);
+        std::vector<mojom::SnapshotSourcePtr> windows;
+        (*remote)->DeprecatedListWindows(&windows);
 
         // There should be exactly 1 window.
         ASSERT_EQ(1u, windows.size());
 
-        (*remote)->TakeWindowSnapshot(windows[0]->id, success, snapshot);
+        (*remote)->DeprecatedTakeWindowSnapshot(windows[0]->id, success,
+                                                snapshot);
       },
       screen_manager_remote_.get(), &success, &snapshot);
   background_sequence_->PostTaskAndReply(
@@ -134,5 +137,82 @@ IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest, TakeWindowSnapshot) {
   EXPECT_EQ(int{snapshot.height}, window->bounds().height());
 }
 
+IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest, ScreenCapturer) {
+  base::RunLoop run_loop;
+  bool success;
+  SkBitmap 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, SkBitmap* snapshot) {
+        mojo::Remote<mojom::SnapshotCapturer> capturer;
+        (*remote)->GetScreenCapturer(capturer.BindNewPipeAndPassReceiver());
+
+        {
+          mojo::ScopedAllowSyncCallForTesting allow_sync;
+
+          std::vector<mojom::SnapshotSourcePtr> screens;
+          capturer->ListSources(&screens);
+
+          // There should be at least one screen!
+          ASSERT_LE(1u, screens.size());
+
+          capturer->TakeSnapshot(screens[0]->id, success, snapshot);
+        }
+      },
+      screen_manager_remote_.get(), &success, &snapshot);
+  background_sequence_->PostTaskAndReply(
+      FROM_HERE, std::move(take_snapshot_background), run_loop.QuitClosure());
+  run_loop.Run();
+
+  // Check that the IPC succeeded.
+  ASSERT_TRUE(success);
+
+  // Check that the screenshot has the right dimensions.
+  aura::Window* primary_window =
+      browser()->window()->GetNativeWindow()->GetRootWindow();
+  EXPECT_EQ(int{snapshot.width()}, primary_window->bounds().width());
+  EXPECT_EQ(int{snapshot.height()}, primary_window->bounds().height());
+}
+
+IN_PROC_BROWSER_TEST_F(ScreenManagerAshBrowserTest, WindowCapturer) {
+  base::RunLoop run_loop;
+  bool success;
+  SkBitmap 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, SkBitmap* snapshot) {
+        mojo::Remote<mojom::SnapshotCapturer> capturer;
+        (*remote)->GetWindowCapturer(capturer.BindNewPipeAndPassReceiver());
+
+        {
+          mojo::ScopedAllowSyncCallForTesting allow_sync;
+
+          std::vector<mojom::SnapshotSourcePtr> windows;
+          capturer->ListSources(&windows);
+
+          // There should be at least one window!
+          ASSERT_LE(1u, windows.size());
+
+          capturer->TakeSnapshot(windows[0]->id, success, snapshot);
+        }
+      },
+      screen_manager_remote_.get(), &success, &snapshot);
+  background_sequence_->PostTaskAndReply(
+      FROM_HERE, std::move(take_snapshot_background), run_loop.QuitClosure());
+  run_loop.Run();
+
+  // Check that the IPC succeeded.
+  ASSERT_TRUE(success);
+
+  // Check that the screenshot has the right dimensions.
+  aura::Window* window = browser()->window()->GetNativeWindow();
+  EXPECT_EQ(int{snapshot.width()}, window->bounds().width());
+  EXPECT_EQ(int{snapshot.height()}, window->bounds().height());
+}
+
 }  // namespace
 }  // namespace crosapi

chrome/browser/lacros/screen_manager_lacros_browsertest.cc

@@ -28,10 +28,47 @@
 
 namespace {
 
+const char* kLacrosPageTitle = "Title Of Lacros Browser Test";
 const char* kLacrosPageTitleHTML =
     "<html><head><title>Title Of Lacros Browser Test</title></head>"
     "<body>This page has a title.</body></html>";
 
+using ListWindowsCallback = base::RepeatingCallback<void(
+    std::vector<crosapi::mojom::SnapshotSourcePtr>*)>;
+
+// Used to find the window corresponding to the test page.
+bool FindTestWindow(ListWindowsCallback list_windows, uint64_t* window_id) {
+  base::RunLoop run_loop;
+  bool found_window = false;
+  auto look_for_window = base::BindRepeating(
+      [](ListWindowsCallback list_windows, base::RunLoop* run_loop,
+         bool* found_window, uint64_t* window_id) {
+        const base::string16 tab_title(base::ASCIIToUTF16(kLacrosPageTitle));
+        std::string expected_window_title = l10n_util::GetStringFUTF8(
+            IDS_BROWSER_WINDOW_TITLE_FORMAT, tab_title);
+        std::vector<crosapi::mojom::SnapshotSourcePtr> windows;
+        list_windows.Run(&windows);
+        for (auto& window : windows) {
+          if (window->title == expected_window_title) {
+            (*found_window) = true;
+            (*window_id) = window->id;
+            run_loop->Quit();
+            break;
+          }
+        }
+      },
+      std::move(list_windows), &run_loop, &found_window, window_id);
+
+  // When the browser test start, there is no guarantee that the window is
+  // open from ash's perspective.
+  base::RepeatingTimer timer;
+  timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(1),
+              std::move(look_for_window));
+  run_loop.Run();
+
+  return found_window;
+}
+
 }  // namespace
 
 class ScreenManagerLacrosBrowserTest : public InProcessBrowserTest {
@@ -46,26 +83,39 @@ class ScreenManagerLacrosBrowserTest : public InProcessBrowserTest {
   ~ScreenManagerLacrosBrowserTest() override = default;
 
   void BindScreenManager() {
-    mojo::PendingRemote<crosapi::mojom::ScreenManager> pending_screen_manager;
-    mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver =
-        pending_screen_manager.InitWithNewPipeAndPassReceiver();
     auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
     ASSERT_TRUE(lacros_chrome_service);
+
+    mojo::PendingRemote<crosapi::mojom::ScreenManager> pending_screen_manager;
     lacros_chrome_service->BindScreenManagerReceiver(
-        std::move(pending_receiver));
+        pending_screen_manager.InitWithNewPipeAndPassReceiver());
+
     screen_manager_.Bind(std::move(pending_screen_manager));
   }
 
+  uint32_t QueryScreenManagerVersion() {
+    // Synchronously fetch the version of the ScreenManager interface. The
+    // version field will be available as |screen_manager_.version()|.
+    base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+    screen_manager_.QueryVersion(
+        base::BindOnce([](base::OnceClosure done_closure,
+                          uint32_t version) { std::move(done_closure).Run(); },
+                       run_loop.QuitClosure()));
+    run_loop.Run();
+    return screen_manager_.version();
+  }
+
   mojo::Remote<crosapi::mojom::ScreenManager> screen_manager_;
 };
 
 // Tests that taking a screen snapshot via crosapi works.
-IN_PROC_BROWSER_TEST_F(ScreenManagerLacrosBrowserTest, TakeScreenSnapshot) {
+IN_PROC_BROWSER_TEST_F(ScreenManagerLacrosBrowserTest,
+                       DeprecatedTakeScreenSnapshot) {
   BindScreenManager();
   crosapi::Bitmap snapshot;
   {
     mojo::ScopedAllowSyncCallForTesting allow_sync_call;
-    screen_manager_->TakeScreenSnapshot(&snapshot);
+    screen_manager_->DeprecatedTakeScreenSnapshot(&snapshot);
   }
   // Verify the snapshot is non-empty.
   EXPECT_GT(snapshot.height, 0u);
@@ -76,51 +126,34 @@ IN_PROC_BROWSER_TEST_F(ScreenManagerLacrosBrowserTest, TakeScreenSnapshot) {
 // Tests that taking a screen snapshot via crosapi works.
 // This test makes the browser load a page with specific title, and then scans
 // through a list of windows to look for the window with the expected title.
-// This test cannot simply asserts exactly 1 window is present because currently
+// This test cannot simply assert exactly 1 window is present because currently
 // in lacros_chrome_browsertests, different browser tests share the same
 // ash-chrome, so a window could come from any one of them.
-IN_PROC_BROWSER_TEST_F(ScreenManagerLacrosBrowserTest, TakeWindowSnapshot) {
+IN_PROC_BROWSER_TEST_F(ScreenManagerLacrosBrowserTest,
+                       DeprecatedTakeWindowSnapshot) {
   GURL url(std::string("data:text/html,") + kLacrosPageTitleHTML);
   ui_test_utils::NavigateToURL(browser(), url);
 
   BindScreenManager();
-  base::RunLoop run_loop;
-  bool found_window = false;
-  uint64_t window_id;
-  auto look_for_window = base::BindRepeating(
+
+  auto list_windows = base::BindRepeating(
       [](mojo::Remote<crosapi::mojom::ScreenManager>* screen_manager,
-         base::RunLoop* run_loop, bool* found_window, uint64_t* window_id) {
+         std::vector<crosapi::mojom::SnapshotSourcePtr>* windows) {
         mojo::ScopedAllowSyncCallForTesting allow_sync_call;
-        const base::string16 tab_title(
-            base::ASCIIToUTF16("Title Of Lacros Browser Test"));
-        std::string expected_window_title = l10n_util::GetStringFUTF8(
-            IDS_BROWSER_WINDOW_TITLE_FORMAT, tab_title);
-
-        std::vector<crosapi::mojom::WindowDetailsPtr> windows;
-        (*screen_manager)->ListWindows(&windows);
-        for (auto& window_details : windows) {
-          if (window_details->title == expected_window_title) {
-            (*found_window) = true;
-            (*window_id) = window_details->id;
-            run_loop->Quit();
-            break;
-          }
-        }
+        (*screen_manager)->DeprecatedListWindows(windows);
       },
-      &screen_manager_, &run_loop, &found_window, &window_id);
-  // When the browser test start, there is no guaranteen that the window is
-  // open from ash's perspective.
-  base::RepeatingTimer timer;
-  timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(1),
-              std::move(look_for_window));
-  run_loop.Run();
+      &screen_manager_);
+
+  uint64_t window_id;
+  bool found_window = FindTestWindow(std::move(list_windows), &window_id);
   ASSERT_TRUE(found_window);
 
   bool success = false;
   crosapi::Bitmap snapshot;
   {
     mojo::ScopedAllowSyncCallForTesting allow_sync_call;
-    screen_manager_->TakeWindowSnapshot(window_id, &success, &snapshot);
+    screen_manager_->DeprecatedTakeWindowSnapshot(window_id, &success,
+                                                  &snapshot);
   }
   ASSERT_TRUE(success);
   // Verify the snapshot is non-empty.
@@ -128,3 +161,79 @@ IN_PROC_BROWSER_TEST_F(ScreenManagerLacrosBrowserTest, TakeWindowSnapshot) {
   EXPECT_GT(snapshot.width, 0u);
   EXPECT_GT(snapshot.pixels.size(), 0u);
 }
+
+// Tests that taking a screen snapshot via crosapi works.
+IN_PROC_BROWSER_TEST_F(ScreenManagerLacrosBrowserTest, ScreenCapturer) {
+  BindScreenManager();
+
+  if (QueryScreenManagerVersion() < 1) {
+    LOG(WARNING) << "Ash version does not support required method.";
+    return;
+  }
+
+  mojo::Remote<crosapi::mojom::SnapshotCapturer> capturer;
+  screen_manager_->GetScreenCapturer(capturer.BindNewPipeAndPassReceiver());
+
+  std::vector<crosapi::mojom::SnapshotSourcePtr> screens;
+  {
+    mojo::ScopedAllowSyncCallForTesting allow_sync_call;
+    capturer->ListSources(&screens);
+  }
+  ASSERT_LE(1u, screens.size());
+
+  bool success = false;
+  SkBitmap snapshot;
+  {
+    mojo::ScopedAllowSyncCallForTesting allow_sync_call;
+    capturer->TakeSnapshot(screens[0]->id, &success, &snapshot);
+  }
+  EXPECT_TRUE(success);
+  // Verify the snapshot is non-empty.
+  EXPECT_GT(snapshot.height(), 0);
+  EXPECT_GT(snapshot.width(), 0);
+}
+
+// Tests that taking a window snapshot via crosapi works.
+// This test makes the browser load a page with specific title, and then scans
+// through a list of windows to look for the window with the expected title.
+// This test cannot simply asserts exactly 1 window is present because currently
+// in lacros_chrome_browsertests, different browser tests share the same
+// ash-chrome, so a window could come from any one of them.
+IN_PROC_BROWSER_TEST_F(ScreenManagerLacrosBrowserTest, WindowCapturer) {
+  BindScreenManager();
+
+  if (QueryScreenManagerVersion() < 1) {
+    LOG(WARNING) << "Ash version does not support required method.";
+    return;
+  }
+
+  GURL url(std::string("data:text/html,") + kLacrosPageTitleHTML);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  mojo::Remote<crosapi::mojom::SnapshotCapturer> capturer;
+  screen_manager_->GetWindowCapturer(capturer.BindNewPipeAndPassReceiver());
+
+  auto list_windows = base::BindRepeating(
+      [](mojo::Remote<crosapi::mojom::SnapshotCapturer>* capturer,
+         std::vector<crosapi::mojom::SnapshotSourcePtr>* windows) {
+        mojo::ScopedAllowSyncCallForTesting allow_sync_call;
+        (*capturer)->ListSources(windows);
+      },
+      &capturer);
+
+  uint64_t window_id;
+  bool found_window = FindTestWindow(std::move(list_windows), &window_id);
+
+  ASSERT_TRUE(found_window);
+
+  bool success = false;
+  SkBitmap snapshot;
+  {
+    mojo::ScopedAllowSyncCallForTesting allow_sync_call;
+    capturer->TakeSnapshot(window_id, &success, &snapshot);
+  }
+  ASSERT_TRUE(success);
+  // Verify the snapshot is non-empty.
+  EXPECT_GT(snapshot.height(), 0);
+  EXPECT_GT(snapshot.width(), 0);
+}

chromeos/crosapi/mojom/screen_manager.mojom

@@ -5,17 +5,32 @@
 module crosapi.mojom;
 
 import "chromeos/crosapi/mojom/bitmap.mojom";
+import "skia/public/mojom/bitmap.mojom";
 
-// A unique identifier and title for a window.
-[Stable]
-struct WindowDetails {
+// A unique identifier and title for a window (or screen).
+[Stable, RenamedFrom="crosapi.mojom.WindowDetails"]
+struct SnapshotSource {
   // Guaranteed to be unique and never reused.
   uint64 id@0;
 
-  // The title of the window in UTF-8 encoding.
+  // The user-visible name of the window or screen in UTF-8 encoding.
   string title@1;
 };
 
+// This interface is used to capture bitmap snapshots of screens or windows.
+// See ScreenManager methods used to create instances of this interface.
+[Stable]
+interface SnapshotCapturer {
+  // Returns list of screens or windows that can be captured.
+  [Sync]
+  ListSources@0() => (array<SnapshotSource> sources);
+
+  // Captures a bitmap snapshot of the specified screen or window. If |success|
+  // is false, that may indicate that the specified source no longer exists.
+  [Sync]
+  TakeSnapshot@1(uint64 id) => (bool success, skia.mojom.Bitmap? snapshot);
+};
+
 // This interface is implemented by ash-chrome. It allows lacros-chrome to query
 // information about existing windows, screens, and displays.
 //
@@ -29,11 +44,8 @@ struct WindowDetails {
 // interface in the future.
 [Stable]
 interface ScreenManager {
-  // TODO(https://crbug.com/1094460): We will need to add more methods for
-  // querying screens, windows, etc. Details still TBD.
-
-  // TODO(https://crbug.com/1094460): Use a more performant transport
-  // mechanism.
+  // DEPRECATED. Use |GetScreenCapturer| instead.
+  //
   // This method assumes that there's exactly one screen. The implementation
   // of this method takes a screenshot and converts it into a bitmap. Each
   // pixel is represented by 4 bytes [RGBA].
@@ -41,16 +53,26 @@ 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() => (Bitmap snapshot);
+  DeprecatedTakeScreenSnapshot@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
-  // assumption that every OS provides a similar synchronous IPC.
+  // DEPRECATED. Use |GetWindowCapturer| instead.
+  //
+  // Returns a list of visible, focusable top-level windows.
   [Sync]
-  ListWindows@1() => (array<WindowDetails> windows);
+  DeprecatedListWindows@1() => (array<SnapshotSource> windows);
 
+  // DEPRECATED. Use |GetWindowCapturer| instead.
+  //
   // 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, Bitmap snapshot);
+  DeprecatedTakeWindowSnapshot@2(uint64 id) => (bool success, Bitmap snapshot);
+
+  // Get a SnapshotCapturer instance that can snapshot available screens.
+  [MinVersion=1]
+  GetScreenCapturer@3(pending_receiver<SnapshotCapturer> capturer);
+
+  // Get a SnapshotCapturer instance that can snapshot available windows.
+  [MinVersion=1]
+  GetWindowCapturer@4(pending_receiver<SnapshotCapturer> capturer);
 };

content/browser/media/capture/desktop_capturer_lacros.cc

@@ -4,6 +4,8 @@
 
 #include "content/browser/media/capture/desktop_capturer_lacros.h"
 
+#include "base/run_loop.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
 
@@ -11,6 +13,30 @@
 #include "chromeos/lacros/lacros_chrome_service_impl.h"
 
 namespace content {
+namespace {
+
+// An SkBitmap backed subclass of DesktopFrame. This enables the webrtc system
+// to retain the SkBitmap buffer without having to copy the pixels out until
+// they are needed (e.g., for encoding).
+class DesktopFrameSkia : public webrtc::DesktopFrame {
+ public:
+  explicit DesktopFrameSkia(const SkBitmap& bitmap)
+      : webrtc::DesktopFrame(
+            webrtc::DesktopSize(bitmap.width(), bitmap.height()),
+            bitmap.rowBytes(),
+            static_cast<uint8_t*>(bitmap.getPixels()),
+            nullptr),
+        bitmap_(bitmap) {}
+  ~DesktopFrameSkia() override = default;
+
+ private:
+  DesktopFrameSkia(const DesktopFrameSkia&) = delete;
+  DesktopFrameSkia& operator=(const DesktopFrameSkia&) = delete;
+
+  SkBitmap bitmap_;
+};
+
+}  // namespace
 
 DesktopCapturerLacros::DesktopCapturerLacros(
     CaptureType capture_type,
@@ -24,31 +50,32 @@ DesktopCapturerLacros::~DesktopCapturerLacros() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
-bool DesktopCapturerLacros::GetSourceList(SourceList* sources) {
+bool DesktopCapturerLacros::GetSourceList(SourceList* result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (capture_type_ == kScreen) {
-    // TODO(https://crbug.com/1094460): Implement this source list
-    // appropriately.
-    Source w;
-    w.id = 1;
-    sources->push_back(w);
-    return true;
-  }
-
-  EnsureScreenManager();
+  std::vector<crosapi::mojom::SnapshotSourcePtr> sources;
 
-  std::vector<crosapi::mojom::WindowDetailsPtr> windows;
-  {
+  if (snapshot_capturer_) {
+    mojo::SyncCallRestrictions::ScopedAllowSyncCall allow_sync_call;
+    snapshot_capturer_->ListSources(&sources);
+  } else {
+    if (capture_type_ == kScreen) {
+      // TODO(https://crbug.com/1094460): Implement this source list
+      // appropriately.
+      Source w;
+      w.id = 1;
+      result->push_back(w);
+      return true;
+    }
     mojo::SyncCallRestrictions::ScopedAllowSyncCall allow_sync_call;
-    screen_manager_->ListWindows(&windows);
+    screen_manager_->DeprecatedListWindows(&sources);
   }
 
-  for (auto& window : windows) {
-    Source w;
-    w.id = window->id;
-    w.title = window->title;
-    sources->push_back(w);
+  for (auto& source : sources) {
+    Source s;
+    s.id = source->id;
+    s.title = source->title;
+    result->push_back(s);
   }
   return true;
 }
@@ -70,22 +97,55 @@ bool DesktopCapturerLacros::FocusOnSelectedSource() {
 void DesktopCapturerLacros::Start(Callback* callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   callback_ = callback;
+
+  // The lacros chrome service exists at all times except during early start-up
+  // and late shut-down. This class should never be used in those two times.
+  auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
+  DCHECK(lacros_chrome_service);
+  lacros_chrome_service->BindScreenManagerReceiver(
+      screen_manager_.BindNewPipeAndPassReceiver());
+
+  // Do a round-trip to make sure we know what version of the screen manager we
+  // are talking to. It is safe to run a nested loop here because we are on a
+  // dedicated thread (see DesktopCaptureDevice) with nothing else happening.
+  // TODO(crbug/1094460): Avoid this nested event loop.
+  base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+  screen_manager_.QueryVersion(
+      base::BindOnce([](base::OnceClosure quit_closure,
+                        uint32_t version) { std::move(quit_closure).Run(); },
+                     run_loop.QuitClosure()));
+  run_loop.Run();
+
+  if (screen_manager_.version() >= 1) {
+    if (capture_type_ == kScreen) {
+      screen_manager_->GetScreenCapturer(
+          snapshot_capturer_.BindNewPipeAndPassReceiver());
+    } else {
+      screen_manager_->GetWindowCapturer(
+          snapshot_capturer_.BindNewPipeAndPassReceiver());
+    }
+  }
 }
 
 void DesktopCapturerLacros::CaptureFrame() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  EnsureScreenManager();
-
-  if (capture_type_ == kScreen) {
-    screen_manager_->TakeScreenSnapshot(
-        base::BindOnce(&DesktopCapturerLacros::DidTakeSnapshot,
-                       weak_factory_.GetWeakPtr(), /*success*/ true));
-  } else {
-    screen_manager_->TakeWindowSnapshot(
+  if (snapshot_capturer_) {
+    snapshot_capturer_->TakeSnapshot(
         selected_source_,
         base::BindOnce(&DesktopCapturerLacros::DidTakeSnapshot,
                        weak_factory_.GetWeakPtr()));
+  } else {
+    if (capture_type_ == kScreen) {
+      screen_manager_->DeprecatedTakeScreenSnapshot(
+          base::BindOnce(&DesktopCapturerLacros::DeprecatedDidTakeSnapshot,
+                         weak_factory_.GetWeakPtr(), /*success*/ true));
+    } else {
+      screen_manager_->DeprecatedTakeWindowSnapshot(
+          selected_source_,
+          base::BindOnce(&DesktopCapturerLacros::DeprecatedDidTakeSnapshot,
+                         weak_factory_.GetWeakPtr()));
+    }
   }
 }
 
@@ -98,22 +158,23 @@ void DesktopCapturerLacros::SetSharedMemoryFactory(
 
 void DesktopCapturerLacros::SetExcludedWindow(webrtc::WindowId window) {}
 
-void DesktopCapturerLacros::EnsureScreenManager() {
+void DesktopCapturerLacros::DidTakeSnapshot(bool success,
+                                            const SkBitmap& snapshot) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (screen_manager_)
+  if (!success) {
+    callback_->OnCaptureResult(Result::ERROR_PERMANENT,
+                               std::unique_ptr<webrtc::DesktopFrame>());
     return;
+  }
 
-  // The lacros chrome service exists at all times except during early start-up
-  // and late shut-down. This class should never be used in those two times.
-  auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
-  DCHECK(lacros_chrome_service);
-  lacros_chrome_service->BindScreenManagerReceiver(
-      screen_manager_.BindNewPipeAndPassReceiver());
+  callback_->OnCaptureResult(Result::SUCCESS,
+                             std::make_unique<DesktopFrameSkia>(snapshot));
 }
 
-void DesktopCapturerLacros::DidTakeSnapshot(bool success,
-                                            crosapi::Bitmap snapshot) {
+void DesktopCapturerLacros::DeprecatedDidTakeSnapshot(
+    bool success,
+    crosapi::Bitmap snapshot) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   if (!success) {

content/browser/media/capture/desktop_capturer_lacros.h

@@ -47,13 +47,10 @@ class DesktopCapturerLacros : public webrtc::DesktopCapturer {
   void SetExcludedWindow(webrtc::WindowId window) override;
 
  private:
-  // This method is used to lazily initialize screen_manager_ on the same
-  // affine sequence on which |Start| is called.
-  void EnsureScreenManager();
-
   // Callback for when ash-chrome returns a snapshot of the screen or window as
   // a bitmap.
-  void DidTakeSnapshot(bool success, crosapi::Bitmap snapshot);
+  void DidTakeSnapshot(bool success, const SkBitmap& snapshot);
+  void DeprecatedDidTakeSnapshot(bool success, crosapi::Bitmap snapshot);
 
   SEQUENCE_CHECKER(sequence_checker_);
 
@@ -79,6 +76,10 @@ class DesktopCapturerLacros : public webrtc::DesktopCapturer {
   // The remote connection to the screen manager.
   mojo::Remote<crosapi::mojom::ScreenManager> screen_manager_;
 
+  // Only set if we are using a newer screen manager. Otherwise, we fallback to
+  // the deprecated methods.
+  mojo::Remote<crosapi::mojom::SnapshotCapturer> snapshot_capturer_;
+
   base::WeakPtrFactory<DesktopCapturerLacros> weak_factory_{this};
 };