cros: Cleanup shelf background animator

ShelfBackgroundAnimator determined color by looking at both ShelfBackgroundType
and also the session state. ShelfBackgroundType was set from ShelfLayoutManager.

Both ShelfLayoutManager and ShelfBackgroundAnimator observed session state
changes and updated the shelf colors from that. This means that there were
multiple color updates, and beyond that the rendered background type could
diverge from what was requested by ShelfLayoutManager in the time between the
two callbacks.

Remove the ability for ShelfLayoutManager to observe session state changes, and
encode the needed information in ShelfBackgroundType. This simplifies control
flow and reduces complexity.

Bug: 857221
Change-Id: I173e8cc47d7f6f22123814d341a94749472f7932
Reviewed-on: https://chromium-review.googlesource.com/1168028
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582259}

ash/public/cpp/shelf_types.h

@@ -65,7 +65,14 @@ enum ShelfBackgroundType {
   // The background when split view mode is active.
   SHELF_BACKGROUND_SPLIT_VIEW,
 
-  // The background when login/lock is active and the wallpaper is not blurred.
+  // The background when OOBE is active.
+  SHELF_BACKGROUND_OOBE,
+
+  // The background when login/lock/user-add is active.
+  SHELF_BACKGROUND_LOGIN,
+
+  // The background when login/lock/user-add is active and the wallpaper is not
+  // blurred.
   SHELF_BACKGROUND_LOGIN_NONBLURRED_WALLPAPER,
 };
 

ash/shelf/shelf_background_animator.cc

@@ -11,7 +11,6 @@
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/public/cpp/login_constants.h"
 #include "ash/public/cpp/wallpaper_types.h"
-#include "ash/session/session_controller.h"
 #include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_background_animator_observer.h"
 #include "ash/shelf/shelf_constants.h"
@@ -59,42 +58,53 @@ ColorProfile GetShelfColorProfile() {
 // given |background_type|.
 std::pair<int, int> GetTargetColorAlphaValues(
     ShelfBackgroundType background_type) {
-  int target_shelf_color_alpha = 0;
-  int target_item_color_alpha = 0;
+  int target_shelf_color_alpha = SK_AlphaTRANSPARENT;
+  int target_item_color_alpha = SK_AlphaTRANSPARENT;
 
   switch (background_type) {
     case SHELF_BACKGROUND_DEFAULT:
       if (chromeos::switches::ShouldUseShelfNewUi()) {
         target_shelf_color_alpha = kShelfTranslucentAlpha;
-        target_item_color_alpha = 0;
+        target_item_color_alpha = SK_AlphaTRANSPARENT;
       } else {
-        target_shelf_color_alpha = 0;
+        target_shelf_color_alpha = SK_AlphaTRANSPARENT;
         target_item_color_alpha = kShelfTranslucentAlpha;
       }
       break;
     case SHELF_BACKGROUND_OVERLAP:
       if (chromeos::switches::ShouldUseShelfNewUi()) {
         target_shelf_color_alpha = kShelfTranslucentWithOverlapAlphaNewUi;
-        target_item_color_alpha = 0;
+        target_item_color_alpha = SK_AlphaTRANSPARENT;
       } else {
         target_shelf_color_alpha = kShelfTranslucentAlpha;
-        target_item_color_alpha = 0;
+        target_item_color_alpha = SK_AlphaTRANSPARENT;
       }
       break;
     case SHELF_BACKGROUND_MAXIMIZED:
       target_shelf_color_alpha = ShelfBackgroundAnimator::kMaxAlpha;
-      target_item_color_alpha = 0;
+      target_item_color_alpha = SK_AlphaTRANSPARENT;
       break;
     case SHELF_BACKGROUND_APP_LIST:
-      target_shelf_color_alpha = 0;
-      target_item_color_alpha = 0;
+      target_shelf_color_alpha = SK_AlphaTRANSPARENT;
+      target_item_color_alpha = SK_AlphaTRANSPARENT;
       break;
     case SHELF_BACKGROUND_SPLIT_VIEW:
       target_shelf_color_alpha = ShelfBackgroundAnimator::kMaxAlpha;
-      target_item_color_alpha = 0;
+      target_item_color_alpha = SK_AlphaTRANSPARENT;
+      break;
+    case SHELF_BACKGROUND_OOBE:
+      target_shelf_color_alpha = SK_AlphaTRANSPARENT;
+      target_item_color_alpha = SK_AlphaOPAQUE;
+      break;
+    case SHELF_BACKGROUND_LOGIN:
+      target_shelf_color_alpha = SK_AlphaTRANSPARENT;
+      target_item_color_alpha = SK_AlphaTRANSPARENT;
       break;
     case SHELF_BACKGROUND_LOGIN_NONBLURRED_WALLPAPER:
-      NOTREACHED();
+      target_shelf_color_alpha =
+          login_constants::kNonBlurredWallpaperBackgroundAlpha;
+      target_item_color_alpha =
+          login_constants::kNonBlurredWallpaperBackgroundAlpha;
       break;
   }
   return std::pair<int, int>(target_shelf_color_alpha, target_item_color_alpha);
@@ -127,8 +137,6 @@ ShelfBackgroundAnimator::ShelfBackgroundAnimator(
     Shelf* shelf,
     WallpaperController* wallpaper_controller)
     : shelf_(shelf), wallpaper_controller_(wallpaper_controller) {
-  if (Shell::HasInstance())  // Null in testing::Test.
-    Shell::Get()->session_controller()->AddObserver(this);
   if (wallpaper_controller_)
     wallpaper_controller_->AddObserver(this);
   if (shelf_)
@@ -144,8 +152,6 @@ ShelfBackgroundAnimator::~ShelfBackgroundAnimator() {
     wallpaper_controller_->RemoveObserver(this);
   if (shelf_)
     shelf_->RemoveObserver(this);
-  if (Shell::HasInstance())  // Null in testing::Test.
-    Shell::Get()->session_controller()->RemoveObserver(this);
 }
 
 void ShelfBackgroundAnimator::AddObserver(
@@ -197,11 +203,6 @@ void ShelfBackgroundAnimator::OnWallpaperColorsChanged() {
   AnimateBackground(target_background_type_, AnimationChangeType::ANIMATE);
 }
 
-void ShelfBackgroundAnimator::OnSessionStateChanged(
-    session_manager::SessionState state) {
-  AnimateBackground(target_background_type_, AnimationChangeType::ANIMATE);
-}
-
 void ShelfBackgroundAnimator::OnBackgroundTypeChanged(
     ShelfBackgroundType background_type,
     AnimationChangeType change_type) {
@@ -270,6 +271,8 @@ void ShelfBackgroundAnimator::CreateAnimator(
       break;
     case SHELF_BACKGROUND_MAXIMIZED:
     case SHELF_BACKGROUND_SPLIT_VIEW:
+    case SHELF_BACKGROUND_OOBE:
+    case SHELF_BACKGROUND_LOGIN:
     case SHELF_BACKGROUND_LOGIN_NONBLURRED_WALLPAPER:
       duration_ms = 250;
       break;
@@ -294,71 +297,55 @@ void ShelfBackgroundAnimator::GetTargetValues(
     ShelfBackgroundType background_type,
     AnimationValues* shelf_background_values,
     AnimationValues* item_background_values) const {
-  // Always use the login base color if the wallpaper is not blurred.
-  if (background_type == SHELF_BACKGROUND_LOGIN_NONBLURRED_WALLPAPER) {
-    auto color =
-        SkColorSetA(login_constants::kDefaultBaseColor,
-                    login_constants::kNonBlurredWallpaperBackgroundAlpha);
-    shelf_background_values->SetTargetValues(color);
-    item_background_values->SetTargetValues(color);
-    return;
-  }
+  // Fetches wallpaper color and darkens it.
+  auto darken_wallpaper = [&](int darkening_alpha) {
+    if (!wallpaper_controller_)
+      return kShelfDefaultBaseColor;
+    SkColor target_color =
+        wallpaper_controller_->GetProminentColor(GetShelfColorProfile());
+    if (target_color == kInvalidWallpaperColor)
+      return kShelfDefaultBaseColor;
+    return color_utils::GetResultingPaintColor(
+        SkColorSetA(kShelfDefaultBaseColor, darkening_alpha), target_color);
+  };
 
-  // Shell may not have instance in tests.
-  if (Shell::HasInstance()) {
-    auto session_state = Shell::Get()->session_controller()->GetSessionState();
-
-    // OOBE always uses a fixed shelf color.
-    if (session_state == session_manager::SessionState::OOBE) {
-      shelf_background_values->SetTargetValues(SK_ColorTRANSPARENT);
-      item_background_values->SetTargetValues(gfx::kGoogleGrey100);
-      return;
-    }
-
-    // All other non-active sessions use transparent colors.
-    if (session_state != session_manager::SessionState::ACTIVE) {
-      shelf_background_values->SetTargetValues(SK_ColorTRANSPARENT);
-      item_background_values->SetTargetValues(SK_ColorTRANSPARENT);
-      return;
-    }
+  SkColor shelf_target_color = kShelfDefaultBaseColor;
+  SkColor item_target_color = kShelfDefaultBaseColor;
+  switch (background_type) {
+    case SHELF_BACKGROUND_DEFAULT:
+    case SHELF_BACKGROUND_OVERLAP:
+    case SHELF_BACKGROUND_APP_LIST:
+      shelf_target_color = darken_wallpaper(kShelfTranslucentColorDarkenAlpha);
+      item_target_color = shelf_target_color;
+      break;
+    case SHELF_BACKGROUND_MAXIMIZED:
+      shelf_target_color = darken_wallpaper(kShelfOpaqueColorDarkenAlpha);
+      item_target_color = shelf_target_color;
+      break;
+    case SHELF_BACKGROUND_SPLIT_VIEW:
+      shelf_target_color = darken_wallpaper(ShelfBackgroundAnimator::kMaxAlpha);
+      item_target_color = shelf_target_color;
+      break;
+    case SHELF_BACKGROUND_OOBE:
+      shelf_target_color = SK_ColorTRANSPARENT;
+      item_target_color = gfx::kGoogleGrey100;
+      break;
+    case SHELF_BACKGROUND_LOGIN:
+      shelf_target_color = SK_ColorTRANSPARENT;
+      item_target_color = SK_ColorTRANSPARENT;
+      break;
+    case SHELF_BACKGROUND_LOGIN_NONBLURRED_WALLPAPER:
+      shelf_target_color = login_constants::kDefaultBaseColor;
+      item_target_color = login_constants::kDefaultBaseColor;
+      break;
   }
 
   std::pair<int, int> target_color_alpha_values =
       GetTargetColorAlphaValues(background_type);
-
-  SkColor target_color =
-      wallpaper_controller_
-          ? wallpaper_controller_->GetProminentColor(GetShelfColorProfile())
-          : kShelfDefaultBaseColor;
-  if (target_color == kInvalidWallpaperColor) {
-    target_color = kShelfDefaultBaseColor;
-  } else {
-    int darkening_alpha = 0;
-
-    switch (background_type) {
-      case SHELF_BACKGROUND_DEFAULT:
-      case SHELF_BACKGROUND_OVERLAP:
-      case SHELF_BACKGROUND_APP_LIST:
-        darkening_alpha = kShelfTranslucentColorDarkenAlpha;
-        break;
-      case SHELF_BACKGROUND_MAXIMIZED:
-        darkening_alpha = kShelfOpaqueColorDarkenAlpha;
-        break;
-      case SHELF_BACKGROUND_SPLIT_VIEW:
-        darkening_alpha = ShelfBackgroundAnimator::kMaxAlpha;
-        break;
-      case SHELF_BACKGROUND_LOGIN_NONBLURRED_WALLPAPER:
-        NOTREACHED();
-        break;
-    }
-    target_color = color_utils::GetResultingPaintColor(
-        SkColorSetA(kShelfDefaultBaseColor, darkening_alpha), target_color);
-  }
-
   shelf_background_values->SetTargetValues(
-      SkColorSetA(target_color, target_color_alpha_values.first));
+      SkColorSetA(shelf_target_color, target_color_alpha_values.first));
   item_background_values->SetTargetValues(
-      SkColorSetA(target_color, target_color_alpha_values.second));
+      SkColorSetA(item_target_color, target_color_alpha_values.second));
 }
 
 void ShelfBackgroundAnimator::SetAnimationValues(double t) {

ash/shelf/shelf_background_animator.h

@@ -10,7 +10,6 @@
 
 #include "ash/ash_export.h"
 #include "ash/public/cpp/shelf_types.h"
-#include "ash/session/session_observer.h"
 #include "ash/shelf/shelf_observer.h"
 #include "ash/wallpaper/wallpaper_controller_observer.h"
 #include "base/macros.h"
@@ -43,8 +42,7 @@ class WallpaperController;
 //       states.
 class ASH_EXPORT ShelfBackgroundAnimator : public ShelfObserver,
                                            public gfx::AnimationDelegate,
-                                           public WallpaperControllerObserver,
-                                           public SessionObserver {
+                                           public WallpaperControllerObserver {
  public:
   // The maximum alpha value that can be used.
   static const int kMaxAlpha = SK_AlphaOPAQUE;
@@ -96,9 +94,6 @@ class ASH_EXPORT ShelfBackgroundAnimator : public ShelfObserver,
   // WallpaperControllerObserver:
   void OnWallpaperColorsChanged() override;
 
-  // SessionObserver:
-  void OnSessionStateChanged(session_manager::SessionState state) override;
-
  private:
   friend class ShelfBackgroundAnimatorTestApi;
 

ash/shelf/shelf_background_animator_unittest.cc

@@ -9,8 +9,11 @@
 #include "ash/animation/animation_change_type.h"
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/session/test_session_controller_client.h"
+#include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_background_animator_observer.h"
 #include "ash/shelf/shelf_constants.h"
+#include "ash/shelf/shelf_widget.h"
+#include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "base/bind.h"
 #include "base/command_line.h"
@@ -335,10 +338,6 @@ class ShelfBackgroundTargetColorTest : public NoSessionAshTestBase {
     base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
         switches::kAshShelfColor, switches::kAshShelfColorDisabled);
     AshTestBase::SetUp();
-
-    animator_.reset(new ShelfBackgroundAnimator(SHELF_BACKGROUND_DEFAULT,
-                                                nullptr, nullptr));
-    test_api_.reset(new ShelfBackgroundAnimatorTestApi(animator_.get()));
   }
 
  protected:
@@ -347,12 +346,6 @@ class ShelfBackgroundTargetColorTest : public NoSessionAshTestBase {
     GetSessionControllerClient()->SetSessionState(state);
   }
 
-  // Test target.
-  std::unique_ptr<ShelfBackgroundAnimator> animator_;
-
-  // Provides internal access to |animator_|.
-  std::unique_ptr<ShelfBackgroundAnimatorTestApi> test_api_;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(ShelfBackgroundTargetColorTest);
 };
@@ -363,63 +356,70 @@ class ShelfBackgroundTargetColorTest : public NoSessionAshTestBase {
 // |ShelfBackgroundAnimatorTest|.
 TEST_F(ShelfBackgroundTargetColorTest,
        ShelfAndItemBackgroundColorUpdatedWithSessionState) {
+  // Use the real ShelfBackgroundAnimator instance because it needs to update
+  // for session state changes.
+  ShelfBackgroundAnimatorTestApi test_api(
+      Shelf::ForWindow(Shell::Get()->GetPrimaryRootWindow())
+          ->shelf_widget()
+          ->background_animator_for_testing());
+
   // The shelf is not initialized until session state becomes active, so the
   // following two cases don't have visible effects until we support views-based
   // shelf for all session states, but it's still good to check them here.
   NotifySessionStateChanged(session_manager::SessionState::OOBE);
-  EXPECT_EQ(GetBaseColor(test_api_->shelf_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.shelf_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
-  EXPECT_EQ(GetBaseColor(test_api_->item_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.item_background_target_color()),
             GetBaseColor(gfx::kGoogleGrey100));
 
   NotifySessionStateChanged(session_manager::SessionState::LOGIN_PRIMARY);
-  EXPECT_EQ(GetBaseColor(test_api_->shelf_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.shelf_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
-  EXPECT_EQ(GetBaseColor(test_api_->item_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.item_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
 
   SimulateUserLogin("user1@test.com");
 
   NotifySessionStateChanged(
       session_manager::SessionState::LOGGED_IN_NOT_ACTIVE);
-  EXPECT_EQ(GetBaseColor(test_api_->shelf_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.shelf_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
-  EXPECT_EQ(GetBaseColor(test_api_->item_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.item_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
 
   // The shelf has a non-transparent background only when session state is
   // active.
   NotifySessionStateChanged(session_manager::SessionState::ACTIVE);
-  EXPECT_EQ(GetBaseColor(test_api_->shelf_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.shelf_background_target_color()),
             GetBaseColor(kShelfDefaultBaseColor));
-  EXPECT_EQ(GetBaseColor(test_api_->item_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.item_background_target_color()),
             GetBaseColor(kShelfDefaultBaseColor));
 
   NotifySessionStateChanged(session_manager::SessionState::LOCKED);
-  EXPECT_EQ(GetBaseColor(test_api_->shelf_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.shelf_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
-  EXPECT_EQ(GetBaseColor(test_api_->item_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.item_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
 
   // Ensure the shelf background color is correct after unlocking.
   NotifySessionStateChanged(session_manager::SessionState::ACTIVE);
-  EXPECT_EQ(GetBaseColor(test_api_->shelf_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.shelf_background_target_color()),
             GetBaseColor(kShelfDefaultBaseColor));
-  EXPECT_EQ(GetBaseColor(test_api_->item_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.item_background_target_color()),
             GetBaseColor(kShelfDefaultBaseColor));
 
   NotifySessionStateChanged(session_manager::SessionState::LOGIN_SECONDARY);
-  EXPECT_EQ(GetBaseColor(test_api_->shelf_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.shelf_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
-  EXPECT_EQ(GetBaseColor(test_api_->item_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.item_background_target_color()),
             GetBaseColor(SK_ColorTRANSPARENT));
 
   // Ensure the shelf background color is correct after closing the user adding
   // screen.
   NotifySessionStateChanged(session_manager::SessionState::ACTIVE);
-  EXPECT_EQ(GetBaseColor(test_api_->shelf_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.shelf_background_target_color()),
             GetBaseColor(kShelfDefaultBaseColor));
-  EXPECT_EQ(GetBaseColor(test_api_->item_background_target_color()),
+  EXPECT_EQ(GetBaseColor(test_api.item_background_target_color()),
             GetBaseColor(kShelfDefaultBaseColor));
 }
 

ash/shelf/shelf_layout_manager.cc

@@ -518,11 +518,13 @@ ShelfBackgroundType ShelfLayoutManager::GetShelfBackgroundType() const {
   if (state_.pre_lock_screen_animation_active)
     return SHELF_BACKGROUND_DEFAULT;
 
-  // Handle all non active screen states, including OOBE and pre-login.
+  // Handle all other non active screen states, including OOBE and pre-login.
+  if (state_.session_state == session_manager::SessionState::OOBE)
+    return SHELF_BACKGROUND_OOBE;
   if (state_.session_state != session_manager::SessionState::ACTIVE) {
     if (!Shell::Get()->wallpaper_controller()->IsWallpaperBlurred())
       return SHELF_BACKGROUND_LOGIN_NONBLURRED_WALLPAPER;
-    return SHELF_BACKGROUND_OVERLAP;
+    return SHELF_BACKGROUND_LOGIN;
   }
 
   // If the app list is active, hide the shelf background to prevent overlap.

ash/shelf/shelf_layout_manager_unittest.cc

@@ -968,7 +968,7 @@ TEST_F(ShelfLayoutManagerTest, VisibleWhenLoginScreenShowing) {
 
   // Blurred wallpaper.
   ash::Shell::Get()->wallpaper_controller()->UpdateWallpaperBlur(/*blur=*/true);
-  EXPECT_EQ(SHELF_BACKGROUND_OVERLAP, GetShelfWidget()->GetBackgroundType());
+  EXPECT_EQ(SHELF_BACKGROUND_LOGIN, GetShelfWidget()->GetBackgroundType());
 
   // Non-blurred wallpaper.
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
@@ -1003,7 +1003,7 @@ TEST_F(ShelfLayoutManagerTest, VisibleWhenLockScreenShowing) {
   LockScreen();
   // Showing a widget in the lock screen should force the shelf to be visible.
   EXPECT_EQ(SHELF_VISIBLE, shelf->GetVisibilityState());
-  EXPECT_EQ(SHELF_BACKGROUND_OVERLAP, GetShelfWidget()->GetBackgroundType());
+  EXPECT_EQ(SHELF_BACKGROUND_LOGIN, GetShelfWidget()->GetBackgroundType());
 
   UnlockScreen();
   EXPECT_EQ(SHELF_AUTO_HIDE, shelf->GetVisibilityState());

ash/shelf/shelf_widget.h

@@ -116,6 +116,9 @@ class ASH_EXPORT ShelfWidget : public views::Widget,
 
   // Internal implementation detail. Do not expose outside of tests.
   ShelfView* shelf_view_for_testing() const { return shelf_view_; }
+  ShelfBackgroundAnimator* background_animator_for_testing() {
+    return &background_animator_;
+  }
 
   void set_activated_from_overflow_bubble(bool val) {
     activated_from_overflow_bubble_ = val;