Commit 3c98368e authored by Sadrul Habib Chowdhury's avatar Sadrul Habib Chowdhury Committed by Commit Bot

[code health] Remove NOTIFICATION_BOOKMARK_CONTEXT_MENU_SHOWN

NOTIFICATION_BOOKMARK_CONTEXT_MENU_SHOWN is used to schedule a callback
right before the bookmark context menu is run. This is used only in
interactive tests.  Instead of using this notification, use a more
explicit API to register callbacks to run before running the context
menu.

BUG=268984

Change-Id: I43c78118d6fd7ef53f0b7e7c70aefc85670cf78b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1831275
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701434}
parent 5e8a48c4
...@@ -198,10 +198,6 @@ enum NotificationType { ...@@ -198,10 +198,6 @@ enum NotificationType {
#endif #endif
#if defined(TOOLKIT_VIEWS) #if defined(TOOLKIT_VIEWS)
// Sent when a bookmark's context menu is shown. Used to notify
// tests that the context menu has been created and shown.
NOTIFICATION_BOOKMARK_CONTEXT_MENU_SHOWN,
// Notification that the nested loop using during tab dragging has returned. // Notification that the nested loop using during tab dragging has returned.
// Used for testing. // Used for testing.
NOTIFICATION_TAB_DRAG_LOOP_DONE, NOTIFICATION_TAB_DRAG_LOOP_DONE,
......
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
#include "chrome/app/chrome_command_ids.h" #include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/chrome_content_browser_client.h" #include "chrome/browser/chrome_content_browser_client.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/bookmarks/bookmark_utils.h" #include "chrome/browser/ui/bookmarks/bookmark_utils.h"
#include "chrome/browser/ui/bookmarks/bookmark_utils_desktop.h" #include "chrome/browser/ui/bookmarks/bookmark_utils_desktop.h"
...@@ -26,6 +25,7 @@ ...@@ -26,6 +25,7 @@
#include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view_observer.h" #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view_observer.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_context_menu.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_menu_controller_views.h" #include "chrome/browser/ui/views/bookmarks/bookmark_menu_controller_views.h"
#include "chrome/browser/ui/views/chrome_constrained_window_views_client.h" #include "chrome/browser/ui/views/chrome_constrained_window_views_client.h"
#include "chrome/browser/ui/views/chrome_layout_provider.h" #include "chrome/browser/ui/views/chrome_layout_provider.h"
...@@ -42,7 +42,6 @@ ...@@ -42,7 +42,6 @@
#include "components/bookmarks/test/bookmark_test_helpers.h" #include "components/bookmarks/test/bookmark_test_helpers.h"
#include "components/constrained_window/constrained_window_views.h" #include "components/constrained_window/constrained_window_views.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/page_navigator.h" #include "content/public/browser/page_navigator.h"
#include "content/public/test/test_browser_thread.h" #include "content/public/test/test_browser_thread.h"
#include "ui/base/clipboard/clipboard.h" #include "ui/base/clipboard/clipboard.h"
...@@ -721,25 +720,21 @@ VIEW_TEST(BookmarkBarViewTest3, Submenus) ...@@ -721,25 +720,21 @@ VIEW_TEST(BookmarkBarViewTest3, Submenus)
// which invokes the event loop. // which invokes the event loop.
// Because |task| is a OnceClosure, callers should use a separate observer // Because |task| is a OnceClosure, callers should use a separate observer
// instance for each successive context menu creation they wish to observe. // instance for each successive context menu creation they wish to observe.
class BookmarkContextMenuNotificationObserver class BookmarkContextMenuNotificationObserver {
: public content::NotificationObserver {
public: public:
explicit BookmarkContextMenuNotificationObserver(base::OnceClosure task) explicit BookmarkContextMenuNotificationObserver(base::OnceClosure task)
: task_(std::move(task)) { : task_(std::move(task)) {
registrar_.Add(this, BookmarkContextMenu::InstallPreRunCallback(base::BindOnce(
chrome::NOTIFICATION_BOOKMARK_CONTEXT_MENU_SHOWN, &BookmarkContextMenuNotificationObserver::ScheduleCallback,
content::NotificationService::AllSources()); base::Unretained(this)));
} }
void Observe(int type, void ScheduleCallback() {
const content::NotificationSource& source,
const content::NotificationDetails& details) override {
DCHECK(!task_.is_null()); DCHECK(!task_.is_null());
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, std::move(task_)); base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, std::move(task_));
} }
private: private:
content::NotificationRegistrar registrar_;
base::OnceClosure task_; base::OnceClosure task_;
DISALLOW_COPY_AND_ASSIGN(BookmarkContextMenuNotificationObserver); DISALLOW_COPY_AND_ASSIGN(BookmarkContextMenuNotificationObserver);
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/i18n/rtl.h" #include "base/i18n/rtl.h"
#include "base/lazy_instance.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/app/chrome_command_ids.h" #include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chrome_notification_types.h"
...@@ -22,6 +23,9 @@ using content::PageNavigator; ...@@ -22,6 +23,9 @@ using content::PageNavigator;
namespace { namespace {
base::LazyInstance<base::OnceClosure>::Leaky pre_run_callback =
LAZY_INSTANCE_INITIALIZER;
// Returns true if |command_id| corresponds to a command that causes one or more // Returns true if |command_id| corresponds to a command that causes one or more
// bookmarks to be removed. // bookmarks to be removed.
bool IsRemoveBookmarksCommand(int command_id) { bool IsRemoveBookmarksCommand(int command_id) {
...@@ -69,15 +73,19 @@ BookmarkContextMenu::BookmarkContextMenu( ...@@ -69,15 +73,19 @@ BookmarkContextMenu::BookmarkContextMenu(
BookmarkContextMenu::~BookmarkContextMenu() { BookmarkContextMenu::~BookmarkContextMenu() {
} }
void BookmarkContextMenu::InstallPreRunCallback(base::OnceClosure callback) {
DCHECK(pre_run_callback.Get().is_null());
pre_run_callback.Get() = std::move(callback);
}
void BookmarkContextMenu::RunMenuAt(const gfx::Point& point, void BookmarkContextMenu::RunMenuAt(const gfx::Point& point,
ui::MenuSourceType source_type) { ui::MenuSourceType source_type) {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode)) if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode))
return; return;
content::NotificationService::current()->Notify( if (!pre_run_callback.Get().is_null())
chrome::NOTIFICATION_BOOKMARK_CONTEXT_MENU_SHOWN, std::move(pre_run_callback.Get()).Run();
content::Source<BookmarkContextMenu>(this),
content::NotificationService::NoDetails());
// width/height don't matter here. // width/height don't matter here.
menu_runner_->RunMenuAt(parent_widget_, nullptr, menu_runner_->RunMenuAt(parent_widget_, nullptr,
gfx::Rect(point.x(), point.y(), 0, 0), gfx::Rect(point.x(), point.y(), 0, 0),
......
...@@ -50,6 +50,11 @@ class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate, ...@@ -50,6 +50,11 @@ class BookmarkContextMenu : public BookmarkContextMenuControllerDelegate,
bool close_on_remove); bool close_on_remove);
~BookmarkContextMenu() override; ~BookmarkContextMenu() override;
// Installs a callback to be run before the context menu is run. The callback
// runs only once, and only one such callback can be set at any time. Once the
// installed callback is run, another callback can be installed.
static void InstallPreRunCallback(base::OnceClosure callback);
// Shows the context menu at the specified point. // Shows the context menu at the specified point.
void RunMenuAt(const gfx::Point& point, void RunMenuAt(const gfx::Point& point,
ui::MenuSourceType source_type); ui::MenuSourceType source_type);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment