Chrome OS : Simplify shelf animation for user switching.

There are two branches currently:
- if at least one user has a maximized window, fade shelf to black
  and back. Note that this branch is broken, and instead all you see is
  a white flash, but this is easily fixed if need be.
- otherwise, set shelf to hidden and then let the user switch update the
  state from there.

If we remove both of these branches and just let the user switch change
the state of the shelf, the outcome is nearly indistinguishable.
Scratching the second bullet point above seems to have no discernible
effect. Scratching the first branch just means there will be no fade-to-
black, however the addition and removal of launcher icons is animated
regardless, so it still looks smooth. Additionally, the fade-to-black
doesn't work on the system tray (clock, avatar), and hasn't worked at
all for 4 months and no one noticed. In a sense, removing this branch is
an improvement because it removes a white flash.

Bug: 756085
Change-Id: If40c65e02035947709be31db466dea144ab0b462
Reviewed-on: https://chromium-review.googlesource.com/683035
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504363}

ash/public/cpp/shelf_prefs.cc

@@ -160,7 +160,7 @@ const char* AutoHideBehaviorToPref(ShelfAutoHideBehavior behavior) {
     case SHELF_AUTO_HIDE_ALWAYS_HIDDEN:
       // This should not be a valid preference option for now. We only want to
       // completely hide it when we run in app mode - or while we temporarily
-      // hide the shelf as part of an animation (e.g. the multi user change).
+      // hide the shelf (e.g. SessionAbortedDialog).
       return nullptr;
   }
   NOTREACHED();

ash/shelf/shelf_layout_manager.cc

@@ -392,11 +392,6 @@ bool ShelfLayoutManager::ProcessGestureEvent(
   return false;
 }
 
-void ShelfLayoutManager::SetAnimationDurationOverride(
-    int duration_override_in_ms) {
-  duration_override_in_ms_ = duration_override_in_ms;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // ShelfLayoutManager, wm::WmSnapToPixelLayoutManager implementation:
 
@@ -613,15 +608,12 @@ void ShelfLayoutManager::UpdateBoundsAndOpacity(
     ui::ScopedLayerAnimationSettings status_animation_setter(
         GetLayer(status_widget)->GetAnimator());
     if (animate) {
-      int duration = duration_override_in_ms_ ? duration_override_in_ms_
-                                              : kAnimationDurationMS;
-      shelf_animation_setter.SetTransitionDuration(
-          base::TimeDelta::FromMilliseconds(duration));
+      auto duration = base::TimeDelta::FromMilliseconds(kAnimationDurationMS);
+      shelf_animation_setter.SetTransitionDuration(duration);
       shelf_animation_setter.SetTweenType(gfx::Tween::EASE_OUT);
       shelf_animation_setter.SetPreemptionStrategy(
           ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-      status_animation_setter.SetTransitionDuration(
-          base::TimeDelta::FromMilliseconds(duration));
+      status_animation_setter.SetTransitionDuration(duration);
       status_animation_setter.SetTweenType(gfx::Tween::EASE_OUT);
       status_animation_setter.SetPreemptionStrategy(
           ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);

ash/shelf/shelf_layout_manager.h

@@ -140,10 +140,6 @@ class ASH_EXPORT ShelfLayoutManager
   // be processed any further, false otherwise.
   bool ProcessGestureEvent(const ui::GestureEvent& event_in_screen);
 
-  // Set an animation duration override for the show / hide animation of the
-  // shelf. Specifying 0 leads to use the default.
-  void SetAnimationDurationOverride(int duration_override_in_ms);
-
   // Overridden from wm::WmSnapToPixelLayoutManager:
   void OnWindowResized() override;
   void SetChildBounds(aura::Window* child,
@@ -392,9 +388,6 @@ class ASH_EXPORT ShelfLayoutManager
   // needs to be removed from the available work area.
   int chromevox_panel_height_ = 0;
 
-  // The show hide animation duration override or 0 for default.
-  int duration_override_in_ms_ = 0;
-
   // Whether background blur is enabled.
   const bool is_background_blur_enabled_;
 

ash/shelf/shelf_widget.cc

@@ -45,7 +45,6 @@ class ShelfWidget::DelegateView : public views::WidgetDelegate,
   FocusCycler* focus_cycler() { return focus_cycler_; }
 
   ui::Layer* opaque_background() { return &opaque_background_; }
-  ui::Layer* opaque_foreground() { return &opaque_foreground_; }
 
   void SetParentLayer(ui::Layer* layer);
 
@@ -67,10 +66,6 @@ class ShelfWidget::DelegateView : public views::WidgetDelegate,
   // A background layer that may be visible depending on a
   // ShelfBackgroundAnimator.
   ui::Layer opaque_background_;
-  // A black foreground layer which is shown while transitioning between users.
-  // Note: Since the back- and foreground layers have different functions they
-  // can be used simultaneously - so no repurposing possible.
-  ui::Layer opaque_foreground_;
 
   DISALLOW_COPY_AND_ASSIGN(DelegateView);
 };
@@ -78,22 +73,17 @@ class ShelfWidget::DelegateView : public views::WidgetDelegate,
 ShelfWidget::DelegateView::DelegateView(ShelfWidget* shelf_widget)
     : shelf_widget_(shelf_widget),
       focus_cycler_(nullptr),
-      opaque_background_(ui::LAYER_SOLID_COLOR),
-      opaque_foreground_(ui::LAYER_SOLID_COLOR) {
+      opaque_background_(ui::LAYER_SOLID_COLOR) {
   DCHECK(shelf_widget_);
   SetLayoutManager(new views::FillLayout());
   set_allow_deactivate_on_esc(true);
   opaque_background_.SetBounds(GetLocalBounds());
-  opaque_foreground_.SetBounds(GetLocalBounds());
-  opaque_foreground_.SetOpacity(0.0f);
-  opaque_foreground_.SetColor(SK_ColorBLACK);
 }
 
 ShelfWidget::DelegateView::~DelegateView() {}
 
 void ShelfWidget::DelegateView::SetParentLayer(ui::Layer* layer) {
   layer->Add(&opaque_background_);
-  layer->Add(&opaque_foreground_);
   ReorderLayers();
 }
 
@@ -105,12 +95,10 @@ bool ShelfWidget::DelegateView::CanActivate() const {
 void ShelfWidget::DelegateView::ReorderChildLayers(ui::Layer* parent_layer) {
   views::View::ReorderChildLayers(parent_layer);
   parent_layer->StackAtBottom(&opaque_background_);
-  parent_layer->StackAtTop(&opaque_foreground_);
 }
 
 void ShelfWidget::DelegateView::OnBoundsChanged(const gfx::Rect& old_bounds) {
   opaque_background_.SetBounds(GetLocalBounds());
-  opaque_foreground_.SetBounds(GetLocalBounds());
 }
 
 void ShelfWidget::DelegateView::UpdateShelfBackground(SkColor color) {
@@ -203,27 +191,6 @@ int ShelfWidget::GetBackgroundAlphaValue(
   return background_animator_.GetBackgroundAlphaValue(background_type);
 }
 
-void ShelfWidget::HideShelfBehindBlackBar(bool hide, int animation_time_ms) {
-  if (IsShelfHiddenBehindBlackBar() == hide)
-    return;
-
-  ui::Layer* opaque_foreground = delegate_view_->opaque_foreground();
-  float target_opacity = hide ? 1.0f : 0.0f;
-  std::unique_ptr<ui::ScopedLayerAnimationSettings> opaque_foreground_animation;
-  opaque_foreground_animation.reset(
-      new ui::ScopedLayerAnimationSettings(opaque_foreground->GetAnimator()));
-  opaque_foreground_animation->SetTransitionDuration(
-      base::TimeDelta::FromMilliseconds(animation_time_ms));
-  opaque_foreground_animation->SetPreemptionStrategy(
-      ui::LayerAnimator::REPLACE_QUEUED_ANIMATIONS);
-
-  opaque_foreground->SetOpacity(target_opacity);
-}
-
-bool ShelfWidget::IsShelfHiddenBehindBlackBar() const {
-  return delegate_view_->opaque_foreground()->GetTargetOpacity() != 0.0f;
-}
-
 void ShelfWidget::OnShelfAlignmentChanged() {
   // Check added for http://crbug.com/738011.
   CHECK(status_area_widget_);

ash/shelf/shelf_widget.h

@@ -55,11 +55,6 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
   // Gets the alpha value of |background_type|.
   int GetBackgroundAlphaValue(ShelfBackgroundType background_type) const;
 
-  // Hide the shelf behind a black bar during e.g. a user transition when |hide|
-  // is true. The |animation_time_ms| will be used as animation duration.
-  void HideShelfBehindBlackBar(bool hide, int animation_time_ms);
-  bool IsShelfHiddenBehindBlackBar() const;
-
   ShelfLayoutManager* shelf_layout_manager() { return shelf_layout_manager_; }
   StatusAreaWidget* status_area_widget() const { return status_area_widget_; }
 

chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc

@@ -10,8 +10,6 @@
 #include "ash/content/shell_content_state.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/root_window_controller.h"
-#include "ash/shelf/shelf.h"
-#include "ash/shelf/shelf_widget.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/ash_test_environment_content.h"
@@ -1036,40 +1034,26 @@ TEST_F(MultiUserWindowManagerChromeOSTest, AnimationSteps) {
   EXPECT_FALSE(CoversScreen(window(1)));
   EXPECT_EQ("S[A], H[B], H[C]", GetStatus());
   EXPECT_EQ("A", GetOwnersOfVisibleWindowsAsString());
-  Shelf* shelf = GetPrimaryShelf();
-  EXPECT_NE(SHELF_AUTO_HIDE_ALWAYS_HIDDEN, shelf->auto_hide_behavior());
   EXPECT_EQ(1.0f, window(0)->layer()->GetTargetOpacity());
-  ShelfWidget* shelf_widget = shelf->shelf_widget();
-  EXPECT_FALSE(shelf_widget->IsShelfHiddenBehindBlackBar());
 
   // Start the animation and see that the old window is becoming invisible, the
-  // new one is becoming visible, the background starts transitionining and the
-  // shelf hides.
+  // new one is becoming visible, and the background starts transitionining.
   StartUserTransitionAnimation(account_id_B);
   EXPECT_EQ(kArrowBAccountIdString,
             GetWallpaperUserIdForTest());
   EXPECT_EQ("H[A], S[B], H[C]", GetStatus());
   EXPECT_EQ(0.0f, window(0)->layer()->GetTargetOpacity());
   EXPECT_EQ(1.0f, window(1)->layer()->GetTargetOpacity());
-  EXPECT_EQ(SHELF_AUTO_HIDE_ALWAYS_HIDDEN, shelf->auto_hide_behavior());
-  EXPECT_FALSE(shelf_widget->IsShelfHiddenBehindBlackBar());
 
-  // Staring the next step should show the shelf again, but there are many
-  // subsystems missing (preferences system, ChromeLauncherController, ...)
-  // which should set the shelf to its users state. Since that isn't there we
-  // can only make sure that it stays where it is.
   AdvanceUserTransitionAnimation();
   EXPECT_EQ(kArrowBAccountIdString,
             GetWallpaperUserIdForTest());
   EXPECT_EQ("H[A], S[B], H[C]", GetStatus());
   EXPECT_EQ(0.0f, window(0)->layer()->GetTargetOpacity());
   EXPECT_EQ(1.0f, window(1)->layer()->GetTargetOpacity());
-  EXPECT_EQ(SHELF_AUTO_HIDE_ALWAYS_HIDDEN, shelf->auto_hide_behavior());
-  EXPECT_FALSE(shelf_widget->IsShelfHiddenBehindBlackBar());
 
   // After the finalize the animation of the wallpaper should be finished.
   AdvanceUserTransitionAnimation();
-  EXPECT_FALSE(shelf_widget->IsShelfHiddenBehindBlackBar());
   EXPECT_EQ(kBAccountIdString, GetWallpaperUserIdForTest());
 }
 
@@ -1309,96 +1293,6 @@ class TestWindowObserver : public aura::WindowObserver {
   DISALLOW_COPY_AND_ASSIGN(TestWindowObserver);
 };
 
-// Test that switching between users with the shelf in the same place, the shelf
-// will get covered with a black bar instead being hidden and re-shown.
-TEST_F(MultiUserWindowManagerChromeOSTest, TestBlackBarCover) {
-  // We need to ensure this works for external connected monitors as well, so
-  // add a secondary display.
-  UpdateDisplay("500x500,500x500");
-
-  SetUpForThisManyWindows(2);
-
-  const AccountId account_id_A(AccountId::FromUserEmail("A"));
-  const AccountId account_id_B(AccountId::FromUserEmail("B"));
-
-  multi_user_window_manager()->SetWindowOwner(window(0), account_id_A);
-  multi_user_window_manager()->SetWindowOwner(window(1), account_id_B);
-
-  // Move the second window to the second display and make sure it is not
-  // maximized for the first part of the test.
-  ASSERT_EQ(2u, display_manager()->GetNumDisplays());
-  window(1)->SetBoundsInScreen(gfx::Rect(0, 0, 100, 100),
-                               display_manager()->GetDisplayAt(1));
-
-  ASSERT_EQ(2u, Shell::GetAllRootWindowControllers().size());
-  Shelf* primary_shelf = Shell::GetAllRootWindowControllers()[0]->shelf();
-  Shelf* secondary_shelf = Shell::GetAllRootWindowControllers()[1]->shelf();
-  ShelfWidget* primary_shelf_widget = primary_shelf->shelf_widget();
-  ShelfWidget* secondary_shelf_widget = secondary_shelf->shelf_widget();
-
-  // Turn the use of delays and animation on.
-  multi_user_window_manager()->SetAnimationSpeedForTest(
-      chrome::MultiUserWindowManagerChromeOS::ANIMATION_SPEED_FAST);
-  EXPECT_NE(SHELF_AUTO_HIDE_ALWAYS_HIDDEN, primary_shelf->auto_hide_behavior());
-  EXPECT_NE(SHELF_AUTO_HIDE_ALWAYS_HIDDEN,
-            secondary_shelf->auto_hide_behavior());
-  EXPECT_FALSE(primary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_FALSE(secondary_shelf_widget->IsShelfHiddenBehindBlackBar());
-
-  // First test that with no maximized window we show/hide the shelf.
-  StartUserTransitionAnimation(account_id_B);
-  EXPECT_FALSE(primary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_FALSE(secondary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_EQ(SHELF_AUTO_HIDE_ALWAYS_HIDDEN, primary_shelf->auto_hide_behavior());
-  EXPECT_EQ(SHELF_AUTO_HIDE_ALWAYS_HIDDEN,
-            secondary_shelf->auto_hide_behavior());
-
-  // Staring the next step should show the shelf again.
-  AdvanceUserTransitionAnimation();
-  EXPECT_FALSE(primary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_FALSE(secondary_shelf_widget->IsShelfHiddenBehindBlackBar());
-
-  AdvanceUserTransitionAnimation();
-  EXPECT_FALSE(primary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_FALSE(secondary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  primary_shelf->SetAutoHideBehavior(SHELF_AUTO_HIDE_BEHAVIOR_NEVER);
-  secondary_shelf->SetAutoHideBehavior(SHELF_AUTO_HIDE_BEHAVIOR_NEVER);
-
-  // Now we maximize the windows which will cause the black overlay to show up.
-  wm::GetWindowState(window(0))->Maximize();
-  wm::GetWindowState(window(1))->Maximize();
-  // We set a window observer on both windows to see that no resize is performed
-  // on our test windows.
-  TestWindowObserver window_observer;
-  window(0)->AddObserver(&window_observer);
-  window(1)->AddObserver(&window_observer);
-
-  // Start the animation and see that the shelf gets hidden by the black bar,
-  // and the AutoHide behavior remains as it was.
-  StartUserTransitionAnimation(account_id_A);
-  EXPECT_TRUE(primary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_TRUE(secondary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_NE(SHELF_AUTO_HIDE_ALWAYS_HIDDEN, primary_shelf->auto_hide_behavior());
-  EXPECT_NE(SHELF_AUTO_HIDE_ALWAYS_HIDDEN,
-            secondary_shelf->auto_hide_behavior());
-
-  // Starting the next step should show the shelf again.
-  AdvanceUserTransitionAnimation();
-  EXPECT_FALSE(primary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_FALSE(secondary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_NE(SHELF_AUTO_HIDE_ALWAYS_HIDDEN, primary_shelf->auto_hide_behavior());
-  EXPECT_NE(SHELF_AUTO_HIDE_ALWAYS_HIDDEN,
-            secondary_shelf->auto_hide_behavior());
-
-  AdvanceUserTransitionAnimation();
-  EXPECT_FALSE(primary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  EXPECT_FALSE(secondary_shelf_widget->IsShelfHiddenBehindBlackBar());
-  window(0)->RemoveObserver(&window_observer);
-  window(1)->RemoveObserver(&window_observer);
-  // No resize should have been done to the window.
-  EXPECT_EQ(0, window_observer.resize_calls());
-}
-
 // Test that switching between different user won't change the activated windows
 // and the property of transient windows.
 TEST_F(MultiUserWindowManagerChromeOSTest, TransientWindowActivationTest) {

chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.cc

@@ -4,10 +4,6 @@
 
 #include "chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.h"
 
-#include "ash/root_window_controller.h"
-#include "ash/shelf/shelf.h"
-#include "ash/shelf/shelf_layout_manager.h"
-#include "ash/shelf/shelf_widget.h"
 #include "ash/shell.h"
 #include "ash/wallpaper/wallpaper_delegate.h"
 #include "ash/wm/mru_window_tracker.h"
@@ -215,62 +211,15 @@ void UserSwitchAnimatorChromeOS::TransitionWallpaper(
 
 void UserSwitchAnimatorChromeOS::TransitionUserShelf(
     AnimationStep animation_step) {
-  ChromeLauncherController* chrome_launcher_controller =
-      ChromeLauncherController::instance();
-  // The shelf animation duration override.
-  int duration_override = animation_speed_ms_;
-  // Handle the shelf order of items. This is done once the old user is hidden.
-  if (animation_step == ANIMATION_STEP_SHOW_NEW_USER) {
-    // Some unit tests have no ChromeLauncherController.
-    if (chrome_launcher_controller)
-      chrome_launcher_controller->ActiveUserChanged(
-          new_account_id_.GetUserEmail());
-    // Hide the black rectangle on top of each shelf again.
-    for (aura::Window* window : ash::Shell::GetAllRootWindows()) {
-      ash::ShelfWidget* shelf = ash::Shelf::ForWindow(window)->shelf_widget();
-      shelf->HideShelfBehindBlackBar(false, duration_override);
-    }
-    // We kicked off the shelf animation above and the override can be
-    // removed.
-    duration_override = 0;
-  }
-
-  if (!animation_speed_ms_ || animation_step == ANIMATION_STEP_FINALIZE)
-    return;
-
-  // Note: The animation duration override will be set before the old user gets
-  // hidden and reset after the animations for the new user got kicked off.
-  ash::Shell::RootWindowControllerList controller =
-      ash::Shell::Get()->GetAllRootWindowControllers();
-  for (ash::Shell::RootWindowControllerList::iterator iter = controller.begin();
-       iter != controller.end(); ++iter) {
-    (*iter)->GetShelfLayoutManager()->SetAnimationDurationOverride(
-        duration_override);
-  }
-
-  if (animation_step != ANIMATION_STEP_HIDE_OLD_USER)
+  if (animation_step != ANIMATION_STEP_SHOW_NEW_USER)
     return;
 
-  // For each root window hide the shelf.
-  for (aura::Window* window : ash::Shell::GetAllRootWindows()) {
-    // Hiding the shelf will cause a resize on a maximized window.
-    // If the shelf is then shown for the following user in the same location,
-    // the window gets resized again. Since each resize can cause a considerable
-    // CPU usage and therefore effect jank, we should avoid hiding the shelf if
-    // the start and end location are the same and cover the shelf instead with
-    // a black rectangle on top.
-    ash::Shelf* shelf = ash::Shelf::ForWindow(window);
-    if (GetScreenCover(window) != NO_USER_COVERS_SCREEN &&
-        (!chrome_launcher_controller ||
-         !chrome_launcher_controller->ShelfBoundsChangesProbablyWithUser(
-             shelf, new_account_id_))) {
-      shelf->shelf_widget()->HideShelfBehindBlackBar(true, duration_override);
-    } else {
-      // This shelf change is only part of the animation and will be updated by
-      // ChromeLauncherController::ActiveUserChanged() to the new users value.
-      // Note that the user preference will not be changed.
-      shelf->SetAutoHideBehavior(ash::SHELF_AUTO_HIDE_ALWAYS_HIDDEN);
-    }
+  ChromeLauncherController* chrome_launcher_controller =
+      ChromeLauncherController::instance();
+  // Some unit tests have no ChromeLauncherController.
+  if (chrome_launcher_controller) {
+    chrome_launcher_controller->ActiveUserChanged(
+        new_account_id_.GetUserEmail());
   }
 }
 

chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.h

@@ -79,7 +79,7 @@ class UserSwitchAnimatorChromeOS {
   // Execute the user wallpaper animations for |animation_step|.
   void TransitionWallpaper(AnimationStep animtion_step);
 
-  // Execute the user shelf animations for |animation_step|.
+  // Update the shelf for |animation_step|.
   void TransitionUserShelf(AnimationStep animtion_step);
 
   // Execute the window animations for |animation_step|.
@@ -108,7 +108,7 @@ class UserSwitchAnimatorChromeOS {
   AnimationStep animation_step_;
 
   // The screen cover status before the animation has started.
-  TransitioningScreenCover screen_cover_;
+  const TransitioningScreenCover screen_cover_;
 
   // Mapping users IDs to the list of windows to show for these users.
   typedef std::map<AccountId, aura::Window::Windows> UserToWindowsMap;