Allow the user to drag an extension to the overflow menu, even when its empty

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_action_manager.h"
 #include "chrome/browser/extensions/extension_util.h"
@@ skipping 250 matching lines @@
   // Global commands are handled by the ExtensionCommandsGlobalRegistry
   // instance.
   DCHECK(!command.global());
   extension_keybinding_registry_->ExecuteCommand(extension->id(),
                                                  command.accelerator());
 }
 
 void BrowserActionsContainer::NotifyActionMovedToOverflow() {
   // When an action is moved to overflow, we shrink the size of the container
   // by 1.
-  if (!profile_->IsOffTheRecord())
-    model_->SetVisibleIconCount(model_->GetVisibleIconCount() - 1);
+  if (!profile_->IsOffTheRecord()) {
+    int icon_count = model_->GetVisibleIconCount();
+    // Since this happens when an icon moves from the main bar to overflow, we
+    // can't possibly have had no visible icons on the main bar.
+    DCHECK_NE(0, icon_count);
+    if (icon_count == -1)
+      icon_count = browser_action_views_.size();
+    model_->SetVisibleIconCount(icon_count - 1);
+  }
   Animate(gfx::Tween::EASE_OUT,
           VisibleBrowserActionsAfterAnimation() - 1);
 }
 
 bool BrowserActionsContainer::ShownInsideMenu() const {
   return in_overflow_mode();
 }
 
 void BrowserActionsContainer::OnBrowserActionViewDragDone() {
   ToolbarVisibleCountChanged();
@@ skipping 32 matching lines @@
     BrowserActionsContainerObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void BrowserActionsContainer::RemoveObserver(
     BrowserActionsContainerObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
 gfx::Size BrowserActionsContainer::GetPreferredSize() const {
-  // Note: We can't use GetIconCount() for the main bar, since we may also
-  // have to include items that are in the chevron's overflow.
-  size_t icon_count =
-      in_overflow_mode() ? GetIconCount() : browser_action_views_.size();
-
-  // If there are no actions to show, or we are in overflow mode and the main
-  // container is already showing them all, then no further work is required.
-  if (icon_count == 0)
-    return gfx::Size();
-
   if (in_overflow_mode()) {
-    // When in overflow, y is multiline, so the pixel count is IconHeight()
-    // times the number of rows needed.
+    int icon_count = GetIconCount();
+    // In overflow, we always have a preferred size of a full row (even if we
+    // don't use it), and always of at least one row. The parent will determine
+    // if the container shouldn't be shown.

[Peter Kasting, 2014/09/16 22:15:57]

Nit: How about something like:

...and always of at least one row, even when we're empty.  The parent may decide
to show us even when empty, e.g. as a drag target for dragging in icons from the
main container.

[Devlin, 2014/09/16 22:25:37]

Done.

+    int row_count =
+        ((std::max(0, icon_count - 1)) / icons_per_overflow_menu_row_) + 1;
     return gfx::Size(
         IconCountToWidth(icons_per_overflow_menu_row_, false),
-        (((icon_count - 1) / icons_per_overflow_menu_row_) + 1) * IconHeight());
+        row_count * IconHeight());
   }
 
+  // If there are no actions to show, then don't show the container at all.
+  if (browser_action_views_.empty())
+    return gfx::Size();
+
   // We calculate the size of the view by taking the current width and
   // subtracting resize_amount_ (the latter represents how far the user is
   // resizing the view or, if animating the snapping, how far to animate it).
   // But we also clamp it to a minimum size and the maximum size, so that the
   // container can never shrink too far or take up more space than it needs.
   // In other words: MinimumNonemptyWidth() < width() - resize < ClampTo(MAX).
   int preferred_width = std::min(
       std::max(MinimumNonemptyWidth(), container_width_ - resize_amount_),
       IconCountToWidth(-1, false));
   return gfx::Size(preferred_width, IconHeight());
@@ skipping 88 matching lines @@
 int BrowserActionsContainer::OnDragUpdated(
     const ui::DropTargetEvent& event) {
   // First check if we are above the chevron (overflow) menu.
   if (chevron_ && GetEventHandlerForPoint(event.location()) == chevron_) {
     if (!show_menu_task_factory_.HasWeakPtrs() && !overflow_menu_)
       StartShowFolderDropMenuTimer();
     return ui::DragDropTypes::DRAG_MOVE;
   }
   StopShowFolderDropMenuTimer();
 
-  // Figure out where to display the indicator.  This is a complex calculation:
+  size_t row_index = 0;
+  size_t before_icon_in_row = 0;
+  // If there are no visible browser actions (such as when dragging an icon to
+  // an empty overflow/main container), then 0, 0 for row, column is correct.
+  if (VisibleBrowserActions() != 0) {
+    // Figure out where to display the indicator. This is a complex calculation:
 
-  // First, we figure out how much space is to the left of the icon area, so we
-  // can calculate the true offset into the icon area. The easiest way to do
-  // this is to just find where the first icon starts.
-  int width_before_icons =
-      browser_action_views_[GetFirstVisibleIconIndex()]->x();
+    // First, we subtract out the padding to the left of the icon area, which is
+    // ToolbarView::kStandardSpacing. If we're right-to-left, we also mirror the
+    // event.x() so that our calculations are consistent with left-to-right.
+    int offset_into_icon_area =
+        GetMirroredXInView(event.x()) - ToolbarView::kStandardSpacing;
 
-  // If we're right-to-left, we flip the mirror the event.x() so that our
-  // calculations are consistent with left-to-right.
-  int offset_into_icon_area =
-      GetMirroredXInView(event.x()) - width_before_icons;
+    // Next, figure out what row we're on. This only matters for overflow mode,
+    // but the calculation is the same for both.
+    row_index = event.y() / IconHeight();
 
-  // Next, figure out what row we're on. This only matters for overflow mode,
-  // but the calculation is the same for both.
-  size_t row_index = event.y() / IconHeight();
+    // Sanity check - we should never be on a different row in the main
+    // container.
+    DCHECK(in_overflow_mode() || row_index == 0);
 
-  // Sanity check - we should never be on a different row in the main container.
-  DCHECK(in_overflow_mode() || row_index == 0);
+    // Next, we determine which icon to place the indicator in front of. We want
+    // to place the indicator in front of icon n when the cursor is between the
+    // midpoints of icons (n - 1) and n.  To do this we take the offset into the
+    // icon area and transform it as follows:
+    //
+    // Real icon area:
+    //   0   a     *  b        c
+    //   |   |        |        |
+    //   |[IC|ON]  [IC|ON]  [IC|ON]
+    // We want to be before icon 0 for 0 < x <= a, icon 1 for a < x <= b, etc.
+    // Here the "*" represents the offset into the icon area, and since it's
+    // between a and b, we want to return "1".
+    //
+    // Transformed "icon area":
+    //   0        a     *  b        c
+    //   |        |        |        |
+    //   |[ICON]  |[ICON]  |[ICON]  |
+    // If we shift both our offset and our divider points later by half an icon
+    // plus one spacing unit, then it becomes very easy to calculate how many
+    // divider points we've passed, because they're the multiples of "one icon
+    // plus padding".
+    int before_icon_unclamped =
+        (offset_into_icon_area + (IconWidth(false) / 2) +
+        kItemSpacing) / IconWidth(true);
 
-  // Next, we determine which icon to place the indicator in front of.  We want
-  // to place the indicator in front of icon n when the cursor is between the
-  // midpoints of icons (n - 1) and n.  To do this we take the offset into the
-  // icon area and transform it as follows:
-  //
-  // Real icon area:
-  //   0   a     *  b        c
-  //   |   |        |        |
-  //   |[IC|ON]  [IC|ON]  [IC|ON]
-  // We want to be before icon 0 for 0 < x <= a, icon 1 for a < x <= b, etc.
-  // Here the "*" represents the offset into the icon area, and since it's
-  // between a and b, we want to return "1".
-  //
-  // Transformed "icon area":
-  //   0        a     *  b        c
-  //   |        |        |        |
-  //   |[ICON]  |[ICON]  |[ICON]  |
-  // If we shift both our offset and our divider points later by half an icon
-  // plus one spacing unit, then it becomes very easy to calculate how many
-  // divider points we've passed, because they're the multiples of "one icon
-  // plus padding".
-  int before_icon_unclamped = (offset_into_icon_area + (IconWidth(false) / 2) +
-      kItemSpacing) / IconWidth(true);
+    // We need to figure out how many icons are visible on the relevant row.
+    // In the main container, this will just be the visible actions.
+    int visible_icons_on_row = VisibleBrowserActionsAfterAnimation();
+    if (in_overflow_mode()) {
+      // If this is the final row of the overflow, then this is the remainder of
+      // visible icons. Otherwise, it's a full row (kIconsPerRow).
+      visible_icons_on_row =
+          row_index ==
+              static_cast<size_t>(visible_icons_on_row /
+                                  icons_per_overflow_menu_row_) ?
+                  visible_icons_on_row % icons_per_overflow_menu_row_ :
+                  icons_per_overflow_menu_row_;
+    }
 
-  // We need to figure out how many icons are visible on the relevant row.
-  // In the main container, this will just be the visible actions.
-  int visible_icons_on_row = VisibleBrowserActionsAfterAnimation();
-  if (in_overflow_mode()) {
-    // If this is the final row of the overflow, then this is the remainder of
-    // visible icons. Otherwise, it's a full row (kIconsPerRow).
-    visible_icons_on_row =
-        row_index ==
-            static_cast<size_t>(visible_icons_on_row /
-                                icons_per_overflow_menu_row_) ?
-                visible_icons_on_row % icons_per_overflow_menu_row_ :
-                icons_per_overflow_menu_row_;
+    // Because the user can drag outside the container bounds, we need to clamp
+    // to the valid range. Note that the maximum allowable value is (num icons),
+    // not (num icons - 1), because we represent the indicator being past the
+    // last icon as being "before the (last + 1) icon".
+    before_icon_in_row =
+        std::min(std::max(before_icon_unclamped, 0), visible_icons_on_row);
   }
 
-  // Because the user can drag outside the container bounds, we need to clamp to
-  // the valid range.  Note that the maximum allowable value is (num icons), not
-  // (num icons - 1), because we represent the indicator being past the last
-  // icon as being "before the (last + 1) icon".
-  size_t before_icon_in_row =
-      std::min(std::max(before_icon_unclamped, 0), visible_icons_on_row);
-
   if (!drop_position_.get() ||
       !(drop_position_->row == row_index &&
         drop_position_->icon_in_row == before_icon_in_row)) {
     drop_position_.reset(new DropPosition(row_index, before_icon_in_row));
     SchedulePaint();
   }
 
   return ui::DragDropTypes::DRAG_MOVE;
 }
 
@@ skipping 10 matching lines @@
     return ui::DragDropTypes::DRAG_NONE;
 
   // Make sure we have the same view as we started with.
   DCHECK_EQ(browser_action_views_[data.index()]->extension()->id(),
             data.id());
   DCHECK(model_);
 
   size_t i = drop_position_->row * icons_per_overflow_menu_row_ +
              drop_position_->icon_in_row;
   if (in_overflow_mode())
-    i += GetFirstVisibleIconIndex();
+    i += main_container_->VisibleBrowserActionsAfterAnimation();
   // |i| now points to the item to the right of the drop indicator*, which is
   // correct when dragging an icon to the left. When dragging to the right,
   // however, we want the icon being dragged to get the index of the item to
   // the left of the drop indicator, so we subtract one.
   // * Well, it can also point to the end, but not when dragging to the left. :)
   if (i > data.index())
     --i;
 
   if (profile_->IsOffTheRecord())
     i = model_->IncognitoIndexToOriginal(i);
@@ skipping 138 matching lines @@
 extensions::ActiveTabPermissionGranter*
     BrowserActionsContainer::GetActiveTabPermissionGranter() {
   content::WebContents* web_contents =
       browser_->tab_strip_model()->GetActiveWebContents();
   if (!web_contents)
     return NULL;
   return extensions::TabHelper::FromWebContents(web_contents)->
       active_tab_permission_granter();
 }
 
-size_t BrowserActionsContainer::GetFirstVisibleIconIndex() const {
-  return in_overflow_mode() ?
-      main_container_->VisibleBrowserActionsAfterAnimation() : 0;
-}
-
 ExtensionPopup* BrowserActionsContainer::TestGetPopup() {
   return popup_owner_ ? popup_owner_->view_controller()->popup() : NULL;
 }
 
 void BrowserActionsContainer::TestSetIconVisibilityCount(size_t icons) {
   model_->SetVisibleIconCountForTest(icons);
 }
 
 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 (model_->is_highlighting() && !browser_action_views_.empty()) {
     views::Painter::PaintPainterAt(
         canvas, highlight_painter_.get(), 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.
-    int drop_icon_x = browser_action_views_[GetFirstVisibleIconIndex()]->x() +
+    int drop_icon_x = ToolbarView::kStandardSpacing +
         (drop_position_->icon_in_row * IconWidth(true));
     // Next, find the space before the drop icon. This will either be
     // kItemSpacing or ToolbarView::kStandardSpacing, depending on whether this
     // is the first icon.
     // NOTE: Right now, these are the same. But let's do this right for if they
     // ever aren't.
     int space_before_drop_icon = drop_position_->icon_in_row == 0 ?
         ToolbarView::kStandardSpacing : kItemSpacing;
     // Now place the drop indicator halfway between this and the end of the
     // previous icon.  If there is an odd amount of available space between the
@@ skipping 387 matching lines @@
   if (initialized_ && profile_->IsOffTheRecord()) {
     main_displayed = in_overflow_mode() ?
         main_container_->VisibleBrowserActionsAfterAnimation() :
         VisibleBrowserActionsAfterAnimation();
   }
 
   // The overflow displays any (displayable) icons not shown by the main bar.
   return in_overflow_mode() ?
       displayable_icon_count - main_displayed : main_displayed;
 }