From f3bb4633d79e00f940472e35f1673bd1cb2d89df Mon Sep 17 00:00:00 2001 From: Max Karolinskiy Date: Tue, 3 Mar 2026 16:49:44 -0500 Subject: [PATCH] [cr147] GetClipboardText is now async. Chromium change: https://source.chromium.org/chromium/chromium/src/+/c6bd1aad8b6b959a042230f6eb6f5d1032af2884 commit c6bd1aad8b6b959a042230f6eb6f5d1032af2884 Author: Tom Anderson Date: Mon Mar 2 15:23:13 2026 -0800 Make Clipboard::ReadBookmark asynchronous This CL removes the synchronous version of Clipboard::ReadBookmark, making the asynchronous version pure virtual and implementing it in all subclasses. Callers are updated to use the asynchronous version. In particular, omnibox usages are handled by caching the clipboard state. Bug: 40398800 --- .../views/omnibox/brave_omnibox_view_views.cc | 82 +++++++++++++------ .../views/omnibox/brave_omnibox_view_views.h | 15 +++- .../brave_omnibox_view_views_browsertest.cc | 31 +++++-- 3 files changed, 93 insertions(+), 35 deletions(-) diff --git a/browser/ui/views/omnibox/brave_omnibox_view_views.cc b/browser/ui/views/omnibox/brave_omnibox_view_views.cc index a32d8b54c36..dc1d2f9d037 100644 --- a/browser/ui/views/omnibox/brave_omnibox_view_views.cc +++ b/browser/ui/views/omnibox/brave_omnibox_view_views.cc @@ -8,6 +8,7 @@ #include #include +#include "base/functional/callback.h" #include "brave/app/brave_command_ids.h" #include "brave/browser/brave_browser_features.h" #include "brave/browser/ui/browser_commands.h" @@ -39,15 +40,17 @@ void BraveUpdateContextMenu(ui::SimpleMenuModel* menu_contents, GURL url) { menu_contents->InsertItemWithStringIdAt( copy_position.value() + 1, IDC_COPY_CLEAN_LINK, IDS_COPY_CLEAN_LINK); } - -std::u16string GetClipboardText() { - return ui::Clipboard::GetForCurrentThread() - ->IsMarkedByOriginatorAsConfidential() - ? std::u16string() - : ::GetClipboardText(/*notify_if_restricted=*/false); -} } // namespace +BraveOmniboxViewViews::BraveOmniboxViewViews(bool popup_window_mode, + OmniboxController* controller, + LocationBarView* location_bar_view, + const gfx::FontList& font_list) + : OmniboxViewViews(popup_window_mode, + controller, + location_bar_view, + font_list) {} + BraveOmniboxViewViews::~BraveOmniboxViewViews() = default; std::optional BraveOmniboxViewViews::GetURLToCopy() { @@ -144,26 +147,38 @@ void BraveOmniboxViewViews::ExecuteCommand(int command_id, int event_flags) { // OmniboxEditModel::PasteAndGo(). In OmniboxEditModel, only normal // profile's search provider is used because same AutocompleteClassifier is // shared between normal and private profile. - if (command_id != IDC_PASTE_AND_GO) { + // Also, early return if |location_bar_view_| is null as it would be needed to + // get Browser instance pointer. It could be null in unit test. + if (command_id != IDC_PASTE_AND_GO || !location_bar_view_) { return OmniboxViewViews::ExecuteCommand(command_id, event_flags); } - // Early return if |location_bar_view_| is null as it's used to get Browser - // instance pointer. It could be null in unit test. - auto clipboard_text = GetClipboardTextForPasteAndSearch(); - if (!location_bar_view_ || !clipboard_text) { - return OmniboxViewViews::ExecuteCommand(command_id, event_flags); + GetClipboardTextForPasteAndSearch(base::BindOnce( + &BraveOmniboxViewViews::OnClipboardTextForPasteAndSearchRetrieved, + weak_factory_.GetWeakPtr(), command_id, event_flags)); +} + +void BraveOmniboxViewViews::OnClipboardTextForPasteAndSearchRetrieved( + int command_id, + int event_flags, + std::u16string clipboard_text) { + DCHECK(location_bar_view_); + + if (clipboard_text.empty()) { + OmniboxViewViews::ExecuteCommand(command_id, event_flags); + return; } constexpr size_t kMaxSelectionTextLength = 50; std::u16string selection_text = gfx::TruncateString( - *clipboard_text, kMaxSelectionTextLength, gfx::WORD_BREAK); + clipboard_text, kMaxSelectionTextLength, gfx::WORD_BREAK); const auto* service = controller()->client()->GetTemplateURLService(); const auto url = service->GenerateSearchURLForDefaultSearchProvider(selection_text); if (!url.is_valid()) { - return OmniboxViewViews::ExecuteCommand(command_id, event_flags); + OmniboxViewViews::ExecuteCommand(command_id, event_flags); + return; } NavigateParams params(location_bar_view_->browser(), url, @@ -172,20 +187,33 @@ void BraveOmniboxViewViews::ExecuteCommand(int command_id, int event_flags) { Navigate(¶ms); } -std::optional -BraveOmniboxViewViews::GetClipboardTextForPasteAndSearch() { - std::u16string clipboard_text = GetClipboardText(); - if (clipboard_text.empty()) { - return std::nullopt; +void BraveOmniboxViewViews::GetClipboardTextForPasteAndSearch( + base::OnceCallback callback) { + if (ui::Clipboard::GetForCurrentThread() + ->IsMarkedByOriginatorAsConfidential()) { + std::move(callback).Run(std::u16string()); + return; } - AutocompleteMatch match; - controller()->edit_model()->ClassifyString(clipboard_text, &match, nullptr); - if (!AutocompleteMatch::IsSearchType(match.type)) { - return std::nullopt; - } - - return clipboard_text; + GetClipboardText( + /*notify_if_restricted=*/false, + base::BindOnce( + [](base::WeakPtr self, + base::OnceCallback callback, + std::u16string clipboard_text) { + if (self) { + if (!clipboard_text.empty()) { + AutocompleteMatch match; + self->controller()->edit_model()->ClassifyString( + clipboard_text, &match, nullptr); + if (!AutocompleteMatch::IsSearchType(match.type)) { + clipboard_text = std::u16string(); + } + } + std::move(callback).Run(std::move(clipboard_text)); + } + }, + weak_factory_.GetWeakPtr(), std::move(callback))); } void BraveOmniboxViewViews::UpdateContextMenu( diff --git a/browser/ui/views/omnibox/brave_omnibox_view_views.h b/browser/ui/views/omnibox/brave_omnibox_view_views.h index 7f22fd7a2ca..23f12c38487 100644 --- a/browser/ui/views/omnibox/brave_omnibox_view_views.h +++ b/browser/ui/views/omnibox/brave_omnibox_view_views.h @@ -8,7 +8,9 @@ #include +#include "base/functional/callback_forward.h" #include "base/gtest_prod_util.h" +#include "base/memory/weak_ptr.h" #include "chrome/browser/ui/views/omnibox/omnibox_view_views.h" #include "ui/base/metadata/metadata_header_macros.h" @@ -18,7 +20,10 @@ class BraveOmniboxViewViews : public OmniboxViewViews { METADATA_HEADER(BraveOmniboxViewViews, OmniboxViewViews) public: - using OmniboxViewViews::OmniboxViewViews; + BraveOmniboxViewViews(bool popup_window_mode, + OmniboxController* controller, + LocationBarView* location_bar_view, + const gfx::FontList& font_list); BraveOmniboxViewViews(const BraveOmniboxViewViews&) = delete; BraveOmniboxViewViews& operator=(const BraveOmniboxViewViews&) = delete; @@ -49,7 +54,13 @@ class BraveOmniboxViewViews : public OmniboxViewViews { private: FRIEND_TEST_ALL_PREFIXES(BraveOmniboxViewViewsTest, PasteAndSearchTest); - std::optional GetClipboardTextForPasteAndSearch(); + void GetClipboardTextForPasteAndSearch( + base::OnceCallback callback); + void OnClipboardTextForPasteAndSearchRetrieved(int command_id, + int event_flags, + std::u16string clipboard_text); + + base::WeakPtrFactory weak_factory_{this}; }; #endif // BRAVE_BROWSER_UI_VIEWS_OMNIBOX_BRAVE_OMNIBOX_VIEW_VIEWS_H_ diff --git a/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc b/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc index 821bfaad10a..05748853fea 100644 --- a/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc +++ b/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc @@ -6,6 +6,7 @@ #include "brave/browser/ui/views/omnibox/brave_omnibox_view_views.h" #include "base/strings/utf_string_conversions.h" +#include "base/test/test_future.h" #include "brave/browser/brave_browser_features.h" #include "brave/browser/url_sanitizer/url_sanitizer_service_factory.h" #include "brave/components/url_sanitizer/core/browser/url_sanitizer_service.h" @@ -24,6 +25,7 @@ #include "components/search_engines/template_url_service.h" #include "content/public/test/browser_test.h" #include "content/public/test/browser_test_utils.h" +#include "content/public/test/test_navigation_observer.h" #include "ui/base/clipboard/clipboard.h" #include "ui/base/clipboard/scoped_clipboard_writer.h" #include "ui/base/clipboard/test/clipboard_test_util.h" @@ -108,7 +110,12 @@ IN_PROC_BROWSER_TEST_F(BraveOmniboxViewViewsTest, PasteAndSearchTest) { auto* brave_omnibox_view = static_cast(omnibox_view()); SetClipboardText(ui::ClipboardBuffer::kCopyPaste, u"Brave browser"); - EXPECT_TRUE(brave_omnibox_view->GetClipboardTextForPasteAndSearch()); + { + base::test::TestFuture future; + brave_omnibox_view->GetClipboardTextForPasteAndSearch(future.GetCallback()); + auto clipboard_text = future.Take(); + EXPECT_NE(std::u16string(), clipboard_text); + } auto* service = TemplateURLServiceFactory::GetForProfile(browser()->profile()); @@ -123,10 +130,13 @@ IN_PROC_BROWSER_TEST_F(BraveOmniboxViewViewsTest, PasteAndSearchTest) { service->SetUserSelectedDefaultSearchProvider(test_url.get()); // Paste and search for normal window. - brave_omnibox_view->ExecuteCommand(IDC_PASTE_AND_GO, ui::EF_NONE); TabStripModel* tab_strip = browser()->tab_strip_model(); auto* active_web_contents = tab_strip->GetActiveWebContents(); - content::WaitForLoadStop(active_web_contents); + { + content::TestNavigationObserver observer(active_web_contents); + brave_omnibox_view->ExecuteCommand(IDC_PASTE_AND_GO, ui::EF_NONE); + observer.Wait(); + } // Check loaded url's host and search provider's url host are same in normal // window. @@ -154,13 +164,22 @@ IN_PROC_BROWSER_TEST_F(BraveOmniboxViewViewsTest, PasteAndSearchTest) { private_browser_view->toolbar()->location_bar_view()->omnibox_view()); SetClipboardText(ui::ClipboardBuffer::kCopyPaste, u"Brave browser"); - EXPECT_TRUE(private_brave_omnibox_view->GetClipboardTextForPasteAndSearch()); + { + base::test::TestFuture future; + private_brave_omnibox_view->GetClipboardTextForPasteAndSearch( + future.GetCallback()); + auto clipboard_text = future.Take(); + EXPECT_NE(std::u16string(), clipboard_text); + } // Paste and search for private window - private_brave_omnibox_view->ExecuteCommand(IDC_PASTE_AND_GO, ui::EF_NONE); TabStripModel* private_tab_strip = private_browser->tab_strip_model(); auto* private_active_web_contents = private_tab_strip->GetActiveWebContents(); - content::WaitForLoadStop(private_active_web_contents); + { + content::TestNavigationObserver observer(private_active_web_contents); + private_brave_omnibox_view->ExecuteCommand(IDC_PASTE_AND_GO, ui::EF_NONE); + observer.Wait(); + } // Check loaded url's host and search provider's url host are same in private // window.