Add lock pod animation on unlock.

Add lock pod animation on unlock.

BUG=138171
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164528

[Denis Kuznetsov (DE-MUC), 2012-10-25 12:50:27]

The best approach I could come to in doing this part of animation.

[Daniel Erat, 2012-10-25 15:45:57]

LGTM after these comments are addressed

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
File ash/wm/session_state_animator.cc (right):

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
ash/wm/session_state_animator.cc:176: // A bit dirty trick: we need to catch the
end of the animation we don't
nit: s/bit dirty/bit of a dirty/ (same below)

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
ash/wm/session_state_animator.cc:181: ui::LayerAnimator* animator =
window->layer()->GetAnimator();
if you only need this if an observer has been supplied, can you move all of it
into an "if (observer)" block?  (same below)

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
ash/wm/session_state_animator.cc:401: new CallbackAnimationObserver(callback);
nit: indent four spaces, not three

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
File ash/wm/session_state_animator.h (right):

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
ash/wm/session_state_animator.h:132: void StartAnimation(int container_mask,
i'd suggest naming this StartAnimationWithCallback() instead
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Overl...)

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
File ash/wm/session_state_controller.h (right):

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
ash/wm/session_state_controller.h:136: // |callback| must be called when all
animations are done.
nit: s/must/will/ ?

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
File ash/wm/session_state_controller_impl.h (right):

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
ash/wm/session_state_controller_impl.h:99: // Returns true iff when we're in
state when user session can be locked.
nit (doesn't need to be addressed in this change): add a "SessionStateController
overrides:" comment and delete all of the per-method comments, as is done above
for other overrides.  (otherwise, these comments will get stale as the versions
in the base class are updated)

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
File ash/wm/session_state_controller_impl2.cc (right):

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
ash/wm/session_state_controller_impl2.cc:13: #include "base/bind_helpers.h"
nit: keep this list alphabetized

https://chromiumcodereview.appspot.com/11276025/diff/4001/ash/wm/session_stat...
ash/wm/session_state_controller_impl2.cc:297: animator_->StartAnimation(
nit: indent two spaces instead of four

https://chromiumcodereview.appspot.com/11276025/diff/4001/chrome/browser/chro...
File chrome/browser/chromeos/login/screen_locker.cc (right):

https://chromiumcodereview.appspot.com/11276025/diff/4001/chrome/browser/chro...
chrome/browser/chromeos/login/screen_locker.cc:318: if (NULL == screen_locker_)
nit: use "screen_locker_ != NULL" instead (it's easier to read and is
overwhelmingly more common in chrome -- the compiler should warn on assignment
in conditionals)

[commit-bot: I haz the power, 2012-10-26 09:22:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/antrim@chromium.org/11276025/11001

[commit-bot: I haz the power, 2012-10-26 09:40:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/antrim@chromium.org/11276025/11001

[commit-bot: I haz the power, 2012-10-26 10:01:54]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-26 10:02:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/antrim@chromium.org/11276025/15002

[Nikita (slow), 2012-10-26 10:34:35]

I don't see cros_* / win_aura trybot runs.

http://codereview.chromium.org/11276025/diff/15002/ash/wm/session_state_anima...
File ash/wm/session_state_animator.cc (right):

http://codereview.chromium.org/11276025/diff/15002/ash/wm/session_state_anima...
ash/wm/session_state_animator.cc:184: if (observer)
nit: You could move everything under the if (observer) { ... }

http://codereview.chromium.org/11276025/diff/15002/ash/wm/session_state_anima...
ash/wm/session_state_animator.cc:207: if (observer)
nit: same here

[Nikita (slow), 2012-10-26 10:37:46]

lgtm with my nit comments (fine to fix in the upcoming CLs with the rest of
implementation with tests).

http://codereview.chromium.org/11276025/diff/15002/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/screen_locker.cc (right):

http://codereview.chromium.org/11276025/diff/15002/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/screen_locker.cc:316: void
ScreenLocker::ScheduleDeletion() {
nit: // static

[Nikita (slow), 2012-10-26 10:40:49]

On 2012/10/26 10:34:35, Nikita Kostylev wrote:
> I don't see cros_* / win_aura trybot runs.

Also full linux_chromeos trybot run is missing.

Please make sure you run all of in the future CLs to avoid breaking the tree.

[commit-bot: I haz the power, 2012-10-26 17:54:38]

Retried try job too often for step(s) sync_integration_tests

[commit-bot: I haz the power, 2012-10-27 07:34:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/antrim@chromium.org/11276025/15002

[commit-bot: I haz the power, 2012-10-27 12:23:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/antrim@chromium.org/11276025/15002

[commit-bot: I haz the power, 2012-10-27 12:25:39]

Change committed as 164528