Improve existing lock transition - remove black splash.

ash/wm/power_button_controller.h

 // 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.
 
 #ifndef ASH_WM_POWER_BUTTON_CONTROLLER_H_
 #define ASH_WM_POWER_BUTTON_CONTROLLER_H_
 
 #include "ash/ash_export.h"
 #include "ash/shell_observer.h"
 #include "base/basictypes.h"
@@ skipping 35 matching lines @@
   // Exposed here for TestApi::ContainerGroupIsAnimated().
   enum AnimationType {
     ANIMATION_SLOW_CLOSE = 0,
     ANIMATION_UNDO_SLOW_CLOSE,
     ANIMATION_FAST_CLOSE,
     ANIMATION_FADE_IN,
     ANIMATION_HIDE,
     ANIMATION_RESTORE,
   };
 
-  // Groups of containers that can be animated.
+  // Specific containers or group of containers that can be animated.

[Daniel Erat, 2012/09/04 16:40:18]

nit: s/group/groups/

[Nikita (slow), 2012/09/04 17:36:42]

Done.

   // Exposed here for TestApi::ContainerGroupIsAnimated().
   enum ContainerGroup {
-    ALL_CONTAINERS = 0,
-    SCREEN_LOCKER_CONTAINERS,
-    SCREEN_LOCKER_AND_RELATED_CONTAINERS,
-    ALL_BUT_SCREEN_LOCKER_AND_RELATED_CONTAINERS,
+    DESKTOP_WALLPAPER = 1 << 0,

[Daniel Erat, 2012/09/04 16:40:18]

use consistent terminology: you're using "background" for the window id but
"wallpaper" here

[Nikita (slow), 2012/09/04 17:36:42]

Done.

+
+    // All user session related containers including system background but
+    // not including desktop background (wallpaper).
+    NON_LOCK_SCREEN_CONTAINERS = 1 << 1,
+
+    // Desktop wallpaper is moved to this layer when screen is locked.

[Daniel Erat, 2012/09/04 16:40:18]

asked over IM, but perhaps you're not there: i didn't realize earlier that the
wallpaper was being moved between layers.  is there any reason that it can't
just be moved from NonLockScreenContainersContainer to
LockScreenBackgroundContainer, so that none of the changes to
PowerButtonController are necessary?  where's the change that does the moving?

[Nikita (slow), 2012/09/04 17:36:42]

http://codereview.chromium.org/10810039/
Discussed over IM.

+    // This layer is excluded from lock animation so that wallpaper stays as is,
+    // user session windows are hidden and lock UI is shown on top of it.
+    // This layer is included in shutdown animation.
+    LOCK_SCREEN_WALLPAPER = 1 << 2,

[Daniel Erat, 2012/09/04 16:40:18]

same background vs. wallpaper issue here

[Nikita (slow), 2012/09/04 17:36:42]

Done.

+
+    // Lock screen and lock screen modal containers.
+    LOCK_SCREEN_CONTAINERS = 1 << 3,
+
+    // Multiple system layers belong here like status, menu, tooltip
+    // and overlay layers.
+    LOCK_SCREEN_RELATED_CONTAINERS = 1 << 4,
   };
 
   // Helper class used by tests to access internal state.
   class ASH_EXPORT TestApi {
    public:
     explicit TestApi(PowerButtonController* controller)
         : controller_(controller) {}
 
     bool lock_timer_is_running() const {
       return controller_->lock_timer_.IsRunning();
@@ skipping 32 matching lines @@
     }
     void trigger_real_shutdown_timeout() {
       controller_->OnRealShutdownTimeout();
       controller_->real_shutdown_timer_.Stop();
     }
     void trigger_hide_background_layer_timeout() {
       controller_->HideBackgroundLayer();
       controller_->hide_background_layer_timer_.Stop();
     }
 
-    // Returns true if the given set of containers was last animated with
-    // |type| (probably; the analysis is fairly ad-hoc).
-    bool ContainerGroupIsAnimated(ContainerGroup group,
-                                  AnimationType type) const;
+    // Returns true if containers of a given |container_mask|
+    // was last animated with |type| (probably; the analysis is fairly ad-hoc).

[Daniel Erat, 2012/09/04 16:40:18]

nit: s/was/were/

[Nikita (slow), 2012/09/04 17:36:42]

Done.

+    // |container_mask| is a bitfield of a ContainerGroup.

[Daniel Erat, 2012/09/04 16:40:18]

nit: s/a ContainerGroup/ContainerGroups/

[Nikita (slow), 2012/09/04 17:36:42]

As discussed renamed to Container

+    bool ContainerGroupIsAnimated(int container_mask, AnimationType type) const;

[Daniel Erat, 2012/09/04 16:40:18]

nit: s/ContainerGroupIs/ContainerGroupsAre/

[Nikita (slow), 2012/09/04 17:36:42]

Done.

 
     // Returns true if |background_layer_| is non-NULL and visible.
     bool BackgroundLayerIsVisible() const;
 
     // Returns |background_layer_|'s bounds, or an empty rect if the layer is
     // NULL.
     gfx::Rect GetBackgroundLayerBounds() const;
 
    private:
     PowerButtonController* controller_;  // not owned
 
     DISALLOW_COPY_AND_ASSIGN(TestApi);
   };
 
+  // Helper method that returns bitfield mask of all containers.

[Daniel Erat, 2012/09/04 16:40:18]

nit: s/returns/returns a/

[Nikita (slow), 2012/09/04 17:36:42]

Done.

+  static int GetAllContainersMask();
+
+  // Helper method that returns bitfield mask of all lock screen containers.

[Daniel Erat, 2012/09/04 16:40:18]

nit: s/returns/returns a/

the comment here makes it sound like this method just returns
LOCK_SCREEN_CONTAINERS.  change it to "mask including LOCK_SCREEN_WALLPAPER,
LOCK_SCREEN_CONTAINERS, and LOCK_SCREEN_RELATED_CONTAINERS", assuming that
that's what it does

[Nikita (slow), 2012/09/04 17:36:42]

Done.

+  static int GetAllScreenLockContainersMask();

[Daniel Erat, 2012/09/04 16:40:18]

nit: s/ScreenLock/LockScreen/, so this is consistent with the mask names

[Nikita (slow), 2012/09/04 17:36:42]

Done.

+
   PowerButtonController();
   virtual ~PowerButtonController();
 
   void set_delegate(PowerButtonControllerDelegate* delegate) {
     delegate_.reset(delegate);
   }
 
   void set_has_legacy_power_button_for_test(bool legacy) {
     has_legacy_power_button_ = legacy;
   }
@@ skipping 63 matching lines @@
   // Are the power or lock buttons currently held?
   bool power_button_down_;
   bool lock_button_down_;
 
   // Is the screen currently turned off?
   bool screen_is_off_;
 
   // Are we in the process of shutting the machine down?
   bool shutting_down_;
 
-  // Should we start |shutdown_timer_| when we receive notification that the
-  // screen has been locked?
-  bool should_start_shutdown_timer_after_lock_;

[Daniel Erat, 2012/09/04 16:40:18]

thanks for deleting this

-
   // Was a command-line switch set telling us that we're running on hardware
   // that misreports power button releases?
   bool has_legacy_power_button_;
 
   // Layer that's stacked under all of the root window's children to provide a
   // black background when we're scaling all of the other windows down.
   // TODO(derat): Remove this in favor of having the compositor only clear the
   // viewport when there are regions not covered by a layer:
   // http://crbug.com/113445
   scoped_ptr<ui::Layer> background_layer_;
@@ skipping 25 matching lines @@
   // |background_layer_|, as the desktop background is now covering the whole
   // screen.
   base::OneShotTimer<PowerButtonController> hide_background_layer_timer_;
 
   DISALLOW_COPY_AND_ASSIGN(PowerButtonController);
 };
 
 }  // namespace ash
 
 #endif  // ASH_WM_POWER_BUTTON_CONTROLLER_H_