From bc609326e427c205c4143c00ff5e50d91d0bc239 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Mon, 6 Dec 2021 19:10:12 +0000 Subject: [PATCH] Clipboard: Allow more flexibility in when PNGs are encoded on Chrome OS While transitioning to reading images as PNGs from the clipboard, we changed the backing store of images on Chrome OS to be raw PNG bytes rather than bitmaps. The naive implementation meant encoding bitmaps as PNGs on the UI thread, which is not acceptable behavior going forward. This CL removes the ability to read images as bitmaps. Only allowing images to be read as PNGs greatly reduces the number of states the clipboard can be in. Images must be encoded to PNG format before they are read. This CL allows the owner of the clipboard to determine the best time to encode the bitmap to a PNG by temporarily storing the bitmap for as long as the PNG has not yet been encoded. This enables the owner to encode the PNG on a background thread at a time it chooses. Bug: 1247356 Change-Id: Ia87a42b3eb64725662eee97f853488342c99473e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3260985 Reviewed-by: Gary Kacmarcik Reviewed-by: Alex Newcomer Reviewed-by: Xiyuan Xia Reviewed-by: Francois Pierre Doray Reviewed-by: Aya Elsayed Reviewed-by: Elly Fong-Jones Reviewed-by: Yuichiro Hanada Reviewed-by: Olivier Li Commit-Queue: Austin Sullivan Cr-Commit-Position: refs/heads/main@{#948608} --- .../clipboard_history_controller_impl.cc | 129 ++++++++++++++++-- .../clipboard_history_controller_impl.h | 31 ++++- .../clipboard_history_controller_unittest.cc | 111 +++++++++++++++ .../clipboard_history_resource_manager.cc | 4 +- ash/clipboard/clipboard_history_unittest.cc | 9 +- .../clipboard_history_bitmap_item_view.cc | 16 ++- ash/public/cpp/clipboard_history_controller.h | 8 +- .../chrome_virtual_keyboard_delegate.cc | 51 ++++--- .../chrome_virtual_keyboard_delegate.h | 2 + .../host/chromeos/clipboard_aura_unittest.cc | 4 +- ui/base/clipboard/clipboard_data.cc | 103 +++++++++++--- ui/base/clipboard/clipboard_data.h | 49 ++++++- ui/base/clipboard/clipboard_data_unittest.cc | 47 +++++-- ui/base/clipboard/clipboard_non_backed.cc | 61 ++++++++- ui/base/clipboard/clipboard_non_backed.h | 1 + .../clipboard_non_backed_unittest.cc | 48 ++++++- ui/views/selection_controller_unittest.cc | 6 +- 17 files changed, 598 insertions(+), 82 deletions(-) diff --git a/ash/clipboard/clipboard_history_controller_impl.cc b/ash/clipboard/clipboard_history_controller_impl.cc index facf479055ec9..a738e86a15537 100644 --- a/ash/clipboard/clipboard_history_controller_impl.cc +++ b/ash/clipboard/clipboard_history_controller_impl.cc @@ -4,7 +4,10 @@ #include "ash/clipboard/clipboard_history_controller_impl.h" +#include #include +#include +#include #include "ash/accelerators/accelerator_controller_impl.h" #include "ash/clipboard/clipboard_history_menu_model_adapter.h" @@ -22,17 +25,24 @@ #include "ash/shell.h" #include "ash/wm/window_util.h" #include "base/bind.h" +#include "base/json/values_util.h" #include "base/location.h" #include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" +#include "base/ranges/algorithm.h" #include "base/strings/utf_string_conversions.h" +#include "base/task/thread_pool.h" #include "base/threading/thread_task_runner_handle.h" +#include "base/unguessable_token.h" +#include "base/values.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "ui/aura/window.h" #include "ui/aura/window_tree_host.h" #include "ui/base/accelerators/accelerator.h" #include "ui/base/clipboard/clipboard_constants.h" #include "ui/base/clipboard/clipboard_data.h" #include "ui/base/clipboard/clipboard_non_backed.h" +#include "ui/base/clipboard/scoped_clipboard_writer.h" #include "ui/base/data_transfer_policy/data_transfer_endpoint.h" #include "ui/base/ime/input_method.h" #include "ui/base/ime/text_input_client.h" @@ -68,6 +78,24 @@ ui::ClipboardNonBacked* GetClipboard() { return clipboard; } +// Serially encodes bitmaps in |bitmaps_to_be_encoded| to PNGs. +// This function should run on a background thread. +// |bitmaps_to_be_encoded| maps ClipboardHistoryItem IDs to their corresponding +// bitmaps which need to be encoded. Returns a map of ClipboardHistoryItem +// IDs to encoded PNGs. +// TODO(crbug.com/1277000): Rather than encoding serially, consider posting each +// encoding as a task. +std::map> EncodeBitmapsToPNG( + std::map bitmaps_to_be_encoded) { + std::map> encoded_pngs; + base::ranges::for_each(bitmaps_to_be_encoded, [&](const auto& id_and_bitmap) { + encoded_pngs.emplace( + id_and_bitmap.first, + ui::ClipboardData::EncodeBitmapData(id_and_bitmap.second)); + }); + return encoded_pngs; +} + } // namespace // ClipboardHistoryControllerImpl::AcceleratorTarget --------------------------- @@ -285,6 +313,12 @@ gfx::Rect ClipboardHistoryControllerImpl::GetMenuBoundsInScreenForTest() const { return context_menu_->GetMenuBoundsInScreenForTest(); } +void ClipboardHistoryControllerImpl::GetHistoryValuesForTest( + GetHistoryValuesCallback callback) const { + GetHistoryValues(/*item_id_filter=*/std::set(), + std::move(callback)); +} + bool ClipboardHistoryControllerImpl::ShouldShowNewFeatureBadge() const { return chromeos::features::IsClipboardHistoryContextMenuNudgeEnabled() && nudge_controller_->ShouldShowNewFeatureBadge(); @@ -312,12 +346,63 @@ ClipboardHistoryControllerImpl::CreateScopedPause() { clipboard_history_.get()); } -base::Value ClipboardHistoryControllerImpl::GetHistoryValues( - const std::set& item_id_filter) const { +// TODO(crbug.com/1272798): If there are multiple calls in a row to +// GetHistoryValues, the same bitmaps may be encoded to PNG multiple times which +// is a resource waste. The ClipboardHistoryControllerImpl should track the ids +// of ClipboardHistoryItems for which there is a pending encoding task and avoid +// scheduling a duplicate encoding task for these items. +void ClipboardHistoryControllerImpl::GetHistoryValues( + const std::set& item_id_filter, + GetHistoryValuesCallback callback) const { + // Map of ClipboardHistoryItem IDs to their corresponding bitmaps. + std::map bitmaps_to_be_encoded; + // Get the clipboard data for each clipboard history item. + for (auto& item : clipboard_history_->GetItems()) { + // If the |item_id_filter| contains values, then only return the clipboard + // items included in it. + if (!item_id_filter.empty() && + item_id_filter.find(item.id().ToString()) == item_id_filter.end()) { + continue; + } + + if (ash::ClipboardHistoryUtil::CalculateDisplayFormat(item.data()) == + ash::ClipboardHistoryUtil::ClipboardHistoryDisplayFormat::kPng) { + const auto& maybe_png = item.data().maybe_png(); + if (!maybe_png.has_value()) { + // The clipboard contains an image which has not yet been encoded to a + // PNG. + auto maybe_bitmap = item.data().GetBitmapIfPngNotEncoded(); + DCHECK(maybe_bitmap.has_value()); + bitmaps_to_be_encoded.emplace(item.id(), + std::move(maybe_bitmap.value())); + } + } + } + + // Encode images on a background thread. + base::ThreadPool::PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce(&EncodeBitmapsToPNG, std::move(bitmaps_to_be_encoded)), + base::BindOnce( + &ClipboardHistoryControllerImpl::GetHistoryValuesWithEncodedPNGs, + weak_ptr_factory_.GetWeakPtr(), item_id_filter, std::move(callback))); + + if (!new_bitmap_to_write_while_encoding_for_test_.isNull()) { + ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste); + scw.WriteImage(new_bitmap_to_write_while_encoding_for_test_); + new_bitmap_to_write_while_encoding_for_test_.reset(); + } +} + +void ClipboardHistoryControllerImpl::GetHistoryValuesWithEncodedPNGs( + const std::set& item_id_filter, + GetHistoryValuesCallback callback, + std::map> encoded_pngs) { base::Value item_results(base::Value::Type::LIST); + bool all_images_encoded = true; // Get the clipboard data for each clipboard history item. - for (const auto& item : history()->GetItems()) { + for (auto& item : clipboard_history_->GetItems()) { // If the |item_id_filter| contains values, then only return the clipboard // items included in it. if (!item_id_filter.empty() && @@ -327,12 +412,33 @@ base::Value ClipboardHistoryControllerImpl::GetHistoryValues( base::Value item_value(base::Value::Type::DICTIONARY); switch (ash::ClipboardHistoryUtil::CalculateDisplayFormat(item.data())) { - case ash::ClipboardHistoryUtil::ClipboardHistoryDisplayFormat::kPng: - item_value.SetKey(kImageDataKey, base::Value(webui::GetPngDataUrl( - item.data().png().data(), - item.data().png().size()))); - item_value.SetKey(kFormatDataKey, base::Value(kPngFormat)); + case ash::ClipboardHistoryUtil::ClipboardHistoryDisplayFormat::kPng: { + if (!item.data().maybe_png().has_value()) { + // The clipboard contains an image which has not yet been encoded to a + // PNG. Hopefully we just finished encoding and the PNG can be found + // in `encoded_pngs`, otherwise this item was added while other PNGs + // were being encoded. + auto png_it = encoded_pngs.find(item.id()); + if (png_it == encoded_pngs.end()) { + // Can't find the encoded PNG. We'll need to restart + // GetHistoryValues from the top, but allow this for loop to finish + // to let PNGs we've already encoded get set to their appropriate + // clipboards, to avoid re-encoding. + all_images_encoded = false; + } else { + item.data().SetPngDataAfterEncoding(std::move(png_it->second)); + } + } + + const auto& maybe_png = item.data().maybe_png(); + if (maybe_png.has_value()) { + item_value.SetKey(kImageDataKey, base::Value(webui::GetPngDataUrl( + maybe_png.value().data(), + maybe_png.value().size()))); + item_value.SetKey(kFormatDataKey, base::Value(kPngFormat)); + } break; + } case ash::ClipboardHistoryUtil::ClipboardHistoryDisplayFormat::kHtml: { const SkBitmap& bitmap = *(resource_manager_->GetImageModel(item).GetImage().ToSkBitmap()); @@ -364,7 +470,12 @@ base::Value ClipboardHistoryControllerImpl::GetHistoryValues( item_results.Append(std::move(item_value)); } - return item_results; + if (!all_images_encoded) { + GetHistoryValues(item_id_filter, std::move(callback)); + return; + } + + std::move(callback).Run(std::move(item_results)); } std::vector ClipboardHistoryControllerImpl::GetHistoryItemIds() diff --git a/ash/clipboard/clipboard_history_controller_impl.h b/ash/clipboard/clipboard_history_controller_impl.h index c6226971ad036..ddad1fc43f442 100644 --- a/ash/clipboard/clipboard_history_controller_impl.h +++ b/ash/clipboard/clipboard_history_controller_impl.h @@ -5,7 +5,9 @@ #ifndef ASH_CLIPBOARD_CLIPBOARD_HISTORY_CONTROLLER_IMPL_H_ #define ASH_CLIPBOARD_CLIPBOARD_HISTORY_CONTROLLER_IMPL_H_ +#include #include +#include #include #include "ash/ash_export.h" @@ -14,9 +16,12 @@ #include "ash/clipboard/clipboard_history_resource_manager.h" #include "ash/public/cpp/clipboard_history_controller.h" #include "base/callback.h" +#include "base/callback_forward.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" #include "base/timer/timer.h" +#include "base/unguessable_token.h" +#include "base/values.h" #include "chromeos/crosapi/mojom/clipboard_history.mojom.h" namespace aura { @@ -71,6 +76,8 @@ class ASH_EXPORT ClipboardHistoryControllerImpl // Returns bounds for the contextual menu in screen coordinates. gfx::Rect GetMenuBoundsInScreenForTest() const; + void GetHistoryValuesForTest(GetHistoryValuesCallback callback) const; + // Whether the ClipboardHistory has items. bool IsEmpty() const; @@ -101,6 +108,10 @@ class ASH_EXPORT ClipboardHistoryControllerImpl confirmed_operation_callback_for_test_ = new_callback; } + void set_new_bitmap_to_write_while_encoding_for_test(const SkBitmap& bitmap) { + new_bitmap_to_write_while_encoding_for_test_ = bitmap; + } + private: class AcceleratorTarget; class MenuDelegate; @@ -111,8 +122,8 @@ class ASH_EXPORT ClipboardHistoryControllerImpl void MarkNewFeatureBadgeShown() override; void OnScreenshotNotificationCreated() override; std::unique_ptr CreateScopedPause() override; - base::Value GetHistoryValues( - const std::set& item_id_filter) const override; + void GetHistoryValues(const std::set& item_id_filter, + GetHistoryValuesCallback callback) const override; std::vector GetHistoryItemIds() const override; bool PasteClipboardItemById(const std::string& item_id) override; bool DeleteClipboardItemById(const std::string& item_id) override; @@ -128,6 +139,16 @@ class ASH_EXPORT ClipboardHistoryControllerImpl void OnCachedImageModelUpdated( const std::vector& menu_item_ids) override; + // Invoked by GetHistoryValues once all clipboard instances with images have + // been encoded into PNGs. Returns the history which tracks what is being + // copied to the clipboard. Only the items listed in `item_id_filter` are + // returned. If `item_id_filter` is empty, then all items in the history are + // returned. + void GetHistoryValuesWithEncodedPNGs( + const std::set& item_id_filter, + GetHistoryValuesCallback callback, + std::map> encoded_pngs); + void ExecuteSelectedMenuItem(int event_flags); // Executes the command specified by `command_id` with the given event flags. @@ -200,6 +221,12 @@ class ASH_EXPORT ClipboardHistoryControllerImpl // operation. base::RepeatingClosure confirmed_operation_callback_for_test_; + // A new bitmap to be written to the clipboard while existing images are being + // encoded during `GetHistoryValues()`, which will force `GetHistoryValues()` + // to re-run in order to encode this new bitmap. This member is marked mutable + // so it can be cleared once it has been written to the clipboard. + mutable SkBitmap new_bitmap_to_write_while_encoding_for_test_; + base::WeakPtrFactory weak_ptr_factory_{this}; }; diff --git a/ash/clipboard/clipboard_history_controller_unittest.cc b/ash/clipboard/clipboard_history_controller_unittest.cc index 27797f403f905..d616e22a8f70b 100644 --- a/ash/clipboard/clipboard_history_controller_unittest.cc +++ b/ash/clipboard/clipboard_history_controller_unittest.cc @@ -5,6 +5,7 @@ #include "ash/clipboard/clipboard_history_controller_impl.h" #include +#include #include "ash/app_list/app_list_controller_impl.h" #include "ash/clipboard/clipboard_history.h" @@ -19,14 +20,19 @@ #include "base/strings/utf_string_conversions.h" #include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_feature_list.h" +#include "base/test/test_future.h" #include "base/threading/sequenced_task_runner_handle.h" #include "base/unguessable_token.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/base/clipboard/clipboard_buffer.h" +#include "ui/base/clipboard/clipboard_data.h" #include "ui/base/clipboard/scoped_clipboard_writer.h" #include "ui/base/models/image_model.h" +#include "ui/base/webui/web_ui_util.h" #include "ui/events/test/event_generator.h" +#include "ui/gfx/codec/png_codec.h" +#include "ui/gfx/image/image_unittest_util.h" namespace ash { @@ -75,6 +81,19 @@ class MockClipboardImageModelFactory : public ClipboardImageModelFactory { void OnShutdown() override {} }; +void ExpectHistoryValueMatchesBitmap(const base::Value& value, + const SkBitmap& expected_bitmap) { + auto* format = value.FindKey("displayFormat"); + EXPECT_TRUE(format); + EXPECT_EQ(format->GetString(), "png"); + + auto* image_data = value.FindKey("imageData"); + EXPECT_TRUE(image_data); + auto png = ui::ClipboardData::EncodeBitmapData(expected_bitmap); + std::string png_data_url = webui::GetPngDataUrl(png.data(), png.size()); + EXPECT_EQ(png_data_url, image_data->GetString()); +} + } // namespace class ClipboardHistoryControllerTest : public AshTestBase { @@ -312,4 +331,96 @@ TEST_F(ClipboardHistoryControllerTest, EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing()); } +TEST_F(ClipboardHistoryControllerTest, EncodeImage) { + // Write a bitmap to ClipboardHistory. + SkBitmap test_bitmap = gfx::test::CreateBitmap(3, 2); + { + ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste); + scw.WriteImage(test_bitmap); + } + FlushMessageLoop(); + + // The bitmap should be encoded to a PNG once this task completes. + base::test::TestFuture future; + GetClipboardHistoryController()->GetHistoryValuesForTest( + future.GetCallback()); + EXPECT_TRUE(future.Wait()); + + // Manually pry into the contents of the result to confirm that the + // newly-encoded PNG is included. + base::Value result = future.Take(); + EXPECT_TRUE(result.is_list()); + EXPECT_EQ(1u, result.GetList().size()); + + ExpectHistoryValueMatchesBitmap(result.GetList()[0], test_bitmap); +} + +TEST_F(ClipboardHistoryControllerTest, EncodeMultipleImages) { + // Write a bunch of bitmaps to ClipboardHistory. + std::vector test_bitmaps; + test_bitmaps.emplace_back(gfx::test::CreateBitmap(2, 1)); + test_bitmaps.emplace_back(gfx::test::CreateBitmap(3, 2)); + test_bitmaps.emplace_back(gfx::test::CreateBitmap(4, 3)); + for (const auto& test_bitmap : test_bitmaps) { + ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste); + scw.WriteImage(test_bitmap); + FlushMessageLoop(); + } + + // The bitmaps should be encoded to PNGs once this task completes. + base::test::TestFuture future; + GetClipboardHistoryController()->GetHistoryValuesForTest( + future.GetCallback()); + EXPECT_TRUE(future.Wait()); + + // Manually pry into the contents of the result to confirm that the + // newly-encoded PNGs are included. + base::Value result = future.Take(); + EXPECT_TRUE(result.is_list()); + auto num_results = result.GetList().size(); + EXPECT_EQ(num_results, test_bitmaps.size()); + + // History values should be sorted by recency. + for (uint i = 0; i < num_results; ++i) { + ExpectHistoryValueMatchesBitmap(result.GetList()[i], + test_bitmaps[num_results - 1 - i]); + } +} + +TEST_F(ClipboardHistoryControllerTest, WriteBitmapWhileEncodingImage) { + // Write a bitmap to ClipboardHistory. + std::vector test_bitmaps; + test_bitmaps.emplace_back(gfx::test::CreateBitmap(3, 2)); + test_bitmaps.emplace_back(gfx::test::CreateBitmap(4, 3)); + { + ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste); + scw.WriteImage(test_bitmaps[0]); + } + FlushMessageLoop(); + + // Write another bitmap to the clipboard while encoding the first bitmap. + GetClipboardHistoryController() + ->set_new_bitmap_to_write_while_encoding_for_test(test_bitmaps[1]); + + base::test::TestFuture future; + GetClipboardHistoryController()->GetHistoryValuesForTest( + future.GetCallback()); + + // Both bitmaps should be encoded to a PNG once this task completes. + EXPECT_TRUE(future.Wait()); + + // Manually pry into the contents of the result to confirm that the + // newly-encoded PNGs are included. + base::Value result = future.Take(); + EXPECT_TRUE(result.is_list()); + auto num_results = result.GetList().size(); + EXPECT_EQ(num_results, test_bitmaps.size()); + + // History values should be sorted by recency. + for (uint i = 0; i < num_results; ++i) { + ExpectHistoryValueMatchesBitmap(result.GetList()[i], + test_bitmaps[num_results - 1 - i]); + } +} + } // namespace ash diff --git a/ash/clipboard/clipboard_history_resource_manager.cc b/ash/clipboard/clipboard_history_resource_manager.cc index 662daa60869a0..203cd67136b51 100644 --- a/ash/clipboard/clipboard_history_resource_manager.cc +++ b/ash/clipboard/clipboard_history_resource_manager.cc @@ -270,7 +270,9 @@ void ClipboardHistoryResourceManager::OnClipboardHistoryItemAdded( // See if we have an |existing| item that will render the same as |item|. auto it = std::find_if(items.begin(), items.end(), [&](const auto& existing) { - return &existing != &item && existing.data().png().empty() && + return &existing != &item && + !(existing.data().format() & + static_cast(ui::ClipboardInternalFormat::kPng)) && existing.data().markup_data() == item.data().markup_data(); }); diff --git a/ash/clipboard/clipboard_history_unittest.cc b/ash/clipboard/clipboard_history_unittest.cc index 107f33f9ab54e..bb177f2b313f9 100644 --- a/ash/clipboard/clipboard_history_unittest.cc +++ b/ash/clipboard/clipboard_history_unittest.cc @@ -26,6 +26,7 @@ #include "ui/base/clipboard/scoped_clipboard_writer.h" #include "ui/events/event_constants.h" #include "ui/events/test/event_generator.h" +#include "ui/gfx/codec/png_codec.h" #include "ui/gfx/image/image_unittest_util.h" #include "ui/gfx/skia_util.h" @@ -105,8 +106,14 @@ class ClipboardHistoryTest : public AshTestBase { int expected_bitmaps_index = 0; for (const auto& item : items) { + // The PNG should not have yet been encoded. + const auto& maybe_png = item.data().maybe_png(); + EXPECT_FALSE(maybe_png.has_value()); + + auto maybe_bitmap = item.data().GetBitmapIfPngNotEncoded(); + EXPECT_TRUE(maybe_bitmap.has_value()); EXPECT_TRUE(gfx::BitmapsAreEqual( - expected_bitmaps[expected_bitmaps_index++], item.data().bitmap())); + expected_bitmaps[expected_bitmaps_index++], maybe_bitmap.value())); } } diff --git a/ash/clipboard/views/clipboard_history_bitmap_item_view.cc b/ash/clipboard/views/clipboard_history_bitmap_item_view.cc index dc013f79486f6..1411af26de392 100644 --- a/ash/clipboard/views/clipboard_history_bitmap_item_view.cc +++ b/ash/clipboard/views/clipboard_history_bitmap_item_view.cc @@ -239,9 +239,19 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView weak_ptr_factory_.GetWeakPtr())); case ui::ClipboardInternalFormat::kPng: { auto image_view = std::make_unique(); - gfx::Image image = gfx::Image::CreateFrom1xPNGBytes( - clipboard_history_item->data().png().data(), - clipboard_history_item->data().png().size()); + gfx::Image image; + const auto& maybe_png = clipboard_history_item->data().maybe_png(); + if (maybe_png.has_value()) { + image = gfx::Image::CreateFrom1xPNGBytes(maybe_png.value().data(), + maybe_png.value().size()); + } else { + // If we have not yet encoded the bitmap to a PNG, just create the + // gfx::Image using the available bitmap. No information is lost here. + auto maybe_bitmap = + clipboard_history_item->data().GetBitmapIfPngNotEncoded(); + DCHECK(maybe_bitmap.has_value()); + image = gfx::Image::CreateFrom1xBitmap(maybe_bitmap.value()); + } ui::ImageModel image_model = ui::ImageModel::FromImage(image); image_view->SetImage(image_model); return image_view; diff --git a/ash/public/cpp/clipboard_history_controller.h b/ash/public/cpp/clipboard_history_controller.h index 02bb4a4637fec..3c1e337a2d77b 100644 --- a/ash/public/cpp/clipboard_history_controller.h +++ b/ash/public/cpp/clipboard_history_controller.h @@ -9,7 +9,9 @@ #include #include "ash/public/cpp/ash_public_export.h" +#include "base/callback_forward.h" #include "base/observer_list_types.h" +#include "base/values.h" #include "chromeos/crosapi/mojom/clipboard_history.mojom.h" #include "ui/base/ui_base_types.h" @@ -29,6 +31,8 @@ class ScopedClipboardHistoryPause; // clipboard history menu. class ASH_PUBLIC_EXPORT ClipboardHistoryController { public: + using GetHistoryValuesCallback = base::OnceCallback; + class Observer : public base::CheckedObserver { public: // Called when the clipboard history menu is shown. @@ -75,8 +79,8 @@ class ASH_PUBLIC_EXPORT ClipboardHistoryController { // Returns the history which tracks what is being copied to the clipboard. // Only the items listed in |item_id_filter| are returned. If |item_id_filter| // is empty, then all items in the history are returned. - virtual base::Value GetHistoryValues( - const std::set& item_id_filter) const = 0; + virtual void GetHistoryValues(const std::set& item_id_filter, + GetHistoryValuesCallback callback) const = 0; // Returns a list of item ids for items contained in the clipboard history. virtual std::vector GetHistoryItemIds() const = 0; diff --git a/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc b/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc index bbd940a8c01a3..80d8c2ca3681f 100644 --- a/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc +++ b/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc @@ -157,6 +157,17 @@ std::string GetKeyboardLayout() { : "qwerty"; } +// Returns a nullptr if a router could not be found or if the the router does +// not have an event listener for the given `event_name`. +extensions::EventRouter* GetRouterForEventName(content::BrowserContext* context, + const std::string& event_name) { + extensions::EventRouter* router = extensions::EventRouter::Get(context); + if (!router || !router->HasEventListener(event_name)) { + return nullptr; + } + return router; +} + } // namespace namespace extensions { @@ -354,8 +365,8 @@ void ChromeVirtualKeyboardDelegate::GetClipboardHistory( ash::ClipboardImageModelFactory::Get()->RenderCurrentPendingRequests(); } - std::move(get_history_callback) - .Run(clipboard_history_controller->GetHistoryValues(item_ids_filter)); + clipboard_history_controller->GetHistoryValues( + item_ids_filter, std::move(get_history_callback)); } bool ChromeVirtualKeyboardDelegate::PasteClipboardItem( @@ -422,11 +433,10 @@ bool ChromeVirtualKeyboardDelegate::IsSettingsEnabled() { } void ChromeVirtualKeyboardDelegate::OnClipboardHistoryItemListAddedOrRemoved() { - EventRouter* router = EventRouter::Get(browser_context_); - if (!router || !router->HasEventListener( - keyboard_api::OnClipboardHistoryChanged::kEventName)) { + EventRouter* router = GetRouterForEventName( + browser_context_, keyboard_api::OnClipboardHistoryChanged::kEventName); + if (!router) return; - } ash::ClipboardHistoryController* clipboard_history_controller = ash::ClipboardHistoryController::Get(); @@ -446,11 +456,10 @@ void ChromeVirtualKeyboardDelegate::OnClipboardHistoryItemListAddedOrRemoved() { void ChromeVirtualKeyboardDelegate::OnClipboardHistoryItemsUpdated( const std::vector& menu_item_ids) { - EventRouter* router = EventRouter::Get(browser_context_); - if (!router || !router->HasEventListener( - keyboard_api::OnClipboardItemUpdated::kEventName)) { + EventRouter* router = GetRouterForEventName( + browser_context_, keyboard_api::OnClipboardItemUpdated::kEventName); + if (!router) return; - } ash::ClipboardHistoryController* clipboard_history_controller = ash::ClipboardHistoryController::Get(); @@ -462,8 +471,19 @@ void ChromeVirtualKeyboardDelegate::OnClipboardHistoryItemsUpdated( item_ids_filter.insert(id.ToString()); } // Make call to get the updated clipboard items. - base::Value updated_items = - clipboard_history_controller->GetHistoryValues(item_ids_filter); + clipboard_history_controller->GetHistoryValues( + item_ids_filter, + base::BindOnce( + &ChromeVirtualKeyboardDelegate::OnGetHistoryValuesAfterItemsUpdated, + weak_this_)); +} + +void ChromeVirtualKeyboardDelegate::OnGetHistoryValuesAfterItemsUpdated( + base::Value updated_items) { + EventRouter* router = GetRouterForEventName( + browser_context_, keyboard_api::OnClipboardItemUpdated::kEventName); + if (!router) + return; // Broadcast an api event for each updated item. for (auto& item : updated_items.GetList()) { @@ -586,10 +606,9 @@ void ChromeVirtualKeyboardDelegate::OnHasInputDevices( void ChromeVirtualKeyboardDelegate::DispatchConfigChangeEvent( std::unique_ptr settings) { - EventRouter* router = EventRouter::Get(browser_context_); - - if (!router || !router->HasEventListener( - keyboard_api::OnKeyboardConfigChanged::kEventName)) + EventRouter* router = GetRouterForEventName( + browser_context_, keyboard_api::OnKeyboardConfigChanged::kEventName); + if (!router) return; auto event_args = std::make_unique(); diff --git a/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.h b/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.h index 87dedb19cc3ca..ef16c2c02046d 100644 --- a/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.h +++ b/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.h @@ -75,6 +75,8 @@ class ChromeVirtualKeyboardDelegate void OnClipboardHistoryItemsUpdated( const std::vector& menu_item_ids) override; + void OnGetHistoryValuesAfterItemsUpdated(base::Value updated_items); + void OnHasInputDevices(OnKeyboardSettingsCallback on_settings_callback, bool has_audio_input_devices); void DispatchConfigChangeEvent( diff --git a/remoting/host/chromeos/clipboard_aura_unittest.cc b/remoting/host/chromeos/clipboard_aura_unittest.cc index 3dc1a2e7ed5fb..62e464ab48563 100644 --- a/remoting/host/chromeos/clipboard_aura_unittest.cc +++ b/remoting/host/chromeos/clipboard_aura_unittest.cc @@ -57,8 +57,8 @@ class ClipboardAuraTest : public testing::Test { protected: void StopAndResetClipboard(); - base::test::SingleThreadTaskEnvironment task_environment_{ - base::test::SingleThreadTaskEnvironment::MainThreadType::UI}; + base::test::TaskEnvironment task_environment_{ + base::test::TaskEnvironment::MainThreadType::UI}; ClientClipboard* client_clipboard_; std::unique_ptr clipboard_; }; diff --git a/ui/base/clipboard/clipboard_data.cc b/ui/base/clipboard/clipboard_data.cc index 65bfddd5a6b54..e575eede9a8a7 100644 --- a/ui/base/clipboard/clipboard_data.cc +++ b/ui/base/clipboard/clipboard_data.cc @@ -6,15 +6,30 @@ #include #include +#include #include "base/notreached.h" +#include "base/threading/thread_restrictions.h" #include "skia/ext/skia_utils_base.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "ui/base/data_transfer_policy/data_transfer_endpoint.h" #include "ui/gfx/codec/png_codec.h" #include "ui/gfx/geometry/skia_conversions.h" +#include "ui/gfx/skia_util.h" namespace ui { +// static +std::vector ClipboardData::EncodeBitmapData(const SkBitmap& bitmap) { + // Encoding a PNG can be a long CPU operation. + base::AssertLongCPUWorkAllowed(); + + std::vector data; + gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, /*discard_transparency=*/false, + &data); + return data; +} + ClipboardData::ClipboardData() : web_smart_paste_(false), format_(0) {} ClipboardData::ClipboardData(const ClipboardData& other) { @@ -23,7 +38,8 @@ ClipboardData::ClipboardData(const ClipboardData& other) { markup_data_ = other.markup_data_; url_ = other.url_; rtf_data_ = other.rtf_data_; - png_ = other.png_; + maybe_png_ = other.maybe_png_; + maybe_bitmap_ = other.maybe_bitmap_; bookmark_title_ = other.bookmark_title_; bookmark_url_ = other.bookmark_url_; custom_data_format_ = other.custom_data_format_; @@ -40,18 +56,45 @@ ClipboardData::~ClipboardData() = default; ClipboardData::ClipboardData(ClipboardData&&) = default; bool ClipboardData::operator==(const ClipboardData& that) const { - return format_ == that.format() && text_ == that.text() && - markup_data_ == that.markup_data() && url_ == that.url() && - rtf_data_ == that.rtf_data() && - bookmark_title_ == that.bookmark_title() && - bookmark_url_ == that.bookmark_url() && - custom_data_format_ == that.custom_data_format() && - custom_data_data_ == that.custom_data_data() && - web_smart_paste_ == that.web_smart_paste() && - svg_data_ == that.svg_data() && filenames_ == that.filenames() && - png_ == that.png() && - (src_.get() ? (that.source() && *src_.get() == *that.source()) - : !that.source()); + bool equal_except_images = + format_ == that.format() && text_ == that.text() && + markup_data_ == that.markup_data() && url_ == that.url() && + rtf_data_ == that.rtf_data() && + bookmark_title_ == that.bookmark_title() && + bookmark_url_ == that.bookmark_url() && + custom_data_format_ == that.custom_data_format() && + custom_data_data_ == that.custom_data_data() && + web_smart_paste_ == that.web_smart_paste() && + svg_data_ == that.svg_data() && filenames_ == that.filenames() && + (src_.get() ? (that.source() && *src_.get() == *that.source()) + : !that.source()); + if (!equal_except_images) + return false; + + // Both instances have encoded PNGs. Compare these. + if (maybe_png_.has_value() && that.maybe_png_.has_value()) + return maybe_png_ == that.maybe_png_; + + // If only one of the these instances has a bitmap which has not yet been + // encoded as a PNG, we can't be sure that the images are equal without + // encoding the bitamp here, on the UI thread. To avoid this, just return + // false. This means that in the below scenario, a != b. + // + // ClipboardData a; + // a.SetBitmapData(image); + // + // ClipboardData b; + // b.SetPngData(EncodeBitmapData(image)); + // + // Avoid this scenario if possible. + if (maybe_bitmap_.has_value() != that.maybe_bitmap_.has_value()) + return false; + + // Both or neither instances have a bitmap. Compare the bitmaps to determine + // equality without encoding. + return !maybe_bitmap_.has_value() || + gfx::BitmapsAreEqual(maybe_bitmap_.value(), + that.maybe_bitmap_.value()); } bool ClipboardData::operator!=(const ClipboardData& that) const { @@ -76,31 +119,47 @@ absl::optional ClipboardData::size() const { total_size += bookmark_title_.size(); total_size += bookmark_url_.size(); } - if (format_ & static_cast(ClipboardInternalFormat::kPng)) - total_size += png_.size(); + if (format_ & static_cast(ClipboardInternalFormat::kPng)) { + // If there is an unencoded image, use the bitmap's size. This will be + // inaccurate by a few bytes. + if (maybe_png_.has_value()) { + total_size += maybe_png_.value().size(); + } else { + DCHECK(maybe_bitmap_.has_value()); + total_size += maybe_bitmap_.value().computeByteSize(); + } + } if (format_ & static_cast(ClipboardInternalFormat::kCustom)) total_size += custom_data_data_.size(); return total_size; } void ClipboardData::SetPngData(std::vector png) { - png_ = std::move(png); + maybe_bitmap_ = absl::nullopt; + maybe_png_ = std::move(png); format_ |= static_cast(ClipboardInternalFormat::kPng); } -SkBitmap ClipboardData::bitmap() const { - SkBitmap bitmap; - gfx::PNGCodec::Decode(png_.data(), png_.size(), &bitmap); - return bitmap; +void ClipboardData::SetPngDataAfterEncoding(std::vector png) const { + // Bitmap data can be kept around since we know it corresponds to this PNG. + // The bitmap can be used to compare two ClipboardData instances even if only + // one had been encoded to a PNG. + DCHECK(maybe_bitmap_.has_value()); + DCHECK(format_ & static_cast(ClipboardInternalFormat::kPng)); + maybe_png_ = std::move(png); } void ClipboardData::SetBitmapData(const SkBitmap& bitmap) { DCHECK_EQ(bitmap.colorType(), kN32_SkColorType); - gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, /*discard_transparency=*/false, - &png_); + maybe_bitmap_ = bitmap; + maybe_png_ = absl::nullopt; format_ |= static_cast(ClipboardInternalFormat::kPng); } +absl::optional ClipboardData::GetBitmapIfPngNotEncoded() const { + return maybe_png_.has_value() ? absl::nullopt : maybe_bitmap_; +} + void ClipboardData::SetCustomData(const std::string& data_format, const std::string& data_data) { if (data_data.size() == 0) { diff --git a/ui/base/clipboard/clipboard_data.h b/ui/base/clipboard/clipboard_data.h index 3bf22dbe16a97..bc48a8df86f2f 100644 --- a/ui/base/clipboard/clipboard_data.h +++ b/ui/base/clipboard/clipboard_data.h @@ -35,6 +35,10 @@ enum class ClipboardInternalFormat { // It mostly just provides APIs to cleanly access and manipulate this data. class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardData { public: + // Encode a bitmap to a PNG. Callers encoding a PNG on a background thread + // should use this method. + static std::vector EncodeBitmapData(const SkBitmap& bitmap); + ClipboardData(); explicit ClipboardData(const ClipboardData&); ClipboardData(ClipboardData&&); @@ -93,13 +97,35 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardData { format_ |= static_cast(ClipboardInternalFormat::kBookmark); } - const std::vector& png() const { return png_; } + // Returns an encoded PNG, or absl::nullopt if either there is no image on the + // clipboard or there is an image which has not yet been encoded to a PNG. + // `GetBitmapIfPngNotEncoded()` will return a value in the latter case. + const absl::optional>& maybe_png() const { + return maybe_png_; + } + // Set PNG data. If an existing image is already on the clipboard, its + // contents will be overwritten. If setting the PNG after encoding the bitmap + // already on the clipboard, use `SetPngDataAfterEncoding()`. void SetPngData(std::vector png); - // Bitmaps are stored as encoded bytes in the `png_` member. This means we - // cannot return a const reference, since the bitmap is created on request. - SkBitmap bitmap() const; + // Use this method if the PNG being set was encoded from the bitmap which is + // already on the clipboard. This allows the operator== method to check + // equality of two clipboard instances if only one has been encoded to a PNG. + // It is invalid to call this method unless a bitmap is already on the + // clipboard. This method is marked const to allow setting this member on + // const ClipboardData instances. + void SetPngDataAfterEncoding(std::vector png) const; + + // Prefer to use `SetPngData()` where possible. Images can be written to the + // clipboard as bitmaps, but must be read out as an encoded PNG. Callers are + // responsible for ensuring that the bitmap eventually gets encoded as a PNG. + // See GetBitmapIfPngNotEncoded() below. void SetBitmapData(const SkBitmap& bitmap); + // Use this method to obtain the bitmap to be encoded to a PNG. It is only + // recommended to call this method after checking that `maybe_png()` returns + // no value. If this returns a value, use `EncodeBitmapData()` to encode the + // bitmap to a PNG on a background thread. + absl::optional GetBitmapIfPngNotEncoded() const; const std::string& custom_data_format() const { return custom_data_format_; } const std::string& custom_data_data() const { return custom_data_data_; } @@ -140,8 +166,19 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardData { std::string bookmark_title_; std::string bookmark_url_; - // PNG image data. Bitmaps are encoded into and decoded from this member. - std::vector png_; + // Image data can take the form of PNGs or bitmaps. Strongly prefer PNGs where + // possible, since images can only be read out of this interface as PNGs. This + // field is marked as mutable so it can be set after a bitmap is encoded to a + // PNG on a const instance. The contents of the clipboard are not changing, + // merely the format. + mutable absl::optional> maybe_png_ = absl::nullopt; + // This member contains a value only in the following cases: + // 1) SetBitmapData() wrote a bitmap to the clipboard, but it has not yet been + // encoded into a PNG. + // 2) SetBitmapData() wrote a bitmap to the clipboard, then this image was + // encoded to PNG. SetPngDataAfterEncoding() was called to indicate that + // this member is the decoded version of `maybe_png_`. + absl::optional maybe_bitmap_ = absl::nullopt; // Data with custom format. std::string custom_data_format_; diff --git a/ui/base/clipboard/clipboard_data_unittest.cc b/ui/base/clipboard/clipboard_data_unittest.cc index 5e85561585030..1315a96b24500 100644 --- a/ui/base/clipboard/clipboard_data_unittest.cc +++ b/ui/base/clipboard/clipboard_data_unittest.cc @@ -8,7 +8,9 @@ #include "base/strings/string_piece_forward.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "ui/base/data_transfer_policy/data_transfer_endpoint.h" +#include "ui/gfx/image/image_unittest_util.h" #include "ui/gfx/skia_util.h" #include "url/gurl.h" @@ -18,9 +20,7 @@ namespace ui { // same bitmap. TEST(ClipboardDataTest, BitmapTest) { ClipboardData data1; - SkBitmap test_bitmap; - test_bitmap.allocN32Pixels(3, 2); - test_bitmap.eraseARGB(255, 0, 255, 0); + SkBitmap test_bitmap = gfx::test::CreateBitmap(3, 2); data1.SetBitmapData(test_bitmap); ClipboardData data2; @@ -44,15 +44,40 @@ TEST(ClipboardDataTest, DataSrcTest) { EXPECT_EQ(data1, data2); } -// Tests that encoding/decoding bitmaps as PNG bytes works as intended. -TEST(ClipboardDataTest, BitmapAsBytesTest) { +TEST(ClipboardDataTest, Equivalence) { ClipboardData data1; - SkBitmap test_bitmap; - test_bitmap.allocN32Pixels(3, 2); - test_bitmap.eraseARGB(255, 0, 255, 0); - EXPECT_FALSE(gfx::BitmapsAreEqual(data1.bitmap(), test_bitmap)); - data1.SetBitmapData(test_bitmap); - EXPECT_TRUE(gfx::BitmapsAreEqual(data1.bitmap(), test_bitmap)); + SkBitmap test_bitmap1 = gfx::test::CreateBitmap(3, 2); + data1.SetBitmapData(test_bitmap1); + + ClipboardData data2(data1); + + // Clipboards are equivalent if they have matching bitmaps. + EXPECT_EQ(data1, data2); + EXPECT_TRUE(data1.GetBitmapIfPngNotEncoded().has_value()); + EXPECT_TRUE(data2.GetBitmapIfPngNotEncoded().has_value()); + + // The PNGs have not yet been encoded. + EXPECT_EQ(data1.maybe_png(), absl::nullopt); + EXPECT_EQ(data2.maybe_png(), absl::nullopt); + + // Encode the PNG of one of the clipboards. + auto png1 = + ClipboardData::EncodeBitmapData(data1.GetBitmapIfPngNotEncoded().value()); + data1.SetPngDataAfterEncoding(png1); + + // Comparing the clipboards when only one has an encoded PNG checks the cached + // bitmap. They should still be equal. + EXPECT_EQ(data1, data2); + + // Put an already-encoded image on a clipboard. + ClipboardData data3; + data3.SetPngData(png1); + + // Comparing a clipboard when one has only an encoded PNG and the other has + // only a bitmap fails, EVEN IF the bitmap would encode into the same PNG. + EXPECT_NE(data2, data3); + // Comparing clipboards with the same encoded PNG succeeds as expected. + EXPECT_EQ(data1, data3); } } // namespace ui diff --git a/ui/base/clipboard/clipboard_non_backed.cc b/ui/base/clipboard/clipboard_non_backed.cc index 70dd16b920c62..f37d0ffe0dbc5 100644 --- a/ui/base/clipboard/clipboard_non_backed.cc +++ b/ui/base/clipboard/clipboard_non_backed.cc @@ -11,15 +11,20 @@ #include #include #include +#include +#include "base/bind.h" #include "base/check_op.h" #include "base/containers/contains.h" #include "base/files/file_path.h" #include "base/memory/ptr_util.h" +#include "base/memory/weak_ptr.h" #include "base/no_destructor.h" #include "base/notreached.h" #include "base/strings/utf_string_conversions.h" #include "base/synchronization/lock.h" +#include "base/task/thread_pool.h" +#include "base/threading/sequenced_task_runner_handle.h" #include "build/chromeos_buildflags.h" #include "skia/ext/skia_utils_base.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -31,6 +36,7 @@ #include "ui/base/clipboard/custom_data_helper.h" #include "ui/base/data_transfer_policy/data_transfer_endpoint.h" #include "ui/base/data_transfer_policy/data_transfer_policy_controller.h" +#include "ui/gfx/codec/png_codec.h" #include "ui/gfx/geometry/size.h" namespace ui { @@ -182,11 +188,37 @@ class ClipboardInternal { } // Reads png from the ClipboardData. - std::vector ReadPng() const { - if (!HasFormat(ClipboardInternalFormat::kPng)) - return std::vector(); + // TODO(crbug.com/1272798): Don't encode the same bitmap multiple times. + void ReadPng(Clipboard::ReadPngCallback callback) const { + if (!HasFormat(ClipboardInternalFormat::kPng)) { + std::move(callback).Run(std::vector()); + return; + } + + const ClipboardData* data = GetData(); + + // Check whether the clipboard contains an encoded PNG. + auto maybe_png = data->maybe_png(); + if (maybe_png.has_value()) { + std::move(callback).Run(std::move(maybe_png.value())); + return; + } - return GetData()->png(); + // Check whether the clipboard contains an image which has not yet been + // encoded to a PNG. If so, encode it on a background thread and return the + // result asynchronously. + auto maybe_bitmap = data->GetBitmapIfPngNotEncoded(); + DCHECK(maybe_bitmap.has_value()) + << "We should not be showing that PNG format is on the " + "clipboard if neither a PNG or bitmap is available."; + + png_encoding_runner_->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce(&ClipboardData::EncodeBitmapData, + std::move(maybe_bitmap.value())), + base::BindOnce(&ClipboardInternal::DidEncodePng, + weak_factory_.GetWeakPtr(), sequence_number(), + std::move(callback))); } // Reads data of type |type| from the ClipboardData. @@ -259,11 +291,30 @@ class ClipboardInternal { return data ? data->format() & static_cast(format) : false; } + void DidEncodePng(ClipboardSequenceNumberToken token, + Clipboard::ReadPngCallback callback, + std::vector png_data) { + if (token == sequence_number()) { + // Cache the encoded PNG. + data_->SetPngDataAfterEncoding(png_data); + } + + std::move(callback).Run(std::move(png_data)); + } + // Current ClipboardData. std::unique_ptr data_; // Sequence number uniquely identifying clipboard state. ClipboardSequenceNumberToken sequence_number_; + + // Runner used to asynchronously encode bitmaps into PNGs if the clipboard + // only contains a bitmap. + scoped_refptr png_encoding_runner_ = + base::ThreadPool::CreateSequencedTaskRunner( + {base::TaskPriority::USER_VISIBLE}); + + base::WeakPtrFactory weak_factory_{this}; }; // Helper class to build a ClipboardData object and write it to clipboard. @@ -584,7 +635,7 @@ void ClipboardNonBacked::ReadPng(ClipboardBuffer buffer, } RecordRead(ClipboardFormatMetric::kPng); - std::move(callback).Run(clipboard_internal_->ReadPng()); + clipboard_internal_->ReadPng(std::move(callback)); #if BUILDFLAG(IS_CHROMEOS_ASH) ClipboardMonitor::GetInstance()->NotifyClipboardDataRead(); diff --git a/ui/base/clipboard/clipboard_non_backed.h b/ui/base/clipboard/clipboard_non_backed.h index 7a9362d6939f5..6cc62456c7bf5 100644 --- a/ui/base/clipboard/clipboard_non_backed.h +++ b/ui/base/clipboard/clipboard_non_backed.h @@ -51,6 +51,7 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardNonBacked friend class Clipboard; friend class ClipboardNonBackedTest; FRIEND_TEST_ALL_PREFIXES(ClipboardNonBackedTest, TextURIList); + FRIEND_TEST_ALL_PREFIXES(ClipboardNonBackedTest, ImageEncoding); ClipboardNonBacked(); ~ClipboardNonBacked() override; diff --git a/ui/base/clipboard/clipboard_non_backed_unittest.cc b/ui/base/clipboard/clipboard_non_backed_unittest.cc index 9cba3aa94d36c..a4adf6b53cf08 100644 --- a/ui/base/clipboard/clipboard_non_backed_unittest.cc +++ b/ui/base/clipboard/clipboard_non_backed_unittest.cc @@ -9,11 +9,18 @@ #include #include "base/pickle.h" +#include "base/run_loop.h" #include "base/strings/utf_string_conversions.h" +#include "base/test/bind.h" #include "base/test/metrics/histogram_tester.h" +#include "base/test/task_environment.h" +#include "base/test/test_future.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/base/clipboard/clipboard_data.h" #include "ui/base/clipboard/custom_data_helper.h" +#include "ui/gfx/codec/png_codec.h" +#include "ui/gfx/image/image_unittest_util.h" +#include "ui/gfx/skia_util.h" namespace ui { namespace { @@ -29,13 +36,17 @@ std::vector UTF8Types(std::vector types) { class ClipboardNonBackedTest : public testing::Test { public: - ClipboardNonBackedTest() = default; + ClipboardNonBackedTest() + : task_environment_(base::test::TaskEnvironment::MainThreadType::UI) {} ClipboardNonBackedTest(const ClipboardNonBackedTest&) = delete; ClipboardNonBackedTest& operator=(const ClipboardNonBackedTest&) = delete; ~ClipboardNonBackedTest() override = default; ClipboardNonBacked* clipboard() { return &clipboard_; } + protected: + base::test::TaskEnvironment task_environment_; + private: ClipboardNonBacked clipboard_; }; @@ -145,4 +156,39 @@ TEST_F(ClipboardNonBackedTest, TextURIList) { /*data_dst=*/nullptr)); } +// Tests that bitmaps written to the clipboard are read out encoded as a PNG. +TEST_F(ClipboardNonBackedTest, ImageEncoding) { + EXPECT_EQ("image/png", ClipboardFormatType::PngType().GetName()); + + auto data = std::make_unique(); + SkBitmap test_bitmap = gfx::test::CreateBitmap(3, 2); + data->SetBitmapData(test_bitmap); + clipboard()->WriteClipboardData(std::move(data)); + + std::vector types; + clipboard()->ReadAvailableTypes(ClipboardBuffer::kCopyPaste, + /*data_dst=*/nullptr, &types); + EXPECT_EQ(std::vector({"image/png"}), UTF8Types(types)); + EXPECT_TRUE(clipboard()->IsFormatAvailable(ClipboardFormatType::PngType(), + ClipboardBuffer::kCopyPaste, + /*data_dst=*/nullptr)); + + // Asynchronously read out the image as a PNG. It should be the encoded + // version of the bitmap we wrote above. + std::vector png; + base::RunLoop loop; + clipboard()->ReadPng( + ClipboardBuffer::kCopyPaste, + /*data_dst=*/nullptr, + base::BindLambdaForTesting([&](const std::vector& png_data) { + png = png_data; + loop.Quit(); + })); + loop.Run(); + + SkBitmap bitmap; + gfx::PNGCodec::Decode(png.data(), png.size(), &bitmap); + EXPECT_TRUE(gfx::BitmapsAreEqual(bitmap, test_bitmap)); +} + } // namespace ui diff --git a/ui/views/selection_controller_unittest.cc b/ui/views/selection_controller_unittest.cc index 2e572768b4dbf..edffbde68a696 100644 --- a/ui/views/selection_controller_unittest.cc +++ b/ui/views/selection_controller_unittest.cc @@ -9,6 +9,7 @@ #include "base/memory/raw_ptr.h" #include "base/strings/utf_string_conversions.h" +#include "base/test/task_environment.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/events/event.h" #include "ui/events/event_constants.h" @@ -75,7 +76,8 @@ class SelectionControllerTest : public ::testing::Test { controller_ = std::make_unique(delegate_.get()); } - SelectionControllerTest() = default; + SelectionControllerTest() + : task_environment_(base::test::TaskEnvironment::MainThreadType::UI) {} SelectionControllerTest(const SelectionControllerTest&) = delete; SelectionControllerTest& operator=(const SelectionControllerTest&) = delete; @@ -143,6 +145,8 @@ class SelectionControllerTest : public ::testing::Test { last_event_time_, mouse_flags_, button)); } + base::test::TaskEnvironment task_environment_; + std::unique_ptr render_text_; std::unique_ptr delegate_; std::unique_ptr controller_;