Add APIs to ClipboardNonBacked to get/write ClipboardData.

This is being done to provide ash::ClipboardHistory the ability to
access and restore ClipboardData directly from ClipboardNonBacked
without having to otherwise read/write the clipboard buffer. Manually
reading/writing the buffer would likely be prone to missing some aspects
of ClipboardData if changes were to get out of sync.

Bug: 1104352
Change-Id: I80c800f1ebde26ad3bb8196bc0f438394bed2067
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316802
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792895}

ash/clipboard/clipboard_history.cc

@@ -7,15 +7,10 @@
 #include "ash/clipboard/clipboard_history_controller.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
-#include "base/bind.h"
 #include "base/stl_util.h"
-#include "base/strings/utf_string_conversions.h"
 #include "components/account_id/account_id.h"
-#include "ui/base/clipboard/clipboard.h"
-#include "ui/base/clipboard/clipboard_buffer.h"
-#include "ui/base/clipboard/clipboard_constants.h"
-#include "ui/base/clipboard/clipboard_format_type.h"
 #include "ui/base/clipboard/clipboard_monitor.h"
+#include "ui/base/clipboard/clipboard_non_backed.h"
 
 namespace ash {
 
@@ -81,57 +76,13 @@ void ClipboardHistory::OnClipboardDataChanged() {
   if (num_pause_clipboard_history_ > 0)
     return;
 
-  ui::Clipboard* clipboard = ui::Clipboard::GetForCurrentThread();
+  auto* clipboard = ui::ClipboardNonBacked::GetForCurrentThread();
   CHECK(clipboard);
 
-  std::vector<base::string16> mime_types;
-  clipboard->ReadAvailableTypes(ui::ClipboardBuffer::kCopyPaste,
-                                /* data_dst = */ nullptr, &mime_types);
-
-  ui::ClipboardData new_data;
-  bool contains_bitmap = false;
-  for (const auto& type16 : mime_types) {
-    const std::string type(base::UTF16ToUTF8(type16));
-    if (type == ui::ClipboardFormatType::GetPlainTextType().GetName()) {
-      base::string16 text;
-      clipboard->ReadText(ui::ClipboardBuffer::kCopyPaste,
-                          /* data_dst = */ nullptr, &text);
-      new_data.set_text(base::UTF16ToUTF8(text));
-    } else if (type == ui::ClipboardFormatType::GetHtmlType().GetName()) {
-      uint32_t start, end;
-      base::string16 html_markup;
-      std::string src_url;
-      clipboard->ReadHTML(ui::ClipboardBuffer::kCopyPaste,
-                          /* data_dst = */ nullptr, &html_markup, &src_url,
-                          &start, &end);
-      new_data.set_markup_data(base::UTF16ToUTF8(html_markup));
-      new_data.set_url(src_url);
-    } else if (type == ui::ClipboardFormatType::GetRtfType().GetName()) {
-      std::string rtf_data;
-      clipboard->ReadRTF(ui::ClipboardBuffer::kCopyPaste,
-                         /* data_dst = */ nullptr, &rtf_data);
-      new_data.SetRTFData(rtf_data);
-    } else if (type == ui::ClipboardFormatType::GetBitmapType().GetName()) {
-      contains_bitmap = true;
-    }
-  }
-
-  if (contains_bitmap) {
-    clipboard->ReadImage(
-        ui::ClipboardBuffer::kCopyPaste, /*data_dst=*/nullptr,
-        base::BindOnce(&ClipboardHistory::OnRecievePNGFromClipboard,
-                       weak_ptr_factory_.GetWeakPtr(), GetActiveAccountId(),
-                       std::move(new_data)));
-    return;
-  }
+  const auto* clipboard_data = clipboard->GetClipboardData();
+  CHECK(clipboard_data);
 
-  std::string custom_data;
-  const auto& custom_format = ui::ClipboardFormatType::GetWebCustomDataType();
-  clipboard->ReadData(custom_format, /* data_dst = */ nullptr, &custom_data);
-  if (!custom_data.empty())
-    new_data.SetCustomData(custom_format.GetName(), custom_data);
-
-  CommitData(GetActiveAccountId(), std::move(new_data));
+  CommitData(GetActiveAccountId(), ui::ClipboardData(*clipboard_data));
 }
 
 void ClipboardHistory::CommitData(const AccountId& account_id,
@@ -159,13 +110,6 @@ void ClipboardHistory::CommitData(const AccountId& account_id,
     history_with_duplicates.pop_front();
 }
 
-void ClipboardHistory::OnRecievePNGFromClipboard(const AccountId& account_id,
-                                                 ui::ClipboardData data,
-                                                 const SkBitmap& bitmap) {
-  data.SetBitmapData(bitmap);
-  CommitData(account_id, std::move(data));
-}
-
 void ClipboardHistory::PauseClipboardHistory() {
   ++num_pause_clipboard_history_;
 }

ash/clipboard/clipboard_history.h

@@ -14,7 +14,6 @@
 #include "ash/shell_observer.h"
 #include "base/component_export.h"
 #include "base/memory/weak_ptr.h"
-#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/clipboard/clipboard_data.h"
 #include "ui/base/clipboard/clipboard_observer.h"
 
@@ -22,7 +21,7 @@ class AccountId;
 
 namespace ui {
 class ClipboardData;
-}
+}  // namespace ui
 
 namespace ash {
 
@@ -68,12 +67,6 @@ class ASH_EXPORT ClipboardHistory : public ui::ClipboardObserver,
   // Adds |data| to the clipboard history belonging to the account indicated
   // by |account_id|.
   void CommitData(const AccountId& account_id, ui::ClipboardData data);
-
-  // Callback to read a bitmap from the clipboard data belonging to the account
-  // indicated by |active_account_id|.
-  void OnRecievePNGFromClipboard(const AccountId& active_account_id,
-                                 ui::ClipboardData data,
-                                 const SkBitmap& bitmap);
   void PauseClipboardHistory();
   void UnPauseClipboardHistory();
 

ash/clipboard/clipboard_history_controller.cc

@@ -17,11 +17,9 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/accelerators/accelerator.h"
-#include "ui/base/clipboard/clipboard.h"
-#include "ui/base/clipboard/clipboard_buffer.h"
 #include "ui/base/clipboard/clipboard_constants.h"
 #include "ui/base/clipboard/clipboard_data.h"
-#include "ui/base/clipboard/scoped_clipboard_writer.h"
+#include "ui/base/clipboard/clipboard_non_backed.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/base/models/image_model.h"
@@ -94,27 +92,10 @@ ui::ImageModel GetImageModelForClipboardData(const ui::ClipboardData& item) {
 }
 
 void WriteClipboardDataToClipboard(const ui::ClipboardData& data) {
-  ui::ScopedClipboardWriter writer(ui::ClipboardBuffer::kCopyPaste);
-  if (data.format() & static_cast<int>(ui::ClipboardInternalFormat::kBitmap))
-    writer.WriteImage(data.bitmap());
-  if (data.format() & static_cast<int>(ui::ClipboardInternalFormat::kText))
-    writer.WriteText(base::UTF8ToUTF16(data.text()));
-  if (data.format() & static_cast<int>(ui::ClipboardInternalFormat::kHtml))
-    writer.WriteHTML(base::UTF8ToUTF16(data.markup_data()), data.url());
-  if (data.format() & static_cast<int>(ui::ClipboardInternalFormat::kRtf))
-    writer.WriteRTF(data.rtf_data());
-  if (data.format() &
-      static_cast<int>(ui::ClipboardInternalFormat::kBookmark)) {
-    writer.WriteBookmark(base::UTF8ToUTF16(data.bookmark_title()),
-                         data.bookmark_url());
-  }
-  if (data.format() & static_cast<int>(ui::ClipboardInternalFormat::kCustom)) {
-    const auto& custom_format = ui::ClipboardFormatType::GetWebCustomDataType();
-    DCHECK_EQ(data.custom_data_format(), custom_format.GetName());
-    writer.WritePickledData(base::Pickle(data.custom_data_data().c_str(),
-                                         data.custom_data_data().size()),
-                            custom_format);
-  }
+  auto* clipboard = ui::ClipboardNonBacked::GetForCurrentThread();
+  CHECK(clipboard);
+
+  clipboard->WriteClipboardData(std::make_unique<ui::ClipboardData>(data));
 }
 
 class ClipboardHistoryMenuDelegate : public ui::SimpleMenuModel::Delegate {

ui/base/clipboard/BUILD.gn

@@ -233,7 +233,10 @@ source_set("clipboard_test") {
   }
 
   if (use_aura && !chromeos_is_browser_only && !use_x11 && !is_win) {
-    sources += [ "clipboard_data_unittest.cc" ]
+    sources += [
+      "clipboard_data_unittest.cc",
+      "clipboard_non_backed_unittest.cc",
+    ]
   }
 
   deps = [

ui/base/clipboard/clipboard_non_backed.cc

@@ -9,6 +9,7 @@
 #include <limits>
 #include <list>
 #include <memory>
+#include <set>
 #include <utility>
 
 #include "base/check_op.h"
@@ -18,6 +19,7 @@
 #include "base/no_destructor.h"
 #include "base/notreached.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/synchronization/lock.h"
 #include "skia/ext/skia_utils_base.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/clipboard/clipboard_constants.h"
@@ -33,6 +35,51 @@
 
 namespace ui {
 
+namespace {
+
+// Returns the registry which tracks all instances of ClipboardNonBacked in
+// existence. This allows us to determine if any arbitrary Clipboard pointer in
+// fact points to a ClipboardNonBacked instance. Only if a pointer exists in
+// this registry is it safe to cast to ClipboardNonBacked*.
+std::set<const ClipboardNonBacked*>* GetInstanceRegistry() {
+  static base::NoDestructor<std::set<const ClipboardNonBacked*>> registry;
+  return registry.get();
+}
+
+// The ClipboardNonBacked instance registry can be accessed by multiple threads.
+// Any inspection/modification of the instance registry must only be performed
+// while this lock is held.
+base::Lock& GetInstanceRegistryLock() {
+  static base::NoDestructor<base::Lock> registry_lock;
+  return *registry_lock;
+}
+
+// Registers the specified |clipboard| as an instance of ClipboardNonBacked.
+// Registration should occur during |clipboard| construction and registration
+// should be maintained until |clipboard| is destroyed. This allows us to check
+// if any arbitrary Clipboard* is safe to cast.
+void RegisterInstance(const ClipboardNonBacked* clipboard) {
+  base::AutoLock lock(GetInstanceRegistryLock());
+  GetInstanceRegistry()->insert(clipboard);
+}
+
+// Unregisters the specified |clipboard| as a instance of ClipboardNonBacked.
+// This should only be done when destroying |clipboard|.
+void UnregisterInstance(const ClipboardNonBacked* clipboard) {
+  base::AutoLock lock(GetInstanceRegistryLock());
+  GetInstanceRegistry()->erase(clipboard);
+}
+
+// Checks if |clipboard| is registered as an instance of ClipboardNonBacked.
+// Only if this method returns true is it safe to cast |clipboard| to
+// ClipboardNonBacked*.
+bool IsRegisteredInstance(const Clipboard* clipboard) {
+  base::AutoLock lock(GetInstanceRegistryLock());
+  return base::Contains(*GetInstanceRegistry(), clipboard);
+}
+
+}  // namespace
+
 // Simple, internal implementation of a clipboard, handling things like format
 // conversion, sequence numbers, etc.
 class ClipboardInternal {
@@ -294,14 +341,34 @@ Clipboard* Clipboard::Create() {
 }
 #endif
 
+// static
+ClipboardNonBacked* ClipboardNonBacked::GetForCurrentThread() {
+  auto* clipboard = Clipboard::GetForCurrentThread();
+  CHECK(IsRegisteredInstance(clipboard));  // Ensure type safety.
+  return static_cast<ClipboardNonBacked*>(clipboard);
+}
+
 // ClipboardNonBacked implementation.
 ClipboardNonBacked::ClipboardNonBacked()
     : clipboard_internal_(std::make_unique<ClipboardInternal>()) {
   DCHECK(CalledOnValidThread());
+  RegisterInstance(this);
 }
 
 ClipboardNonBacked::~ClipboardNonBacked() {
   DCHECK(CalledOnValidThread());
+  UnregisterInstance(this);
+}
+
+const ClipboardData* ClipboardNonBacked::GetClipboardData() const {
+  DCHECK(CalledOnValidThread());
+  return clipboard_internal_->GetData();
+}
+
+void ClipboardNonBacked::WriteClipboardData(
+    std::unique_ptr<ClipboardData> data) {
+  DCHECK(CalledOnValidThread());
+  clipboard_internal_->WriteData(std::move(data));
 }
 
 void ClipboardNonBacked::OnPreShutdown() {}

ui/base/clipboard/clipboard_non_backed.h

@@ -8,11 +8,13 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "base/component_export.h"
 #include "base/macros.h"
 #include "ui/base/clipboard/clipboard.h"
 
 namespace ui {
 
+class ClipboardData;
 class ClipboardInternal;
 
 // In-memory clipboard implementation not backed by an underlying platform.
@@ -21,9 +23,23 @@ class ClipboardInternal;
 // ClipboardWin on Windows or ClipboardMac on MacOS. As this isn't backed by an
 // underlying platform, the clipboard data isn't persisted after an instance
 // goes away.
-class ClipboardNonBacked : public Clipboard {
+class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardNonBacked
+    : public Clipboard {
+ public:
+  // Returns the in-memory clipboard for the current thread. Note that this
+  // method must *only* be used when the caller is sure that the clipboard for
+  // the current thread is in fact an instance of ClipboardNonBacked.
+  static ClipboardNonBacked* GetForCurrentThread();
+
+  // Returns the current ClipboardData.
+  const ClipboardData* GetClipboardData() const;
+
+  // Writes the current ClipboardData.
+  void WriteClipboardData(std::unique_ptr<ClipboardData> data);
+
  private:
   friend class Clipboard;
+  friend class ClipboardNonBackedTest;
   ClipboardNonBacked();
   ~ClipboardNonBacked() override;
 

ui/base/clipboard/clipboard_non_backed_unittest.cc

+// Copyright (c) 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.
+
+#include "ui/base/clipboard/clipboard_non_backed.h"
+
+#include <memory>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/clipboard/clipboard_data.h"
+
+namespace ui {
+
+class ClipboardNonBackedTest : public testing::Test {
+ public:
+  ClipboardNonBackedTest() = default;
+  ClipboardNonBackedTest(const ClipboardNonBackedTest&) = delete;
+  ClipboardNonBackedTest& operator=(const ClipboardNonBackedTest&) = delete;
+  ~ClipboardNonBackedTest() override = default;
+
+  ClipboardNonBacked* clipboard() { return &clipboard_; }
+
+ private:
+  ClipboardNonBacked clipboard_;
+};
+
+// Verifies that GetClipboardData() returns the same instance of ClipboardData
+// as was written via WriteClipboardData().
+TEST_F(ClipboardNonBackedTest, WriteAndGetClipboardData) {
+  auto clipboard_data = std::make_unique<ClipboardData>();
+
+  auto* expected_clipboard_data_ptr = clipboard_data.get();
+  clipboard()->WriteClipboardData(std::move(clipboard_data));
+  auto* actual_clipboard_data_ptr = clipboard()->GetClipboardData();
+
+  EXPECT_EQ(expected_clipboard_data_ptr, actual_clipboard_data_ptr);
+}
+
+}  // namespace ui