Commit 294151f2 authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

Reland r786979, part 2: add hooks to ToolbarAccountIconContainer.

The original CL was landed in one piece and reverted due to apparent
perf regressions.  Reland in sections to try and determine more
precisely whether there is any perf impact.

This CL relands the pieces that will be necessary to use callbacks
instead of ButtonObserver in ToolbarAccountIconContainer, but does not
actually switch to using them yet.

If this causes regressions, then something about the memory overhead of
the subscription list, or the cost of Button::GetHighlighted(), is the
issue.

Bug: none
Change-Id: I34921876c23579c01ca378ad4fc2ae6fee9042fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347312Reviewed-by: default avatarAllen Bauer <kylixrd@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796616}
parent db8db018
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <memory> #include <memory>
#include "base/bind_helpers.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "chrome/browser/themes/theme_properties.h" #include "chrome/browser/themes/theme_properties.h"
#include "chrome/browser/ui/layout_constants.h" #include "chrome/browser/ui/layout_constants.h"
...@@ -97,6 +98,12 @@ void ToolbarIconContainerView::AddMainButton(views::Button* main_button) { ...@@ -97,6 +98,12 @@ void ToolbarIconContainerView::AddMainButton(views::Button* main_button) {
} }
void ToolbarIconContainerView::ObserveButton(views::Button* button) { void ToolbarIconContainerView::ObserveButton(views::Button* button) {
// We don't care about the main button being highlighted.
if (button != main_button_) {
subscriptions_.push_back(
button->AddHighlightedChangedCallback(base::DoNothing()));
}
subscriptions_.push_back(button->AddStateChangedCallback(base::DoNothing()));
button->AddButtonObserver(this); button->AddButtonObserver(this);
button->AddObserver(this); button->AddObserver(this);
} }
...@@ -132,12 +139,7 @@ void ToolbarIconContainerView::OnHighlightChanged( ...@@ -132,12 +139,7 @@ void ToolbarIconContainerView::OnHighlightChanged(
if (observed_button == main_button_) if (observed_button == main_button_)
return; return;
if (highlighted) OnButtonHighlightedChanged(observed_button);
highlighted_buttons_.insert(observed_button);
else
highlighted_buttons_.erase(observed_button);
UpdateHighlight();
} }
void ToolbarIconContainerView::OnStateChanged( void ToolbarIconContainerView::OnStateChanged(
...@@ -245,3 +247,13 @@ void ToolbarIconContainerView::SetHighlightBorder() { ...@@ -245,3 +247,13 @@ void ToolbarIconContainerView::SetHighlightBorder() {
SetBorder(nullptr); SetBorder(nullptr);
} }
} }
void ToolbarIconContainerView::OnButtonHighlightedChanged(
views::Button* button) {
if (button->GetHighlighted())
highlighted_buttons_.insert(button);
else
highlighted_buttons_.erase(button);
UpdateHighlight();
}
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_UI_VIEWS_TOOLBAR_TOOLBAR_ICON_CONTAINER_VIEW_H_ #ifndef CHROME_BROWSER_UI_VIEWS_TOOLBAR_TOOLBAR_ICON_CONTAINER_VIEW_H_
#define CHROME_BROWSER_UI_VIEWS_TOOLBAR_TOOLBAR_ICON_CONTAINER_VIEW_H_ #define CHROME_BROWSER_UI_VIEWS_TOOLBAR_TOOLBAR_ICON_CONTAINER_VIEW_H_
#include <list>
#include "base/observer_list.h" #include "base/observer_list.h"
#include "ui/gfx/animation/animation_delegate.h" #include "ui/gfx/animation/animation_delegate.h"
#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/button.h"
...@@ -91,6 +93,9 @@ class ToolbarIconContainerView : public views::View, ...@@ -91,6 +93,9 @@ class ToolbarIconContainerView : public views::View,
void UpdateHighlight(); void UpdateHighlight();
void SetHighlightBorder(); void SetHighlightBorder();
// Called by |button| when its ink drop highlighted state changes.
void OnButtonHighlightedChanged(views::Button* button);
// Determine whether the container shows its highlight border. // Determine whether the container shows its highlight border.
const bool uses_highlight_; const bool uses_highlight_;
...@@ -113,6 +118,8 @@ class ToolbarIconContainerView : public views::View, ...@@ -113,6 +118,8 @@ class ToolbarIconContainerView : public views::View,
// Tracks when the widget is restored and resets the layout. // Tracks when the widget is restored and resets the layout.
std::unique_ptr<WidgetRestoreObserver> restore_observer_; std::unique_ptr<WidgetRestoreObserver> restore_observer_;
std::list<views::PropertyChangedSubscription> subscriptions_;
base::ObserverList<Observer> observers_; base::ObserverList<Observer> observers_;
}; };
......
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