Clip tabs in overflow mode.

chrome/browser/ui/views/tabs/tab_strip.cc

 // Copyright (c) 2012 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/tabs/tab_strip.h"
 
 #if defined(OS_WIN)
 #include <windowsx.h>
 #endif
 
@@ skipping 109 matching lines @@
 base::string16 GetClipboardText() {
   if (!ui::Clipboard::IsSupportedClipboardType(ui::CLIPBOARD_TYPE_SELECTION))
     return base::string16();
   ui::Clipboard* clipboard = ui::Clipboard::GetForCurrentThread();
   CHECK(clipboard);
   base::string16 clipboard_text;
   clipboard->ReadText(ui::CLIPBOARD_TYPE_SELECTION, &clipboard_text);
   return clipboard_text;
 }
 
+// Animation delegate used for any automatic tab movement.  Hides the tab if it
+// is not fully visible within the tabstrip area, to prevent overflow clipping.
+class TabAnimationDelegate : public gfx::AnimationDelegate {
+ public:
+  TabAnimationDelegate(TabStrip* tab_strip, Tab* tab);
+  virtual ~TabAnimationDelegate();
+
+  virtual void AnimationProgressed(const gfx::Animation* animation) OVERRIDE;
+
+ protected:
+  TabStrip* tab_strip() { return tab_strip_; }
+  Tab* tab() { return tab_; }
+
+ private:
+  TabStrip* const tab_strip_;
+  Tab* const tab_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabAnimationDelegate);
+};
+
+TabAnimationDelegate::TabAnimationDelegate(TabStrip* tab_strip, Tab* tab)
+    : tab_strip_(tab_strip),
+      tab_(tab) {
+}
+
+TabAnimationDelegate::~TabAnimationDelegate() {
+}
+
+void TabAnimationDelegate::AnimationProgressed(
+    const gfx::Animation* animation) {
+  tab_->SetVisible(tab_strip_->ShouldTabBeVisible(tab_));
+}
+
 // Animation delegate used when a dragged tab is released. When done sets the
 // dragging state to false.
-class ResetDraggingStateDelegate : public gfx::AnimationDelegate {
+class ResetDraggingStateDelegate : public TabAnimationDelegate {
  public:
-  explicit ResetDraggingStateDelegate(Tab* tab);
+  ResetDraggingStateDelegate(TabStrip* tab_strip, Tab* tab);
   virtual ~ResetDraggingStateDelegate();
 
   virtual void AnimationEnded(const gfx::Animation* animation) OVERRIDE;
   virtual void AnimationCanceled(const gfx::Animation* animation) OVERRIDE;
 
  private:
-  Tab* tab_;
-
   DISALLOW_COPY_AND_ASSIGN(ResetDraggingStateDelegate);
 };
 
-ResetDraggingStateDelegate::ResetDraggingStateDelegate(Tab* tab) : tab_(tab) {
+ResetDraggingStateDelegate::ResetDraggingStateDelegate(TabStrip* tab_strip,
+                                                       Tab* tab)
+    : TabAnimationDelegate(tab_strip, tab) {
 }
 
 ResetDraggingStateDelegate::~ResetDraggingStateDelegate() {
 }
 
 void ResetDraggingStateDelegate::AnimationEnded(
     const gfx::Animation* animation) {
-  tab_->set_dragging(false);
+  tab()->set_dragging(false);
+  AnimationProgressed(animation);  // Forces tab visibility to update.
 }
 
 void ResetDraggingStateDelegate::AnimationCanceled(
     const gfx::Animation* animation) {
   AnimationEnded(animation);
 }
 
 // If |dest| contains the point |point_in_source| the event handler from |dest|
 // is returned. Otherwise NULL is returned.
 views::View* ConvertPointToViewAndGetEventHandler(
@@ skipping 284 matching lines @@
   }
 
   DISALLOW_COPY_AND_ASSIGN(NewTabButtonTargeter);
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 // TabStrip::RemoveTabDelegate
 //
 // AnimationDelegate used when removing a tab. Does the necessary cleanup when
 // done.
-class TabStrip::RemoveTabDelegate : public gfx::AnimationDelegate {
+class TabStrip::RemoveTabDelegate : public TabAnimationDelegate {
  public:
   RemoveTabDelegate(TabStrip* tab_strip, Tab* tab);
 
   virtual void AnimationEnded(const gfx::Animation* animation) OVERRIDE;
   virtual void AnimationCanceled(const gfx::Animation* animation) OVERRIDE;
 
  private:
-  void CompleteRemove();
-
-  // When the animation completes, we send the Container a message to simulate
-  // a mouse moved event at the current mouse position. This tickles the Tab
-  // the mouse is currently over to show the "hot" state of the close button.
-  void HighlightCloseButton();
-
-  TabStrip* tabstrip_;
-  Tab* tab_;
-
   DISALLOW_COPY_AND_ASSIGN(RemoveTabDelegate);
 };
 
 TabStrip::RemoveTabDelegate::RemoveTabDelegate(TabStrip* tab_strip,
                                                Tab* tab)
-    : tabstrip_(tab_strip),
-      tab_(tab) {
+    : TabAnimationDelegate(tab_strip, tab) {
 }
 
 void TabStrip::RemoveTabDelegate::AnimationEnded(
     const gfx::Animation* animation) {
-  CompleteRemove();
+  DCHECK(tab()->closing());
+  tab_strip()->RemoveAndDeleteTab(tab());
+
+  // Send the Container a message to simulate a mouse moved event at the current
+  // mouse position. This tickles the Tab the mouse is currently over to show
+  // the "hot" state of the close button.  Note that this is not required (and
+  // indeed may crash!) for removes spawned by non-mouse closes and
+  // drag-detaches.
+  if (!tab_strip()->IsDragSessionActive() &&
+      tab_strip()->ShouldHighlightCloseButtonAfterRemove()) {
+    // The widget can apparently be null during shutdown.
+    views::Widget* widget = tab_strip()->GetWidget();
+    if (widget)
+      widget->SynthesizeMouseMoveEvent();
+  }
 }
 
 void TabStrip::RemoveTabDelegate::AnimationCanceled(
     const gfx::Animation* animation) {
   AnimationEnded(animation);
 }
 
-void TabStrip::RemoveTabDelegate::CompleteRemove() {
-  DCHECK(tab_->closing());
-  tabstrip_->RemoveAndDeleteTab(tab_);
-  HighlightCloseButton();
-}
-
-void TabStrip::RemoveTabDelegate::HighlightCloseButton() {
-  if (tabstrip_->IsDragSessionActive() ||
-      !tabstrip_->ShouldHighlightCloseButtonAfterRemove()) {
-    // This function is not required (and indeed may crash!) for removes
-    // spawned by non-mouse closes and drag-detaches.
-    return;
-  }
-
-  views::Widget* widget = tabstrip_->GetWidget();
-  // This can be null during shutdown. See http://crbug.com/42737.
-  if (!widget)
-    return;
-
-  widget->SynthesizeMouseMoveEvent();
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // TabStrip, public:
 
 // static
 const char TabStrip::kViewClassName[] = "TabStrip";
 const int TabStrip::kNewTabButtonHorizontalOffset = -11;
 const int TabStrip::kNewTabButtonVerticalOffset = 7;
 const int TabStrip::kMiniToNonMiniGap = 3;
 const int TabStrip::kNewTabButtonAssetWidth = 34;
 const int TabStrip::kNewTabButtonAssetHeight = 18;
@@ skipping 185 matching lines @@
       touch_layout_->SetXAndMiniCount(start_x, mini_tab_count);
     }
     if (GetWidget() && GetWidget()->IsVisible())
       StartMiniTabAnimation();
     else
       DoLayout();
   }
   SwapLayoutIfNecessary();
 }
 
+bool TabStrip::ShouldTabBeVisible(const Tab* tab) const {
+  // When stacking tabs, all tabs should always be visible.
+  if (stacked_layout_)

[sky, 2014/06/17 19:41:17]

touch_layout_ is more correct here.

[Peter Kasting, 2014/06/17 20:20:20]

Why?  If stacked_layout_ is true, then either we're stacking tabs (touch_layout_
is non-NULL) or we don't have any overflow.  In either case, all tabs are
visible.  Thus using |stacked_layout_| short-circuits cases that would otherwise
have to run through the whole function.

Or am I mistaken?

[sky, 2014/06/17 23:53:13]

No, you're right.

+    return true;
+
+  // If the tab is currently clipped, it shouldn't be visible.  Note that we
+  // allow dragged tabs to draw over the "New Tab button" region as well,
+  // because either the New Tab button will be hidden, or the dragged tabs will
+  // be animating back to their normal positions and we don't want to hide them
+  // in the New Tab button region in case they re-appear after leaving it.
+  // (This prevents flickeriness.)  We never draw non-dragged tabs in New Tab
+  // button area, even when the button is invisible, so that they don't appear
+  // to "pop in" when the button disappears.
+  // TODO: Probably doesn't work for RTL
+  int right_edge = tab->bounds().right();
+  const int visible_width = tab->dragging() ? width() : tab_area_width();
+  if (right_edge > visible_width)
+    return false;
+
+  // Non-clipped dragging tabs should always be visible.
+  if (tab->dragging())
+    return true;
+
+  // Let all non-clipped closing tabs be visible.  These will probably finish
+  // closing before the user changes the active tab, so there's little reason to
+  // try and make the more complex logic below apply.
+  if (tab->closing())
+    return true;
+
+  // Now we need to check whether the tab isn't currently clipped, but could
+  // become clipped if we changed the active tab, widening either this tab or
+  // the tabstrip portion before it.
+
+  // Mini tabs don't change size when activated, so any tab in the mini tab
+  // region is safe.
+  const int tab_index = GetModelIndexOfTab(tab);
+  if (tab_index <= GetMiniTabCount())

[sky, 2014/06/17 19:41:18]

I would check tab->data().mini here. The conditional you have is the same, but
more expensive. Arguably it doesn't matter as you need the index below.

[Peter Kasting, 2014/06/17 20:20:20]

Done.

+    return true;
+
+  // If the active tab is on or before this tab, we're safe.
+  if (controller_->GetActiveIndex() <= tab_index)
+    return true;
+
+  // We need to check what would happen if the active tab were to move to this
+  // tab or before.
+  return (right_edge + current_selected_width_ - current_unselected_width_) <=
+      tab_area_width();
+}
+
 void TabStrip::PrepareForCloseAt(int model_index, CloseTabSource source) {
   if (!in_tab_close_ && IsAnimating()) {
     // Cancel any current animations. We do this as remove uses the current
     // ideal bounds and we need to know ideal bounds is in a good state.
     StopAnimating(true);
   }
 
   if (!GetWidget())
     return;
 
@@ skipping 752 matching lines @@
   // strip.
   if (TabDragController::IsAttachedTo(this)) {
     bounds_animator_.StopAnimatingView(newtab_button_);
     newtab_button_->SetBoundsRect(newtab_button_bounds_);
   }
 }
 
 void TabStrip::AnimateToIdealBounds() {
   for (int i = 0; i < tab_count(); ++i) {
     Tab* tab = tab_at(i);
-    if (!tab->dragging())
+    if (!tab->dragging()) {
       bounds_animator_.AnimateViewTo(tab, ideal_bounds(i));
+      bounds_animator_.SetAnimationDelegate(
+          tab, make_scoped_ptr(new TabAnimationDelegate(this, tab)));
+    }
   }
 
   bounds_animator_.AnimateViewTo(newtab_button_, newtab_button_bounds_);
 }
 
 bool TabStrip::ShouldHighlightCloseButtonAfterRemove() {
   return in_tab_close_;
 }
 
 void TabStrip::DoLayout() {
   last_layout_size_ = size();
 
   StopAnimating(false);
 
   SwapLayoutIfNecessary();
 
   if (touch_layout_)
     touch_layout_->SetWidth(tab_area_width());
 
   GenerateIdealBounds();
 
   views::ViewModelUtils::SetViewBoundsToIdealBounds(tabs_);
+  SetTabVisibility();
 
   SchedulePaint();
 
   bounds_animator_.StopAnimatingView(newtab_button_);
   newtab_button_->SetBoundsRect(newtab_button_bounds_);
 }
 
+void TabStrip::SetTabVisibility() {

[sky, 2014/06/17 19:41:17]

Can you still hit the crash you sent to me with your code? I don't see you
calling out to the model here, so it seems like it shouldn't be effected. Is
that right?

[Peter Kasting, 2014/06/17 20:20:20]

I removed the DCHECK in ShouldTabBeVisible() that I was seeing, so as to not
trip this.
With the DCHECK removed, I think the only call to the model is when
ShouldTabBeVisible() calls TabStripController::GetActiveIndex().

+  // We could probably be more efficient here by making use of the fact that the
+  // tabstrip will always have any visible tabs, and then any invisible tabs, so
+  // we could e.g. binary-search for the changeover point.  But since we have to
+  // iterate through all the tabs to call SetVisible() anyway, it doesn't seem
+  // worth it.
+  for (int i = 0; i < tab_count(); ++i) {
+    Tab* tab = tab_at(i);
+    tab->SetVisible(ShouldTabBeVisible(tab));
+  }
+  for (TabsClosingMap::const_iterator i(tabs_closing_map_.begin());
+       i != tabs_closing_map_.end(); ++i) {
+    for (Tabs::const_iterator j(i->second.begin()); j != i->second.end(); ++j) {
+      Tab* tab = *j;
+      tab->SetVisible(ShouldTabBeVisible(tab));
+    }
+  }
+}
+
 void TabStrip::DragActiveTab(const std::vector<int>& initial_positions,
                              int delta) {
   DCHECK_EQ(tab_count(), static_cast<int>(initial_positions.size()));
   if (!touch_layout_) {
     StackDraggedTabs(delta);
     return;
   }
   SetIdealBoundsFromPositions(initial_positions);
   touch_layout_->DragActiveTab(delta);
   DoLayout();
@@ skipping 110 matching lines @@
     // consecutive animate the view into position. Do the same if the tab is
     // already animating (which means we previously caused it to animate).
     if ((initial_drag &&
          GetModelIndexOfTab(tabs[i]) != consecutive_index) ||
         bounds_animator_.IsAnimating(tabs[i])) {
       bounds_animator_.SetTargetBounds(tabs[i], new_bounds);
     } else {
       tab->SetBoundsRect(new_bounds);
     }
   }
+  SetTabVisibility();
 }
 
 void TabStrip::CalculateBoundsForDraggedTabs(const Tabs& tabs,
                                              std::vector<gfx::Rect>* bounds) {
   int x = 0;
   for (size_t i = 0; i < tabs.size(); ++i) {
     Tab* tab = tabs[i];
     if (i > 0 && tab->data().mini != tabs[i - 1]->data().mini)
       x += kMiniToNonMiniGap;
     gfx::Rect new_bounds = tab->bounds();
@@ skipping 17 matching lines @@
 }
 
 int TabStrip::GetMiniTabCount() const {
   int mini_count = 0;
   while (mini_count < tab_count() && tab_at(mini_count)->data().mini)
     mini_count++;
   return mini_count;
 }
 
 const Tab* TabStrip::GetLastVisibleTab() const {
-  return tab_at(tab_count() - 1);
+  for (int i = tab_count() - 1; i >= 0; --i) {
+    const Tab* tab = tab_at(i);
+    if (tab->visible())
+      return tab;
+  }
+  NOTREACHED();
+  return NULL;
 }
 
 void TabStrip::RemoveTabFromViewModel(int index) {
   // We still need to paint the tab until we actually remove it. Put it
   // in tabs_closing_map_ so we can find it.
   tabs_closing_map_[index].push_back(tab_at(index));
   UpdateTabsClosingMap(index + 1, -1);
   tabs_.Remove(index);
 }
 
@@ skipping 50 matching lines @@
 
   // Move the dragged tabs to their ideal bounds.
   GenerateIdealBounds();
 
   // Sets the bounds of the dragged tabs.
   for (size_t i = 0; i < tabs.size(); ++i) {
     int tab_data_index = GetModelIndexOfTab(tabs[i]);
     DCHECK_NE(-1, tab_data_index);
     tabs[i]->SetBoundsRect(ideal_bounds(tab_data_index));
   }
+  SetTabVisibility();
   SchedulePaint();
 }
 
 void TabStrip::DraggedTabsDetached() {
   // Let the controller know that the user is not dragging this tabstrip's tabs
   // anymore.
   controller()->OnStoppedDraggingTabs();
   newtab_button_->SetVisible(true);
 }
 
@@ skipping 29 matching lines @@
 
     // Animate the view back to its correct position.
     GenerateIdealBounds();
     AnimateToIdealBounds();
   }
   bounds_animator_.AnimateViewTo(tab, ideal_bounds(tab_data_index));
   // Install a delegate to reset the dragging state when done. We have to leave
   // dragging true for the tab otherwise it'll draw beneath the new tab button.
   bounds_animator_.SetAnimationDelegate(
       tab,
-      scoped_ptr<gfx::AnimationDelegate>(new ResetDraggingStateDelegate(tab)));
+      scoped_ptr<gfx::AnimationDelegate>(
+          new ResetDraggingStateDelegate(this, tab)));
 }
 
 void TabStrip::OwnDragController(TabDragController* controller) {
   // Typically, ReleaseDragController() and OwnDragController() calls are paired
   // via corresponding calls to TabDragController::Detach() and
   // TabDragController::Attach(). There is one exception to that rule: when a
   // drag might start, we create a TabDragController that is owned by the
   // potential source tabstrip in MaybeStartDrag(). If a drag actually starts,
   // we then call Attach() on the source tabstrip, but since the source tabstrip
   // already owns the TabDragController, so we don't need to do anything.
@@ skipping 127 matching lines @@
 
   *unselected_width = min_unselected_width;
   *selected_width = min_selected_width;
 
   if (tab_count == 0) {
     // Return immediately to avoid divide-by-zero below.
     return;
   }
 
   // Determine how much space we can actually allocate to tabs.
-  int available_width;
-  if (available_width_for_tabs_ < 0) {
-    available_width = width() - new_tab_button_width();
-  } else {
-    // Interesting corner case: if |available_width_for_tabs_| > the result
-    // of the calculation in the conditional arm above, the strip is in
-    // overflow.  We can either use the specified width or the true available
-    // width here; the first preserves the consistent "leave the last tab under
-    // the user's mouse so they can close many tabs" behavior at the cost of
-    // prolonging the glitchy appearance of the overflow state, while the second
-    // gets us out of overflow as soon as possible but forces the user to move
-    // their mouse for a few tabs' worth of closing.  We choose visual
-    // imperfection over behavioral imperfection and select the first option.
-    available_width = available_width_for_tabs_;
-  }
-
+  int available_width = (available_width_for_tabs_ < 0) ?
+      tab_area_width() : available_width_for_tabs_;
   if (mini_tab_count > 0) {
     available_width -=
         mini_tab_count * (Tab::GetMiniWidth() + kTabHorizontalOffset);
     tab_count -= mini_tab_count;
     if (tab_count == 0) {
       *selected_width = *unselected_width = Tab::GetStandardSize().width();
       return;
     }
     // Account for gap between the last mini-tab and first non-mini-tab.
     available_width -= kMiniToNonMiniGap;
@@ skipping 466 matching lines @@
     // many mini-tabs there are).
     GenerateIdealBoundsForMiniTabs(NULL);
     touch_layout_->SetXAndMiniCount(GetStartXForNormalTabs(),
                                     GetMiniTabCount());
     touch_layout_->SetActiveIndex(controller_->GetActiveIndex());
   } else {
     touch_layout_.reset();
   }
   PrepareForAnimation();
   GenerateIdealBounds();
+  SetTabVisibility();
   AnimateToIdealBounds();
 }
 
 bool TabStrip::NeedsTouchLayout() const {
   if (!stacked_layout_)
     return false;
 
   int mini_tab_count = GetMiniTabCount();
   int normal_count = tab_count() - mini_tab_count;
   if (normal_count <= 1 || normal_count == mini_tab_count)
@@ skipping 132 matching lines @@
         action = TouchUMA::GESTURE_TABSWITCH_TAP;
       TouchUMA::RecordGestureAction(action);
       break;
     }
 
     default:
       break;
   }
   event->SetHandled();
 }