[Extensions UI] Highlight toolbar extensions when the redesign bubble is active

chrome/browser/ui/views/toolbar/browser_actions_container.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
 
 #include "base/compiler_specific.h"
 #include "base/stl_util.h"
 #include "chrome/browser/extensions/extension_message_bubble_controller.h"
 #include "chrome/browser/extensions/tab_helper.h"
@@ skipping 183 matching lines @@
 
 views::MenuButton* BrowserActionsContainer::GetOverflowReferenceView() {
   // With traditional overflow, the reference is the chevron. With the
   // redesign, we use the wrench menu instead.
   return chevron_ ?
       static_cast<views::MenuButton*>(chevron_) :
       static_cast<views::MenuButton*>(BrowserView::GetBrowserViewForBrowser(
           browser_)->toolbar()->app_menu());
 }
 
+void BrowserActionsContainer::OnMouseEnteredToolbarActionView() {
+  if (!shown_bubble_ && !toolbar_action_views_.empty() &&
+      ExtensionToolbarIconSurfacingBubbleDelegate::ShouldShowForProfile(
+          browser_->profile())) {
+    ExtensionToolbarIconSurfacingBubble* bubble =
+        new ExtensionToolbarIconSurfacingBubble(
+            toolbar_action_views_[0],
+            make_scoped_ptr(new ExtensionToolbarIconSurfacingBubbleDelegate(
+                browser_->profile())));
+    views::BubbleDelegateView::CreateBubble(bubble);
+    bubble->Show();
+  }
+  shown_bubble_ = true;
+}
+
 void BrowserActionsContainer::AddViewForAction(
    ToolbarActionViewController* view_controller,
    size_t index) {
   if (chevron_)
     chevron_->CloseMenu();
 
   ToolbarActionView* view =
       new ToolbarActionView(view_controller, browser_->profile(), this);
   toolbar_action_views_.insert(toolbar_action_views_.begin() + index, view);
   AddChildViewAt(view, index);
@@ skipping 233 matching lines @@
                           index_in_row * ToolbarActionsBar::IconWidth(true),
                       row_index * ToolbarActionsBar::IconHeight(),
                       ToolbarActionsBar::IconWidth(false),
                       ToolbarActionsBar::IconHeight());
       view->SetVisible(true);
     }
   }
 }
 
 void BrowserActionsContainer::OnMouseEntered(const ui::MouseEvent& event) {
-  if (!shown_bubble_ && !toolbar_action_views_.empty() &&
-      ExtensionToolbarIconSurfacingBubbleDelegate::ShouldShowForProfile(
-          browser_->profile())) {
-    ExtensionToolbarIconSurfacingBubble* bubble =
-        new ExtensionToolbarIconSurfacingBubble(
-            toolbar_action_views_[0],
-            make_scoped_ptr(new ExtensionToolbarIconSurfacingBubbleDelegate(
-                browser_->profile())));
-    views::BubbleDelegateView::CreateBubble(bubble);
-    bubble->Show();
-  }
-  shown_bubble_ = true;
+  OnMouseEnteredToolbarActionView();
 }
 
 bool BrowserActionsContainer::GetDropFormats(
     int* formats,
     std::set<OSExchangeData::CustomFormat>* custom_formats) {
   return BrowserActionDragData::GetDropFormats(custom_formats);
 }
 
 bool BrowserActionsContainer::AreDropTypesRequired() {
   return BrowserActionDragData::AreDropTypesRequired();
@@ skipping 213 matching lines @@
   if (!web_contents)
     return NULL;
   return extensions::TabHelper::FromWebContents(web_contents)->
       active_tab_permission_granter();
 }
 
 void BrowserActionsContainer::OnPaint(gfx::Canvas* canvas) {
   // If the views haven't been initialized yet, wait for the next call to
   // paint (one will be triggered by entering highlight mode).
   if (toolbar_actions_bar_->is_highlighting() &&
-      !toolbar_action_views_.empty() &&
-      highlight_painter_) {
-    views::Painter::PaintPainterAt(
-        canvas, highlight_painter_.get(), GetLocalBounds());
+      !toolbar_action_views_.empty() && !in_overflow_mode()) {

[Peter Kasting, 2015/07/23 00:00:12]

Was this just an oversight before?  Seems like we would have wanted this
condition before this change.

[Devlin, 2015/07/23 02:43:30]

It was implicitly implied by the highlight_painter_ check, since that's only set
for non-overflow bars (in LoadImages()).

+    ToolbarActionsBar::HighlightType highlight_type =
+        toolbar_actions_bar_->GetHighlightType();
+    // The toolbar actions bar shouldn't return true for is_highlighting() with
+    // a highlight type of NONE.
+    DCHECK_NE(ToolbarActionsBar::HIGHLIGHT_NONE, highlight_type);

[Peter Kasting, 2015/07/23 00:00:12]

Nit: Personally I'd leave out this comment and DCHECK since they're pretty
obvious.

[Devlin, 2015/07/23 15:30:14]

Done.

+    views::Painter* painter =
+        highlight_type == ToolbarActionsBar::HIGHLIGHT_INFO
+            ? info_highlight_painter_.get()
+            : warning_highlight_painter_.get();
+    views::Painter::PaintPainterAt(canvas, painter, GetLocalBounds());
   }
 
   // TODO(sky/glen): Instead of using a drop indicator, animate the icons while
   // dragging (like we do for tab dragging).
   if (drop_position_.get()) {
     // The two-pixel width drop indicator.
     static const int kDropIndicatorWidth = 2;
 
     // Convert back to a pixel offset into the container.  First find the X
     // coordinate of the drop icon.
@@ skipping 60 matching lines @@
 void BrowserActionsContainer::LoadImages() {
   if (in_overflow_mode())
     return;  // Overflow mode has neither a chevron nor highlighting.
 
   ui::ThemeProvider* tp = GetThemeProvider();
   if (tp && chevron_) {
     chevron_->SetImage(views::Button::STATE_NORMAL,
                        *tp->GetImageSkiaNamed(IDR_BROWSER_ACTIONS_OVERFLOW));
   }
 
-  const int kImages[] = IMAGE_GRID(IDR_DEVELOPER_MODE_HIGHLIGHT);
-  highlight_painter_.reset(views::Painter::CreateImageGridPainter(kImages));
+  const int kInfoImages[] = IMAGE_GRID(IDR_TOOLBAR_ACTION_HIGHLIGHT);
+  info_highlight_painter_.reset(
+      views::Painter::CreateImageGridPainter(kInfoImages));
+  const int kWarningImages[] = IMAGE_GRID(IDR_DEVELOPER_MODE_HIGHLIGHT);
+  warning_highlight_painter_.reset(
+      views::Painter::CreateImageGridPainter(kWarningImages));
 }
 
 void BrowserActionsContainer::ClearActiveBubble(views::Widget* widget) {
   DCHECK(active_bubble_);
   DCHECK_EQ(active_bubble_->GetWidget(), widget);
   widget->RemoveObserver(this);
   active_bubble_ = nullptr;
 }