Skip to content

Commit

Permalink
Remove button throbbing code (dead, replaced by InkDropHighlight)
Browse files Browse the repository at this point in the history 
hover_animation_ will also likely be removed in a future CL. Further investigation required to determine what all it will impact.

Bug: 1412217
Change-Id: I97630f24eb656871651d42b20556c6eef0e89cfe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4216105
Reviewed-by: Mike Wasserman <msw@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Joseph Park <josephjoopark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1103068}
  • Loading branch information
Joseph Park authored and Chromium LUCI CQ committed Feb 9, 2023
1 parent e3a71a9 commit 7616a4c
Show file tree
Hide file tree
Showing 13 changed files with 19 additions and 256 deletions.
1 change: 0 additions & 1 deletion chrome/browser/ui/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,6 @@ static_library("ui") {
"bluetooth/chrome_bluetooth_chooser_controller.cc",
"bluetooth/chrome_bluetooth_chooser_controller.h",
"bookmarks/bookmark_bar.h",
"bookmarks/bookmark_bubble_observer.h",
"bookmarks/bookmark_context_menu_controller.cc",
"bookmarks/bookmark_context_menu_controller.h",
"bookmarks/bookmark_drag_drop.cc",
Expand Down
23 changes: 0 additions & 23 deletions chrome/browser/ui/bookmarks/bookmark_bubble_observer.h
View file

This file was deleted.

122 changes: 3 additions & 119 deletions chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,25 +349,6 @@ END_METADATA
// BookmarkTabGroupButton
// -------------------------------------------------------

// OverflowButton (chevron) --------------------------------------------------

class OverflowButton : public BookmarkMenuButtonBase {
public:
METADATA_HEADER(OverflowButton);
OverflowButton(PressedCallback callback, BookmarkBarView* owner)
: BookmarkMenuButtonBase(std::move(callback)), owner_(owner) {}
OverflowButton(const OverflowButton&) = delete;
OverflowButton& operator=(const OverflowButton&) = delete;

bool OnMousePressed(const ui::MouseEvent& e) override {
owner_->StopThrobbing(true);
return BookmarkMenuButtonBase::OnMousePressed(e);
}

private:
raw_ptr<BookmarkBarView> owner_;
};

void RecordAppLaunch(Profile* profile, const GURL& url) {
const extensions::Extension* extension =
extensions::ExtensionRegistry::Get(profile)
Expand All @@ -380,9 +361,6 @@ void RecordAppLaunch(Profile* profile, const GURL& url) {
extension->GetType());
}

BEGIN_METADATA(OverflowButton, BookmarkMenuButtonBase)
END_METADATA

} // namespace

// DropLocation ---------------------------------------------------------------
Expand Down Expand Up @@ -629,15 +607,6 @@ views::MenuItemView* BookmarkBarView::GetDropMenu() {
return bookmark_drop_menu_ ? bookmark_drop_menu_->menu() : nullptr;
}

void BookmarkBarView::StopThrobbing(bool immediate) {
if (!throbbing_view_)
return;

// If not immediate, cycle through 2 more complete cycles.
throbbing_view_->StartThrobbing(immediate ? 0 : 4);
throbbing_view_ = nullptr;
}

// static
std::u16string BookmarkBarView::CreateToolTipForURLAndTitle(
int max_width,
Expand Down Expand Up @@ -1120,17 +1089,6 @@ void BookmarkBarView::BookmarkMenuControllerDeleted(
bookmark_drop_menu_ = nullptr;
}

void BookmarkBarView::OnBookmarkBubbleShown(const BookmarkNode* node) {
StopThrobbing(true);
if (!node)
return; // Generally shouldn't happen.
StartThrobbing(node, false);
}

void BookmarkBarView::OnBookmarkBubbleHidden() {
StopThrobbing(false);
}

void BookmarkBarView::BookmarkModelLoaded(BookmarkModel* model,
bool ids_reassigned) {
// There should be no buttons. If non-zero it means Load was invoked more than
Expand Down Expand Up @@ -1170,17 +1128,10 @@ void BookmarkBarView::BookmarkNodeMoved(BookmarkModel* model,
// mouse/touch-device, the location will update accordingly.
InvalidateDrop();

bool was_throbbing =
throbbing_view_ &&
throbbing_view_ == DetermineViewToThrobFromRemove(old_parent, old_index);
if (was_throbbing)
throbbing_view_->StopThrobbing();
bool needs_layout_and_paint =
BookmarkNodeRemovedImpl(model, old_parent, old_index);
if (BookmarkNodeAddedImpl(model, new_parent, new_index))
needs_layout_and_paint = true;
if (was_throbbing && new_index < bookmark_buttons_.size())
StartThrobbing(new_parent->children()[new_index].get(), false);
if (needs_layout_and_paint)
LayoutAndPaint();

Expand Down Expand Up @@ -1224,8 +1175,6 @@ void BookmarkBarView::BookmarkAllUserNodesRemoved(

UpdateOtherAndManagedButtonsVisibility();

StopThrobbing(true);

// Remove the existing buttons.
for (views::LabelButton* button : bookmark_buttons_) {
delete button;
Expand Down Expand Up @@ -1511,14 +1460,13 @@ std::unique_ptr<MenuButton> BookmarkBarView::CreateManagedBookmarksButton() {
}

std::unique_ptr<MenuButton> BookmarkBarView::CreateOverflowButton() {
auto button = std::make_unique<OverflowButton>(
base::BindRepeating(
auto button =
std::make_unique<BookmarkMenuButtonBase>(base::BindRepeating(
[](BookmarkBarView* bar, const ui::Event& event) {
bar->OnMenuButtonPressed(bar->bookmark_model_->bookmark_bar_node(),
event);
},
base::Unretained(this)),
this);
base::Unretained(this)));

// The overflow button's image contains an arrow and therefore it is a
// direction sensitive image and we need to flip it if the UI layout is
Expand Down Expand Up @@ -1659,10 +1607,6 @@ bool BookmarkBarView::BookmarkNodeRemovedImpl(BookmarkModel* model,
size_t index) {
const bool needs_layout = UpdateOtherAndManagedButtonsVisibility();

StopThrobbing(true);
// No need to start throbbing again as the bookmark bubble can't be up at
// the same time as the user reorders.

if (parent != model->bookmark_bar_node()) {
// Only children of the bookmark_bar_node get buttons.
return needs_layout;
Expand Down Expand Up @@ -1888,66 +1832,6 @@ void BookmarkBarView::WriteBookmarkDragData(const BookmarkNode* node,
drag_data.Write(browser_->profile()->GetPath(), data);
}

void BookmarkBarView::StartThrobbing(const BookmarkNode* node,
bool overflow_only) {
DCHECK(!throbbing_view_);

// Determine which visible button is showing the bookmark (or is an ancestor
// of the bookmark).
const BookmarkNode* bbn = bookmark_model_->bookmark_bar_node();
const BookmarkNode* parent_on_bb = node;
while (parent_on_bb) {
const BookmarkNode* parent = parent_on_bb->parent();
if (parent == bbn)
break;
parent_on_bb = parent;
}
if (parent_on_bb) {
size_t index = bbn->GetIndexOf(parent_on_bb).value();
if (index >= GetFirstHiddenNodeIndex()) {
// Node is hidden, animate the overflow button.
throbbing_view_ = overflow_button_;
} else if (!overflow_only) {
throbbing_view_ = static_cast<views::Button*>(bookmark_buttons_[index]);
}
} else if (bookmarks::IsDescendantOf(node, managed_->managed_node())) {
throbbing_view_ = managed_bookmarks_button_;
} else if (!overflow_only) {
throbbing_view_ = other_bookmarks_button_;
}

// Use a large number so that the button continues to throb.
if (throbbing_view_)
throbbing_view_->StartThrobbing(std::numeric_limits<int>::max());
}

views::Button* BookmarkBarView::DetermineViewToThrobFromRemove(
const BookmarkNode* parent,
size_t old_index) {
const BookmarkNode* bbn = bookmark_model_->bookmark_bar_node();
const BookmarkNode* old_node = parent;
size_t old_index_on_bb = old_index;
while (old_node && old_node != bbn) {
const BookmarkNode* old_parent = old_node->parent();
if (old_parent == bbn) {
old_index_on_bb = bbn->GetIndexOf(old_node).value();
break;
}
old_node = old_parent;
}
if (old_node) {
if (old_index_on_bb >= GetFirstHiddenNodeIndex()) {
// Node is hidden, animate the overflow button.
return overflow_button_;
}
return static_cast<views::Button*>(bookmark_buttons_[old_index_on_bb]);
}
if (bookmarks::IsDescendantOf(parent, managed_->managed_node()))
return managed_bookmarks_button_;
// Node wasn't on the bookmark bar, use the "Other Bookmarks" button.
return other_bookmarks_button_;
}

void BookmarkBarView::UpdateAppearanceForTheme() {
// We don't always have a color provider (ui tests, for example).
const ui::ColorProvider* color_provider = GetColorProvider();
Expand Down
32 changes: 1 addition & 31 deletions chrome/browser/ui/views/bookmarks/bookmark_bar_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "chrome/browser/ui/bookmarks/bookmark_bar.h"
#include "chrome/browser/ui/bookmarks/bookmark_bubble_observer.h"
#include "chrome/browser/ui/bookmarks/bookmark_stats.h"
#include "chrome/browser/ui/tabs/tab_group_theme.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_menu_controller_observer.h"
Expand Down Expand Up @@ -52,7 +51,6 @@ class FontList;
}

namespace views {
class Button;
class MenuButton;
class MenuItemView;
class LabelButton;
Expand All @@ -70,8 +68,7 @@ class BookmarkBarView : public views::AccessiblePaneView,
public views::ContextMenuController,
public views::DragController,
public views::AnimationDelegateViews,
public BookmarkMenuControllerObserver,
public bookmarks::BookmarkBubbleObserver {
public BookmarkMenuControllerObserver {
public:
class ButtonSeparatorView;

Expand Down Expand Up @@ -139,11 +136,6 @@ class BookmarkBarView : public views::AccessiblePaneView,
// Returns the drop MenuItemView, or NULL if a menu isn't showing.
views::MenuItemView* GetDropMenu();

// If a button is currently throbbing, it is stopped. If immediate is true
// the throb stops immediately, otherwise it stops after a couple more
// throbs.
void StopThrobbing(bool immediate);

// Returns the tooltip text for the specified url and title. The returned
// text is clipped to fit |max_tooltip_width|.
//
Expand Down Expand Up @@ -186,10 +178,6 @@ class BookmarkBarView : public views::AccessiblePaneView,
void BookmarkMenuControllerDeleted(
BookmarkMenuController* controller) override;

// bookmarks::BookmarkBubbleObserver:
void OnBookmarkBubbleShown(const bookmarks::BookmarkNode* node) override;
void OnBookmarkBubbleHidden() override;

// bookmarks::BookmarkModelObserver:
void BookmarkModelLoaded(bookmarks::BookmarkModel* model,
bool ids_reassigned) override;
Expand Down Expand Up @@ -328,19 +316,6 @@ class BookmarkBarView : public views::AccessiblePaneView,
void WriteBookmarkDragData(const bookmarks::BookmarkNode* node,
ui::OSExchangeData* data);

// This determines which view should throb and starts it
// throbbing (e.g when the bookmark bubble is showing).
// If |overflow_only| is true, start throbbing only if |node| is hidden in
// the overflow menu.
void StartThrobbing(const bookmarks::BookmarkNode* node, bool overflow_only);

// Returns the view to throb when a node is removed. |parent| is the parent of
// the node that was removed, and |old_index| the index of the node that was
// removed.
views::Button* DetermineViewToThrobFromRemove(
const bookmarks::BookmarkNode* parent,
size_t old_index);

// Sets/updates the colors and icons for all the child objects in the
// bookmarks bar.
void UpdateAppearanceForTheme();
Expand Down Expand Up @@ -445,11 +420,6 @@ class BookmarkBarView : public views::AccessiblePaneView,
// Animation controlling showing and hiding of the bar.
gfx::SlideAnimation size_animation_{this};

// If the bookmark bubble is showing, this is the visible ancestor of the URL.
// The visible ancestor is either the |other_bookmarks_button_|,
// |overflow_button_| or a button on the bar.
raw_ptr<views::Button> throbbing_view_ = nullptr;

BookmarkBar::State bookmark_bar_state_ = BookmarkBar::SHOW;

base::ObserverList<BookmarkBarViewObserver>::Unchecked observers_;
Expand Down
21 changes: 3 additions & 18 deletions chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "chrome/browser/commerce/shopping_service_factory.h"
#include "chrome/browser/platform_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/bookmarks/bookmark_bubble_observer.h"
#include "chrome/browser/ui/bookmarks/bookmark_editor.h"
#include "chrome/browser/ui/bookmarks/recently_used_folders_combo_model.h"
#include "chrome/browser/ui/browser_element_identifiers.h"
Expand Down Expand Up @@ -58,14 +57,10 @@ DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kBookmarkFolder);
class BookmarkBubbleView::BookmarkBubbleDelegate
: public ui::DialogModelDelegate {
public:
BookmarkBubbleDelegate(bookmarks::BookmarkBubbleObserver* observer,
std::unique_ptr<BubbleSyncPromoDelegate> delegate,
BookmarkBubbleDelegate(std::unique_ptr<BubbleSyncPromoDelegate> delegate,
Profile* profile,
const GURL& url)
: observer_(observer),
delegate_(std::move(delegate)),
profile_(profile),
url_(url) {}
: delegate_(std::move(delegate)), profile_(profile), url_(url) {}

void RemoveBookmark() {
base::RecordAction(UserMetricsAction("BookmarkBubble_Unstar"));
Expand All @@ -82,8 +77,6 @@ class BookmarkBubbleView::BookmarkBubbleDelegate
if (should_apply_edits_)
ApplyEdits();
bookmark_bubble_ = nullptr;
if (observer_)
observer_->OnBookmarkBubbleHidden();
}

void OnEditButton(const ui::Event& event) {
Expand Down Expand Up @@ -160,7 +153,6 @@ class BookmarkBubbleView::BookmarkBubbleDelegate
BubbleSyncPromoDelegate* delegate() { return delegate_.get(); }

private:
const raw_ptr<bookmarks::BookmarkBubbleObserver> observer_;
std::unique_ptr<BubbleSyncPromoDelegate> delegate_;
const raw_ptr<Profile> profile_;
const GURL url_;
Expand All @@ -173,7 +165,6 @@ void BookmarkBubbleView::ShowBubble(
views::View* anchor_view,
content::WebContents* web_contents,
views::Button* highlighted_button,
bookmarks::BookmarkBubbleObserver* observer,
std::unique_ptr<BubbleSyncPromoDelegate> delegate,
Profile* profile,
const GURL& url,
Expand All @@ -189,7 +180,7 @@ void BookmarkBubbleView::ShowBubble(
bookmark_model->GetMostRecentlyAddedUserNodeForURL(url);

auto bubble_delegate_unique = std::make_unique<BookmarkBubbleDelegate>(
observer, std::move(delegate), profile, url);
std::move(delegate), profile, url);
BookmarkBubbleDelegate* bubble_delegate = bubble_delegate_unique.get();

auto dialog_model_builder =
Expand Down Expand Up @@ -283,12 +274,6 @@ void BookmarkBubbleView::ShowBubble(
views::Widget* const widget =
views::BubbleDialogDelegate::CreateBubble(std::move(bubble));
widget->Show();

if (observer) {
observer->OnBookmarkBubbleShown(
BookmarkModelFactory::GetForBrowserContext(profile)
->GetMostRecentlyAddedUserNodeForURL(url));
}
}

// static
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@
class GURL;
class Profile;

namespace bookmarks {
class BookmarkBubbleObserver;
}

namespace content {
class WebContents;
} // namespace content
Expand All @@ -36,7 +32,6 @@ class BookmarkBubbleView {
static void ShowBubble(views::View* anchor_view,
content::WebContents* web_contents,
views::Button* highlighted_button,
bookmarks::BookmarkBubbleObserver* observer,
std::unique_ptr<BubbleSyncPromoDelegate> delegate,
Profile* profile,
const GURL& url,
Expand Down

0 comments on commit 7616a4c

Please sign in to comment.