ash: Fix launcher icon overlaps with status.

ash/launcher/launcher_view.cc

Index: ash/launcher/launcher_view.cc
diff --git a/ash/launcher/launcher_view.cc b/ash/launcher/launcher_view.cc
index f844b7ed87658bc4dd270de6ec7ca4b4de6bfad5..3fa923953dcaecbfa5edbb81fcd99d0f884e6731 100644
--- a/ash/launcher/launcher_view.cc
+++ b/ash/launcher/launcher_view.cc
@@ -181,6 +181,22 @@ void ReflectItemStatus(const ash::LauncherItem& item,
       break;
   }
 }
+
+// A class used by TestAPI to wait for animations.
+class TestAPIAnimationObserver : public views::BoundsAnimatorObserver {
+ public:
+  TestAPIAnimationObserver() {}
+  virtual ~TestAPIAnimationObserver() {}
+
+  // views::BoundsAnimatorObserver overrides:
+  virtual void OnBoundsAnimatorDone(views::BoundsAnimator* animator) OVERRIDE {
+    MessageLoop::current()->Quit();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestAPIAnimationObserver);
+};
+
 }  // namespace
 
 // AnimationDelegate used when inserting a new item. This steadily decreased the
@@ -225,7 +241,6 @@ class LauncherView::StartFadeAnimationDelegate :
 
   // AnimationDelegate overrides:
   virtual void AnimationEnded(const Animation* animation) OVERRIDE {
-    view_->SetVisible(true);
     launcher_view_->FadeIn(view_);
   }
   virtual void AnimationCanceled(const Animation* animation) OVERRIDE {
@@ -244,17 +259,45 @@ int LauncherView::TestAPI::GetButtonCount() {
 }
 
 LauncherButton* LauncherView::TestAPI::GetButton(int index) {
-  if (index == 0)
+  // App list button is not a LauncherButton.
+  if (index == GetButtonCount() - 1)
     return NULL;
 
   return static_cast<LauncherButton*>(
       launcher_view_->view_model_->view_at(index));
 }
 
+int LauncherView::TestAPI::GetLastVisibleIndex() {
+  return launcher_view_->last_visible_index_;
+}
+
+bool LauncherView::TestAPI::IsOverflowButtonVisible() {
+  return launcher_view_->overflow_button_->visible();
+}
+
+void LauncherView::TestAPI::SetAnimationDuration(int duration_ms) {
+  launcher_view_->bounds_animator_->SetAnimationDuration(duration_ms);
+}
+
+void LauncherView::TestAPI::RunMessageLoopUntilAnimationsDone() {
+  if (!launcher_view_->bounds_animator_->IsAnimating())
+    return;
+
+  scoped_ptr<TestAPIAnimationObserver> observer(new TestAPIAnimationObserver());
+  launcher_view_->bounds_animator_->AddObserver(observer.get());
+
+  // This nest loop will quit when TestAPIAnimationObserver's
+  // OnBoundsAnimatorDone is called.
+  MessageLoop::current()->Run();
+
+  launcher_view_->bounds_animator_->RemoveObserver(observer.get());
+}
+
 LauncherView::LauncherView(LauncherModel* model, LauncherDelegate* delegate)
     : model_(model),
       delegate_(delegate),
       view_model_(new views::ViewModel),
+      last_visible_index_(-1),
       overflow_button_(NULL),
       dragging_(NULL),
       drag_view_(NULL),
@@ -346,6 +389,7 @@ void LauncherView::LayoutToIdealBounds() {
   IdealBounds ideal_bounds;
   CalculateIdealBounds(&ideal_bounds);
   views::ViewModelUtils::SetViewBoundsToIdealBounds(*view_model_);
+  views::ViewModelUtils::SetViewVisibilityToIdealVisibility(*view_model_);
   overflow_button_->SetBoundsRect(ideal_bounds.overflow_bounds);
 }
 
@@ -364,30 +408,25 @@ void LauncherView::CalculateIdealBounds(IdealBounds* bounds) {
   }
 
   bounds->overflow_bounds.set_size(gfx::Size(kButtonWidth, kButtonHeight));
-  int last_visible_index = DetermineLastVisibleIndex(
+  last_visible_index_ = DetermineLastVisibleIndex(
       available_width - kLeadingInset - bounds->overflow_bounds.width() -
       kButtonSpacing - kButtonWidth);
-  bool show_overflow =
-      (last_visible_index + 1 != view_model_->view_size());
   int app_list_index = view_model_->view_size() - 1;
-  if (overflow_button_->visible() != show_overflow) {
-    // Only change visibility of the views if the visibility of the overflow
-    // button changes. Otherwise we'll effect the insertion animation, which
-    // changes the visibility.
-    for (int i = 0; i <= last_visible_index; ++i)
-      view_model_->view_at(i)->SetVisible(true);
-    for (int i = last_visible_index + 1; i < view_model_->view_size(); ++i) {
-      if (i != app_list_index)
-        view_model_->view_at(i)->SetVisible(false);
-    }
+  bool show_overflow = (last_visible_index_ + 1 < app_list_index);
+
+  for (int i = 0; i < view_model_->view_size(); ++i) {
+    view_model_->set_ideal_visibility(
+        i,
+        i == app_list_index || i <= last_visible_index_);
   }
+
   overflow_button_->SetVisible(show_overflow);
   if (show_overflow) {
     DCHECK_NE(0, view_model_->view_size());
     // We always want the app list visible.
     gfx::Rect app_list_bounds = view_model_->ideal_bounds(app_list_index);
-    x = last_visible_index == -1 ?
-        kLeadingInset : view_model_->ideal_bounds(last_visible_index).right();
+    x = last_visible_index_ == -1 ?
+        kLeadingInset : view_model_->ideal_bounds(last_visible_index_).right();
     app_list_bounds.set_x(x);
     view_model_->set_ideal_bounds(app_list_index, app_list_bounds);
     x = app_list_bounds.right() + kButtonSpacing;
@@ -677,7 +716,7 @@ void LauncherView::LauncherItemAdded(int model_index) {
   view->SetVisible(false);
   view_model_->Add(view, model_index);
 
-  // Give the button it's ideal bounds. That way if we end up animating the
+  // Give the button its ideal bounds. That way if we end up animating the
   // button before this animation completes it doesn't appear at some random
   // spot (because it was in the middle of animating from 0,0 0x0 to its
   // target).
@@ -685,13 +724,21 @@ void LauncherView::LauncherItemAdded(int model_index) {
   CalculateIdealBounds(&ideal_bounds);
   view->SetBoundsRect(view_model_->ideal_bounds(model_index));
 
-  // The first animation moves all the views to their target position. |view| is
-  // hidden, so it visually appears as though we are providing space for
+  // The first animation moves all the views to their target position. |view|
+  // is hidden, so it visually appears as though we are providing space for
   // it. When done we'll fade the view in.
   AnimateToIdealBounds();
-  if (!overflow_button_->visible()) {
+  if (model_index <= last_visible_index_) {
     bounds_animator_->SetAnimationDelegate(
         view, new StartFadeAnimationDelegate(this, view), true);
+
+    // Hides view that should become invisible immediately.

[sky, 2012/04/13 22:00:04]

If possible, I would rather change the visibility immediately. It's less error
prone and simpler.

How about changing CalculateIdealBounds to always change the visibility, and
then change this method so that it resets the visibility after calculate? I
believe that would mean no need to cache the ideal visibility and you could
remove this and the same from remove.

[xiyuan, 2012/04/13 22:49:53]

This causes flashes. When CalculateIdealBounds sets it to visible, it is visible
immediately. Reset hides it and then it shows up again after animation. :(

[xiyuan, 2012/04/23 18:08:53]

Done. Set opacity to 0 work as expected and thus removed ideal_visibility etc
from ViewModel.

+    for (int i = 0; i < view_model_->view_size(); ++i) {
+      if (!view_model_->ideal_visibility(i) &&
+          view_model_->view_at(i)->visible()) {
+        view_model_->view_at(i)->SetVisible(false);
+      }
+    }
   }
 }
 
@@ -708,6 +755,18 @@ void LauncherView::LauncherItemRemoved(int model_index, LauncherID id) {
   bounds_animator_->AnimateViewTo(view, view->bounds());
   bounds_animator_->SetAnimationDelegate(
       view, new FadeOutAnimationDelegate(this, view), true);
+
+  IdealBounds ideal_bounds;
+  CalculateIdealBounds(&ideal_bounds);
+  if (model_index <= last_visible_index_) {
+    // Shows view that should become visible immediately.
+    for (int i = 0; i < view_model_->view_size(); ++i) {
+      if (view_model_->ideal_visibility(i) &&
+          !view_model_->view_at(i)->visible()) {
+        view_model_->view_at(i)->SetVisible(true);
+      }
+    }
+  }
 }
 
 void LauncherView::LauncherItemChanged(int model_index,