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.