Add more tests to test OverviewButtonTray.

Add more tests to test OverviewButtonTray and ScreenRotationAnimator.

Add more tests to test the hide animation of OverviewButtonTray and interaction with ScreenRotationAnimator.

BUG=718198
TEST=OverviewButtonTrayTest.HideAnimationAlwaysCompletes* and ScreenRotationAnimator[Smooth|Slow]AnimationTest.OverviewButtonTrayHideAnimation*

Review-Url: https://codereview.chromium.org/2845123003
Cr-Commit-Position: refs/heads/master@{#476111}
Committed: https://chromium.googlesource.com/chromium/src/+/310d861a8399fae1b082cd0d83db69b98d1df7da

[wutao, 2017-04-27 17:49:13]

wutao@chromium.org changed reviewers:
+ bruthig@chromium.org

[wutao, 2017-04-27 17:49:14]

Hi Ben,
Please have a look first on the tests added for OverviewButtonTrayTest.

Thanks,
Tao

[wutao, 2017-04-27 17:49:27]

The CQ bit was checked by wutao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 17:49:55]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-27 18:06:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-27 18:06:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bruthig, 2017-04-27 20:41:35]

Hey Tao,

Just a few notes but feel free to add OWNERs.

BTW I am OOO tomorrow.

Cheers,
Ben

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
File ash/system/overview/overview_button_tray_unittest.cc (right):

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
ash/system/overview/overview_button_tray_unittest.cc:236:
std::unique_ptr<ui::ScopedAnimationDurationScaleMode> rotate_duration(
Is |rotate_duration| required?

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
ash/system/overview/overview_button_tray_unittest.cc:247:
TEST_F(OverviewButtonTrayTest, HideAnimationAlwaysCompletesOnDelete) {
IMO the test above replaces this one and you can nuke this one.

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
ash/system/overview/overview_button_tray_unittest.cc:263:
std::unique_ptr<ui::ScopedAnimationDurationScaleMode> rotate_duration(
Is |rotate_duration| still required?

[wutao, 2017-04-28 21:21:29]

wutao@chromium.org changed reviewers:
+ jonross@chromium.org

[wutao, 2017-04-28 21:21:30]

+jonross@.

OnAbort for VisibilityTransition does not progress visibility to the target
value. So we need to change the code here [1] or implementation of the
OverviewButtonTray if we need this. Is there a case we will abort animation and
but care about the visibility?

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
File ash/system/overview/overview_button_tray_unittest.cc (right):

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
ash/system/overview/overview_button_tray_unittest.cc:236:
std::unique_ptr<ui::ScopedAnimationDurationScaleMode> rotate_duration(
On 2017/04/27 20:41:35, bruthig wrote:
> Is |rotate_duration| required?

Found out OnAbort for VisibilityTransition does not progress to the target. So
we need to change the code here [1] or implementation of the OverviewButtonTray
if we need this. Is there a case we will abort animation and but care about the
visibility?

[1]
https://cs.chromium.org/chromium/src/ui/compositor/layer_animation_element.cc...

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
ash/system/overview/overview_button_tray_unittest.cc:247:
TEST_F(OverviewButtonTrayTest, HideAnimationAlwaysCompletesOnDelete) {
On 2017/04/27 20:41:35, bruthig wrote:
> IMO the test above replaces this one and you can nuke this one.

Will remove.

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
ash/system/overview/overview_button_tray_unittest.cc:263:
std::unique_ptr<ui::ScopedAnimationDurationScaleMode> rotate_duration(
On 2017/04/27 20:41:34, bruthig wrote:
> Is |rotate_duration| still required?

Will remove this test.

[jonross, 2017-05-01 17:26:58]

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
File ash/system/overview/overview_button_tray_unittest.cc (right):

https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
ash/system/overview/overview_button_tray_unittest.cc:236:
std::unique_ptr<ui::ScopedAnimationDurationScaleMode> rotate_duration(
On 2017/04/28 21:21:30, wutao wrote:
> On 2017/04/27 20:41:35, bruthig wrote:
> > Is |rotate_duration| required?
> 
> Found out OnAbort for VisibilityTransition does not progress to the target. So
> we need to change the code here [1] or implementation of the
OverviewButtonTray
> if we need this. Is there a case we will abort animation and but care about
the
> visibility?
> 
> [1]
>
https://cs.chromium.org/chromium/src/ui/compositor/layer_animation_element.cc...
> 

All view's only expect the visible state to change at the end of the animation.

For this animation, if it was cancelled we would want to maintain the previous
visibility so that users can still click on the button.

Setting animation duration to Zero with ScopedAnimationDuration* is a pretty
standard practice if we only want to test pre+post states of a view which
animates.

[wutao1, 2017-05-01 17:42:45]

On 2017/05/01 17:26:58, jonross wrote:
>
https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
> File ash/system/overview/overview_button_tray_unittest.cc (right):
> 
>
https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
> ash/system/overview/overview_button_tray_unittest.cc:236:
> std::unique_ptr<ui::ScopedAnimationDurationScaleMode> rotate_duration(
> On 2017/04/28 21:21:30, wutao wrote:
> > On 2017/04/27 20:41:35, bruthig wrote:
> > > Is |rotate_duration| required?
> > 
> > Found out OnAbort for VisibilityTransition does not progress to the target.
So
> > we need to change the code here [1] or implementation of the
> OverviewButtonTray
> > if we need this. Is there a case we will abort animation and but care about
> the
> > visibility?
> > 
> > [1]
> >
>
https://cs.chromium.org/chromium/src/ui/compositor/layer_animation_element.cc...
> > 
> 
> All view's only expect the visible state to change at the end of the
animation.
> 
> For this animation, if it was cancelled we would want to maintain the previous
> visibility so that users can still click on the button.
> 
> Setting animation duration to Zero with ScopedAnimationDuration* is a pretty
> standard practice if we only want to test pre+post states of a view which
> animates.

Should we still have this test?
1. Abort animation will not make the button to hide.
2. The Zero ScopedAnimationDuration will not do anything to Abort animation. It
makes the test passed because: (when looking at the call stack),
RunAllPendingInMessageLoop() will end MaximizeMode, and it will trigger
OverviewButtonTray::OnMaximizeModeEnded to call UpdateIconVisibility and
SetVisible().
[1]
https://cs.chromium.org/chromium/src/ash/system/overview/overview_button_tray...

[jonross, 2017-05-01 18:02:21]

On 2017/05/01 17:42:45, wutao1 wrote:
> On 2017/05/01 17:26:58, jonross wrote:
> >
>
https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
> > File ash/system/overview/overview_button_tray_unittest.cc (right):
> > 
> >
>
https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
> > ash/system/overview/overview_button_tray_unittest.cc:236:
> > std::unique_ptr<ui::ScopedAnimationDurationScaleMode> rotate_duration(
> > On 2017/04/28 21:21:30, wutao wrote:
> > > On 2017/04/27 20:41:35, bruthig wrote:
> > > > Is |rotate_duration| required?
> > > 
> > > Found out OnAbort for VisibilityTransition does not progress to the
target.
> So
> > > we need to change the code here [1] or implementation of the
> > OverviewButtonTray
> > > if we need this. Is there a case we will abort animation and but care
about
> > the
> > > visibility?
> > > 
> > > [1]
> > >
> >
>
https://cs.chromium.org/chromium/src/ui/compositor/layer_animation_element.cc...
> > > 
> > 
> > All view's only expect the visible state to change at the end of the
> animation.
> > 
> > For this animation, if it was cancelled we would want to maintain the
previous
> > visibility so that users can still click on the button.
> > 
> > Setting animation duration to Zero with ScopedAnimationDuration* is a pretty
> > standard practice if we only want to test pre+post states of a view which
> > animates.
> 
> Should we still have this test?
> 1. Abort animation will not make the button to hide.
> 2. The Zero ScopedAnimationDuration will not do anything to Abort animation.
It
> makes the test passed because: (when looking at the call stack),
> RunAllPendingInMessageLoop() will end MaximizeMode, and it will trigger
> OverviewButtonTray::OnMaximizeModeEnded to call UpdateIconVisibility and
> SetVisible().
> [1]
>
https://cs.chromium.org/chromium/src/ash/system/overview/overview_button_tray...

This is beginning to sound like the new test is not testing the bug which the
original HideAnimationAlwaysCompletesOnDelete covered.

Originally, with the old screen rotation animation, the animation was connected
to layers which would delete. And the View would not be updated appropriated
with a hidden state.

The condition which could cause this is if we stacked the visibility animation
on the overview button with the screen rotation.

On device we would not be receiving an OnMaximizeModeEnded call a second time.
I'm actually surprised that you are receiving one. What's triggering it during
this test?

[wutao, 2017-05-01 19:19:46]

On 2017/05/01 18:02:21, jonross wrote:
> On 2017/05/01 17:42:45, wutao1 wrote:
> > On 2017/05/01 17:26:58, jonross wrote:
> > >
> >
>
https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
> > > File ash/system/overview/overview_button_tray_unittest.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2845123003/diff/1/ash/system/overview/overvie...
> > > ash/system/overview/overview_button_tray_unittest.cc:236:
> > > std::unique_ptr<ui::ScopedAnimationDurationScaleMode> rotate_duration(
> > > On 2017/04/28 21:21:30, wutao wrote:
> > > > On 2017/04/27 20:41:35, bruthig wrote:
> > > > > Is |rotate_duration| required?
> > > > 
> > > > Found out OnAbort for VisibilityTransition does not progress to the
> target.
> > So
> > > > we need to change the code here [1] or implementation of the
> > > OverviewButtonTray
> > > > if we need this. Is there a case we will abort animation and but care
> about
> > > the
> > > > visibility?
> > > > 
> > > > [1]
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/ui/compositor/layer_animation_element.cc...
> > > > 
> > > 
> > > All view's only expect the visible state to change at the end of the
> > animation.
> > > 
> > > For this animation, if it was cancelled we would want to maintain the
> previous
> > > visibility so that users can still click on the button.
> > > 
> > > Setting animation duration to Zero with ScopedAnimationDuration* is a
pretty
> > > standard practice if we only want to test pre+post states of a view which
> > > animates.
> > 
> > Should we still have this test?
> > 1. Abort animation will not make the button to hide.
> > 2. The Zero ScopedAnimationDuration will not do anything to Abort animation.
> It
> > makes the test passed because: (when looking at the call stack),
> > RunAllPendingInMessageLoop() will end MaximizeMode, and it will trigger
> > OverviewButtonTray::OnMaximizeModeEnded to call UpdateIconVisibility and
> > SetVisible().
> > [1]
> >
>
https://cs.chromium.org/chromium/src/ash/system/overview/overview_button_tray...
> 
> This is beginning to sound like the new test is not testing the bug which the
> original HideAnimationAlwaysCompletesOnDelete covered.
> 
> Originally, with the old screen rotation animation, the animation was
connected
> to layers which would delete. And the View would not be updated appropriated
> with a hidden state.
> 
> The condition which could cause this is if we stacked the visibility animation
> on the overview button with the screen rotation.
> 
> On device we would not be receiving an OnMaximizeModeEnded call a second time.
> I'm actually surprised that you are receiving one. What's triggering it during
> this test?

Please see the stack trace in the test:
RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded():

#0 0x7f075e93c9eb base::debug::StackTrace::StackTrace()
#1 0x7f075e93b72c base::debug::StackTrace::StackTrace()
#2 0x7f075f561879 ash::TrayBackgroundView::SetVisible()
#3 0x7f075f4c882a ash::OverviewButtonTray::UpdateIconVisibility()
#4 0x7f075f4c8935 ash::OverviewButtonTray::OnMaximizeModeEnded()
#5 0x7f075f434131 ash::Shell::NotifyMaximizeModeEnded()
#6 0x7f075f5c2368 ash::MaximizeModeController::EnableMaximizeModeWindowManager()
#7 0x7f075f5c2cc8 ash::MaximizeModeController::LeaveMaximizeMode()
#8 0x7f075f5c2c6b ash::MaximizeModeController::LidEventReceived()
#9 0x7f075f5c1e75 ash::MaximizeModeController::OnGetSwitchStates()
#10 0x7f075f5c6003
_ZN4base8internal13FunctorTraitsIMN3ash22MaximizeModeControllerEFvN8chromeos18PowerManagerClient8LidStateENS5_10TabletModeEEvE6InvokeIRKNS_7WeakPtrIS3_EEJS6_S7_EEEvS9_OT_DpOT0_
#11 0x7f075f5c5f24
_ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN3ash22MaximizeModeControllerEFvN8chromeos18PowerManagerClient8LidStateENS7_10TabletModeEERKNS_7WeakPtrIS5_EEJS8_S9_EEEvOT_OT0_DpOT1_
#12 0x7f075f5c5e7c
_ZN4base8internal7InvokerINS0_9BindStateIMN3ash22MaximizeModeControllerEFvN8chromeos18PowerManagerClient8LidStateENS6_10TabletModeEEJNS_7WeakPtrIS4_EEEEEFvS7_S8_EE7RunImplIRKSA_RKSt5tupleIJSC_
EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEEOS7_OS8_
#13 0x7f075f5c5d9e
_ZN4base8internal7InvokerINS0_9BindStateIMN3ash22MaximizeModeControllerEFvN8chromeos18PowerManagerClient8LidStateENS6_10TabletModeEEJNS_7WeakPtrIS4_EEEEEFvS7_S8_EE3RunEPNS0_13BindStateBaseEOS7
_OS8_
#14 0x7f075e233b55
_ZNKR4base8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS2_10TabletModeEELNS_8internal8CopyModeE1ELNS6_10RepeatModeE1EE3RunES3_S4_
#15 0x7f075e233ae7
_ZN4base8internal13FunctorTraitsINS_8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS4_10TabletModeEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEvE6InvokeIRKSA_JRKS5_RKS6_EEEvOT_DpOT0_
#16 0x7f075e233957
_ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKNS_8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS6_10TabletModeEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEJRKS7_RKS8_EEEvOT_DpOT0_
#17 0x7f075e233903
_ZN4base8internal7InvokerINS0_9BindStateINS_8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS5_10TabletModeEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEJS6_S7_EEEFvvEE7RunImplIRKSB_RKSt5tuple
IJS6_S7_EEJLm0ELm1EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#18 0x7f075e23381c
_ZN4base8internal7InvokerINS0_9BindStateINS_8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS5_10TabletModeEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEJS6_S7_EEEFvvEE3RunEPNS0_13BindStateBas
eE
#19 0x7f075e8fbe4e
_ZNO4base8CallbackIFvvELNS_8internal8CopyModeE0ELNS2_10RepeatModeE0EE3RunEv
#20 0x7f075e941ee1 base::debug::TaskAnnotator::RunTask()
#21 0x7f075e9cc3ae base::MessageLoop::RunTask()
#22 0x7f075e9cc614 base::MessageLoop::DeferOrRunPendingTask()
#23 0x7f075e9cc904 base::MessageLoop::DoWork()
#24 0x7f075e9e366c base::MessagePumpLibevent::Run()
#25 0x7f075e9cbf92 base::MessageLoop::RunHandler()
#26 0x7f075ea6a604 base::RunLoop::Run()
#27 0x7f075ea6a819 base::RunLoop::RunUntilIdle()
#28 0x000000ce8bd1 ash::test::AshTestHelper::RunAllPendingInMessageLoop()
#29 0x000000ce5751 ash::test::AshTestBase::RunAllPendingInMessageLoop()
#30 0x000000755c45
ash::OverviewButtonTrayTest_HideAnimationAlwaysCompletesOnAbort_Test::TestBody()
#31 0x000000cbbafe testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#32 0x000000cacee2 testing::internal::HandleExceptionsInMethodIfSupported<>()
#33 0x000000ca1d76 testing::Test::Run()
#34 0x000000ca24ad testing::TestInfo::Run()
#35 0x000000ca2a4f testing::TestCase::Run()
#36 0x000000ca7dbc testing::internal::UnitTestImpl::RunAllTests()

[jonross, 2017-05-01 20:19:54]

On 2017/05/01 19:19:46, wutao wrote:
> Please see the stack trace in the test:
> RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded():
> 
> #0 0x7f075e93c9eb base::debug::StackTrace::StackTrace()
> #1 0x7f075e93b72c base::debug::StackTrace::StackTrace()
> #2 0x7f075f561879 ash::TrayBackgroundView::SetVisible()
> #3 0x7f075f4c882a ash::OverviewButtonTray::UpdateIconVisibility()
> #4 0x7f075f4c8935 ash::OverviewButtonTray::OnMaximizeModeEnded()
> #5 0x7f075f434131 ash::Shell::NotifyMaximizeModeEnded()
> #6 0x7f075f5c2368
ash::MaximizeModeController::EnableMaximizeModeWindowManager()
> #7 0x7f075f5c2cc8 ash::MaximizeModeController::LeaveMaximizeMode()
> #8 0x7f075f5c2c6b ash::MaximizeModeController::LidEventReceived()
> #9 0x7f075f5c1e75 ash::MaximizeModeController::OnGetSwitchStates()
> #10 0x7f075f5c6003
>
_ZN4base8internal13FunctorTraitsIMN3ash22MaximizeModeControllerEFvN8chromeos18PowerManagerClient8LidStateENS5_10TabletModeEEvE6InvokeIRKNS_7WeakPtrIS3_EEJS6_S7_EEEvS9_OT_DpOT0_
> #11 0x7f075f5c5f24
>
_ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN3ash22MaximizeModeControllerEFvN8chromeos18PowerManagerClient8LidStateENS7_10TabletModeEERKNS_7WeakPtrIS5_EEJS8_S9_EEEvOT_OT0_DpOT1_
> #12 0x7f075f5c5e7c
>
_ZN4base8internal7InvokerINS0_9BindStateIMN3ash22MaximizeModeControllerEFvN8chromeos18PowerManagerClient8LidStateENS6_10TabletModeEEJNS_7WeakPtrIS4_EEEEEFvS7_S8_EE7RunImplIRKSA_RKSt5tupleIJSC_
> EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEEOS7_OS8_
> #13 0x7f075f5c5d9e
>
_ZN4base8internal7InvokerINS0_9BindStateIMN3ash22MaximizeModeControllerEFvN8chromeos18PowerManagerClient8LidStateENS6_10TabletModeEEJNS_7WeakPtrIS4_EEEEEFvS7_S8_EE3RunEPNS0_13BindStateBaseEOS7
> _OS8_
> #14 0x7f075e233b55
>
_ZNKR4base8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS2_10TabletModeEELNS_8internal8CopyModeE1ELNS6_10RepeatModeE1EE3RunES3_S4_
> #15 0x7f075e233ae7
>
_ZN4base8internal13FunctorTraitsINS_8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS4_10TabletModeEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEvE6InvokeIRKSA_JRKS5_RKS6_EEEvOT_DpOT0_
> #16 0x7f075e233957
>
_ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKNS_8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS6_10TabletModeEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEJRKS7_RKS8_EEEvOT_DpOT0_
> #17 0x7f075e233903
>
_ZN4base8internal7InvokerINS0_9BindStateINS_8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS5_10TabletModeEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEJS6_S7_EEEFvvEE7RunImplIRKSB_RKSt5tuple
> IJS6_S7_EEJLm0ELm1EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
> #18 0x7f075e23381c
>
_ZN4base8internal7InvokerINS0_9BindStateINS_8CallbackIFvN8chromeos18PowerManagerClient8LidStateENS5_10TabletModeEELNS0_8CopyModeE1ELNS0_10RepeatModeE1EEEJS6_S7_EEEFvvEE3RunEPNS0_13BindStateBas
> eE
> #19 0x7f075e8fbe4e
> _ZNO4base8CallbackIFvvELNS_8internal8CopyModeE0ELNS2_10RepeatModeE0EE3RunEv
> #20 0x7f075e941ee1 base::debug::TaskAnnotator::RunTask()
> #21 0x7f075e9cc3ae base::MessageLoop::RunTask()
> #22 0x7f075e9cc614 base::MessageLoop::DeferOrRunPendingTask()
> #23 0x7f075e9cc904 base::MessageLoop::DoWork()
> #24 0x7f075e9e366c base::MessagePumpLibevent::Run()
> #25 0x7f075e9cbf92 base::MessageLoop::RunHandler()
> #26 0x7f075ea6a604 base::RunLoop::Run()
> #27 0x7f075ea6a819 base::RunLoop::RunUntilIdle()
> #28 0x000000ce8bd1 ash::test::AshTestHelper::RunAllPendingInMessageLoop()
> #29 0x000000ce5751 ash::test::AshTestBase::RunAllPendingInMessageLoop()
> #30 0x000000755c45
>
ash::OverviewButtonTrayTest_HideAnimationAlwaysCompletesOnAbort_Test::TestBody()
> #31 0x000000cbbafe
testing::internal::HandleSehExceptionsInMethodIfSupported<>()
> #32 0x000000cacee2 testing::internal::HandleExceptionsInMethodIfSupported<>()
> #33 0x000000ca1d76 testing::Test::Run()
> #34 0x000000ca24ad testing::TestInfo::Run()
> #35 0x000000ca2a4f testing::TestCase::Run()
> #36 0x000000ca7dbc testing::internal::UnitTestImpl::RunAllTests()

So you've found a bug in ash_unittests. It looks like a startup task from
MaximizeModeController is waiting on the queue. For now, can you add a
RunAllPendingInMessageLoop at the start of this test?

I can look into fixing the root cause

[wutao, 2017-05-01 21:12:53]

On 2017/05/01 20:19:54, jonross wrote:
> On 2017/05/01 19:19:46, wutao wrote:
> > Please see the stack trace in the test:
> > RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded():

> So you've found a bug in ash_unittests. It looks like a startup task from
> MaximizeModeController is waiting on the queue. For now, can you add a
> RunAllPendingInMessageLoop at the start of this test?
> 
> I can look into fixing the root cause

Thank you.
It is not clear to me why only in this new test
*HideAnimationAlwaysCompletesOnAbort* has this bug? I do not see any specialty
here.
By adding the RunAllPendingInMessageLoop at the start of this test resolve the
problem: RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded().
But the test will not pass because AbortAllAnimations will not continue to make
the visibility to false in this case.

The other test *HideAnimationAlwaysCompletesOnDelete* in this cl does pass by
removing the ZERO_DURATION.
But this has another issue: when the old layer was deleted, however in the test,
we are testing the new overview button layer(), which is not set to visible yet?
So we are not testing Hide Animation Always Completes On Delete, instead testing
the OverviewButtonTray visibility is false when initialized? Only when it
receives ash::MaximizeModeController::EnableMaximizeModeWindowManager, it will
be set to visible true. But there is no such an event in the test after
RunAllPendingInMessageLoop().

[jonross, 2017-05-01 21:26:40]

On 2017/05/01 21:12:53, wutao wrote:
> On 2017/05/01 20:19:54, jonross wrote:
> > On 2017/05/01 19:19:46, wutao wrote:
> > > Please see the stack trace in the test:
> > > RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded():
> 
> > So you've found a bug in ash_unittests. It looks like a startup task from
> > MaximizeModeController is waiting on the queue. For now, can you add a
> > RunAllPendingInMessageLoop at the start of this test?
> > 
> > I can look into fixing the root cause
> 
> Thank you.
> It is not clear to me why only in this new test
> *HideAnimationAlwaysCompletesOnAbort* has this bug? I do not see any specialty
> here.
> By adding the RunAllPendingInMessageLoop at the start of this test resolve the
> problem: RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded().
> But the test will not pass because AbortAllAnimations will not continue to
make
> the visibility to false in this case.
> 
> The other test *HideAnimationAlwaysCompletesOnDelete* in this cl does pass by
> removing the ZERO_DURATION.
> But this has another issue: when the old layer was deleted, however in the
test,
> we are testing the new overview button layer(), which is not set to visible
yet?
> So we are not testing Hide Animation Always Completes On Delete, instead
testing
> the OverviewButtonTray visibility is false when initialized? Only when it
> receives ash::MaximizeModeController::EnableMaximizeModeWindowManager, it will
> be set to visible true. But there is no such an event in the test after
> RunAllPendingInMessageLoop().

A lot of other ash_unittests testing maximize mode code only ever explicitly
toggle the state. So they don't end up advancing the message loop. Meanwhile
here it was needed to advance to wrap up animation processing.

For the new test is the failure indicative of the bug being back under the new
animation style? Or is the test incorrect? With the new animation style, if a
convertible device triggers a simultaneous screen rotation and exiting of
maximize mode, does the button hide?

For the older test, we actually only care about the new layer. The fact that the
new layer is created not visible is the desired state. As we are testing the
animation of exiting maximize mode.

[wutao, 2017-05-01 21:40:01]

On 2017/05/01 21:26:40, jonross wrote:
> On 2017/05/01 21:12:53, wutao wrote:
> > On 2017/05/01 20:19:54, jonross wrote:
> > > On 2017/05/01 19:19:46, wutao wrote:
> > > > Please see the stack trace in the test:
> > > > RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded():
> > 
> > > So you've found a bug in ash_unittests. It looks like a startup task from
> > > MaximizeModeController is waiting on the queue. For now, can you add a
> > > RunAllPendingInMessageLoop at the start of this test?
> > > 
> > > I can look into fixing the root cause
> > 
> > Thank you.
> > It is not clear to me why only in this new test
> > *HideAnimationAlwaysCompletesOnAbort* has this bug? I do not see any
specialty
> > here.
> > By adding the RunAllPendingInMessageLoop at the start of this test resolve
the
> > problem: RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded().
> > But the test will not pass because AbortAllAnimations will not continue to
> make
> > the visibility to false in this case.
> > 
> > The other test *HideAnimationAlwaysCompletesOnDelete* in this cl does pass
by
> > removing the ZERO_DURATION.
> > But this has another issue: when the old layer was deleted, however in the
> test,
> > we are testing the new overview button layer(), which is not set to visible
> yet?
> > So we are not testing Hide Animation Always Completes On Delete, instead
> testing
> > the OverviewButtonTray visibility is false when initialized? Only when it
> > receives ash::MaximizeModeController::EnableMaximizeModeWindowManager, it
will
> > be set to visible true. But there is no such an event in the test after
> > RunAllPendingInMessageLoop().
> 
> A lot of other ash_unittests testing maximize mode code only ever explicitly
> toggle the state. So they don't end up advancing the message loop. Meanwhile
> here it was needed to advance to wrap up animation processing.
> 
> For the new test is the failure indicative of the bug being back under the new
> animation style? Or is the test incorrect? With the new animation style, if a
> convertible device triggers a simultaneous screen rotation and exiting of
> maximize mode, does the button hide?
> 
> For the older test, we actually only care about the new layer. The fact that
the
> new layer is created not visible is the desired state. As we are testing the
> animation of exiting maximize mode.

All the tests in this cl are testing the old screen rotation animation style.
For the new screen rotation animation style, there is no deletion of layers so
the hide animation  of the overview button should continue to visible false.

The failed test indicates the new test itself
(HideAnimationAlwaysCompletesOnAbort) is incorrect, because the overview button
is visible true when aborting the hide animation. We can change this test to
HideAnimationAlwaysStopsOnAbort and expect true when abort the hide animation?

[jonross, 2017-05-01 21:55:17]

On 2017/05/01 21:40:01, wutao wrote:
> On 2017/05/01 21:26:40, jonross wrote:
> > On 2017/05/01 21:12:53, wutao wrote:
> > > On 2017/05/01 20:19:54, jonross wrote:
> > > > On 2017/05/01 19:19:46, wutao wrote:
> > > > > Please see the stack trace in the test:
> > > > > RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded():
> > > 
> > > > So you've found a bug in ash_unittests. It looks like a startup task
from
> > > > MaximizeModeController is waiting on the queue. For now, can you add a
> > > > RunAllPendingInMessageLoop at the start of this test?
> > > > 
> > > > I can look into fixing the root cause
> > > 
> > > Thank you.
> > > It is not clear to me why only in this new test
> > > *HideAnimationAlwaysCompletesOnAbort* has this bug? I do not see any
> specialty
> > > here.
> > > By adding the RunAllPendingInMessageLoop at the start of this test resolve
> the
> > > problem: RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded().
> > > But the test will not pass because AbortAllAnimations will not continue to
> > make
> > > the visibility to false in this case.
> > > 
> > > The other test *HideAnimationAlwaysCompletesOnDelete* in this cl does pass
> by
> > > removing the ZERO_DURATION.
> > > But this has another issue: when the old layer was deleted, however in the
> > test,
> > > we are testing the new overview button layer(), which is not set to
visible
> > yet?
> > > So we are not testing Hide Animation Always Completes On Delete, instead
> > testing
> > > the OverviewButtonTray visibility is false when initialized? Only when it
> > > receives ash::MaximizeModeController::EnableMaximizeModeWindowManager, it
> will
> > > be set to visible true. But there is no such an event in the test after
> > > RunAllPendingInMessageLoop().
> > 
> > A lot of other ash_unittests testing maximize mode code only ever explicitly
> > toggle the state. So they don't end up advancing the message loop. Meanwhile
> > here it was needed to advance to wrap up animation processing.
> > 
> > For the new test is the failure indicative of the bug being back under the
new
> > animation style? Or is the test incorrect? With the new animation style, if
a
> > convertible device triggers a simultaneous screen rotation and exiting of
> > maximize mode, does the button hide?
> > 
> > For the older test, we actually only care about the new layer. The fact that
> the
> > new layer is created not visible is the desired state. As we are testing the
> > animation of exiting maximize mode.
> 
> All the tests in this cl are testing the old screen rotation animation style.
> For the new screen rotation animation style, there is no deletion of layers so
> the hide animation  of the overview button should continue to visible false.
> 
> The failed test indicates the new test itself
> (HideAnimationAlwaysCompletesOnAbort) is incorrect, because the overview
button
> is visible true when aborting the hide animation. We can change this test to
> HideAnimationAlwaysStopsOnAbort and expect true when abort the hide animation?

Ah, well if the new rotation animation style does not cancel the animations
anymore, then this test is completely redundant.

We could shift the test to trigger the hide, along with the new form of the
screen rotation animation. Allowing each to complete. Then verify that the
button is hidden. But we do not need a test to verify cancelling, as that is not
an expected action anymore.

[wutao, 2017-05-01 22:05:56]

On 2017/05/01 21:55:17, jonross wrote:
> On 2017/05/01 21:40:01, wutao wrote:
> > On 2017/05/01 21:26:40, jonross wrote:
> > > On 2017/05/01 21:12:53, wutao wrote:
> > > > On 2017/05/01 20:19:54, jonross wrote:
> > > > > On 2017/05/01 19:19:46, wutao wrote:
> > > > > > Please see the stack trace in the test:
> > > > > > RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded():
> > > > 
> > > > > So you've found a bug in ash_unittests. It looks like a startup task
> from
> > > > > MaximizeModeController is waiting on the queue. For now, can you add a
> > > > > RunAllPendingInMessageLoop at the start of this test?
> > > > > 
> > > > > I can look into fixing the root cause
> > > > 
> > > > Thank you.
> > > > It is not clear to me why only in this new test
> > > > *HideAnimationAlwaysCompletesOnAbort* has this bug? I do not see any
> > specialty
> > > > here.
> > > > By adding the RunAllPendingInMessageLoop at the start of this test
resolve
> > the
> > > > problem: RunAllPendingInMessageLoop() triggering OnMaximizeModeEnded().
> > > > But the test will not pass because AbortAllAnimations will not continue
to
> > > make
> > > > the visibility to false in this case.
> > > > 
> > > > The other test *HideAnimationAlwaysCompletesOnDelete* in this cl does
pass
> > by
> > > > removing the ZERO_DURATION.
> > > > But this has another issue: when the old layer was deleted, however in
the
> > > test,
> > > > we are testing the new overview button layer(), which is not set to
> visible
> > > yet?
> > > > So we are not testing Hide Animation Always Completes On Delete, instead
> > > testing
> > > > the OverviewButtonTray visibility is false when initialized? Only when
it
> > > > receives ash::MaximizeModeController::EnableMaximizeModeWindowManager,
it
> > will
> > > > be set to visible true. But there is no such an event in the test after
> > > > RunAllPendingInMessageLoop().
> > > 
> > > A lot of other ash_unittests testing maximize mode code only ever
explicitly
> > > toggle the state. So they don't end up advancing the message loop.
Meanwhile
> > > here it was needed to advance to wrap up animation processing.
> > > 
> > > For the new test is the failure indicative of the bug being back under the
> new
> > > animation style? Or is the test incorrect? With the new animation style,
if
> a
> > > convertible device triggers a simultaneous screen rotation and exiting of
> > > maximize mode, does the button hide?
> > > 
> > > For the older test, we actually only care about the new layer. The fact
that
> > the
> > > new layer is created not visible is the desired state. As we are testing
the
> > > animation of exiting maximize mode.
> > 
> > All the tests in this cl are testing the old screen rotation animation
style.
> > For the new screen rotation animation style, there is no deletion of layers
so
> > the hide animation  of the overview button should continue to visible false.
> > 
> > The failed test indicates the new test itself
> > (HideAnimationAlwaysCompletesOnAbort) is incorrect, because the overview
> button
> > is visible true when aborting the hide animation. We can change this test to
> > HideAnimationAlwaysStopsOnAbort and expect true when abort the hide
animation?
> 
> Ah, well if the new rotation animation style does not cancel the animations
> anymore, then this test is completely redundant.
> 
> We could shift the test to trigger the hide, along with the new form of the
> screen rotation animation. Allowing each to complete. Then verify that the
> button is hidden. But we do not need a test to verify cancelling, as that is
not
> an expected action anymore.

The old screen rotation animation is behind a switch now (disabled by default).
It could be co-exist with the new screen rotation animation for a while.
We added a switch to ash_test_helper so that all current tests will continue
using the old screen rotation animation, otherwise there are many tests will
fail with the new screen rotation style.

Therefore in this cl, it could make sense to keep the old test *OnDelete*, but
explicitly deleting the layers as in the new test.
I can add another test to test the new screen rotation with the hide animation
as you suggested.
I will delete the OnAbort test, since it is not expected behavior.

[jonross, 2017-05-02 13:45:25]

On 2017/05/01 22:05:56, wutao wrote:
> Therefore in this cl, it could make sense to keep the old test *OnDelete*, but
> explicitly deleting the layers as in the new test.
> I can add another test to test the new screen rotation with the hide animation
> as you suggested.
> I will delete the OnAbort test, since it is not expected behavior.

SGTM

[wutao, 2017-05-03 22:28:00]

Description was changed from

==========
Add more tests to test OverviewButtonTray.

Add more tests to test the hide animation of OverviewButtonTray.

BUG=NONE
TEST=OverviewButtonTrayTest.HideAnimationAlwaysCompletes*
==========

to

==========
Add more tests to test OverviewButtonTray and ScreenRotationAnimator.

Add more tests to test the hide animation of OverviewButtonTray and interaction
with ScreenRotationAnimator.

BUG=718198
TEST=OverviewButtonTrayTest.HideAnimationAlwaysCompletes* and
ScreenRotationAnimator[Smooth|Slow]AnimationTest.OverviewButtonTrayHideAnimation*
==========

[wutao, 2017-05-03 22:28:23]

wutao@chromium.org changed reviewers:
+ oshima@chromium.org

[wutao, 2017-05-03 22:28:24]

Hi Jon, please have a look of the new tests.

Thanks,
Tao

[wutao, 2017-05-03 22:29:01]

The CQ bit was checked by wutao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 22:29:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jonross, 2017-05-03 23:37:06]

On 2017/05/03 22:28:24, wutao wrote:
> Hi Jon, please have a look of the new tests.
> 
> Thanks,
> Tao

The format of the screen_rotation_animation_unittest.cc seems good to me. I'm
not sure if the owner oshima@ would object to having the tests depend on
OverviewButtonTray.

It is conveniently already on screen and affected by the rotation animation. But
is an extra dependency for the test. However making a custom view, and injecting
it into the screen hierarchy seems like a lot of work.

If oshima@ has no objections to this, then LGTM

[jonross, 2017-05-03 23:37:08]

On 2017/05/03 22:28:24, wutao wrote:
> Hi Jon, please have a look of the new tests.
> 
> Thanks,
> Tao

The format of the screen_rotation_animation_unittest.cc seems good to me. I'm
not sure if the owner oshima@ would object to having the tests depend on
OverviewButtonTray.

It is conveniently already on screen and affected by the rotation animation. But
is an extra dependency for the test. However making a custom view, and injecting
it into the screen hierarchy seems like a lot of work.

If oshima@ has no objections to this, then LGTM

[commit-bot: I haz the power, 2017-05-03 23:47:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-03 23:47:53]

Dry run: This issue passed the CQ dry run.

[wutao, 2017-05-04 00:02:02]

On 2017/05/03 23:37:08, jonross wrote:
> On 2017/05/03 22:28:24, wutao wrote:
> > Hi Jon, please have a look of the new tests.
> > 
> > Thanks,
> > Tao
> 
> The format of the screen_rotation_animation_unittest.cc seems good to me. I'm
> not sure if the owner oshima@ would object to having the tests depend on
> OverviewButtonTray.
> 
> It is conveniently already on screen and affected by the rotation animation.
But
> is an extra dependency for the test. However making a custom view, and
injecting
> it into the screen hierarchy seems like a lot of work.
> 
> If oshima@ has no objections to this, then LGTM

This is both ways. If we use screen rotation animator in overview button tray
test, that is adding extra dependency for the OverviewButtonTray tests.
But we are testing the interactions anyway.

[oshima, 2017-05-06 21:02:20]

https://codereview.chromium.org/2845123003/diff/20001/ash/rotator/screen_rota...
File ash/rotator/screen_rotation_animator_unittest.cc (right):

https://codereview.chromium.org/2845123003/diff/20001/ash/rotator/screen_rota...
ash/rotator/screen_rotation_animator_unittest.cc:419:
std::unique_ptr<ui::ScopedAnimationDurationScaleMode> hide_duration(
can't you just create on a stack?

same for the rest.

[wutao, 2017-05-31 17:29:55]

Hi Oshima,

ptal.

Thank you,
Tao

https://codereview.chromium.org/2845123003/diff/20001/ash/rotator/screen_rota...
File ash/rotator/screen_rotation_animator_unittest.cc (right):

https://codereview.chromium.org/2845123003/diff/20001/ash/rotator/screen_rota...
ash/rotator/screen_rotation_animator_unittest.cc:419:
std::unique_ptr<ui::ScopedAnimationDurationScaleMode> hide_duration(
On 2017/05/06 21:02:20, oshima wrote:
> can't you just create on a stack?
> 
> same for the rest.

Done.

[wutao, 2017-05-31 17:30:04]

The CQ bit was checked by wutao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-31 17:30:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 17:37:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-31 17:37:18]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_headless_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[wutao, 2017-05-31 17:49:26]

The CQ bit was checked by wutao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-31 17:49:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 19:53:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-31 19:53:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[wutao, 2017-05-31 22:04:16]

The CQ bit was checked by wutao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-31 22:05:34]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oshima, 2017-05-31 22:23:02]

lgtm

[commit-bot: I haz the power, 2017-05-31 22:59:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-31 22:59:08]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[wutao, 2017-05-31 23:01:05]

The CQ bit was checked by wutao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-31 23:02:08]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 00:27:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-01 00:27:17]

Dry run: This issue passed the CQ dry run.

[wutao, 2017-06-01 00:31:24]

The CQ bit was checked by wutao@chromium.org

[wutao, 2017-06-01 00:31:24]

The patchset sent to the CQ was uploaded after l-g-t-m from jonross@chromium.org
Link to the patchset: https://codereview.chromium.org/2845123003/#ps60001
(title: "Fix nits in patch 2.")

[commit-bot: I haz the power, 2017-06-01 00:32:19]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 00:37:08]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496277084576010,
"parent_rev": "5d546d806f9753d54593be07a4aa014a2ab875d3", "commit_rev":
"310d861a8399fae1b082cd0d83db69b98d1df7da"}

[commit-bot: I haz the power, 2017-06-01 00:37:20]

Description was changed from

==========
Add more tests to test OverviewButtonTray and ScreenRotationAnimator.

Add more tests to test the hide animation of OverviewButtonTray and interaction
with ScreenRotationAnimator.

BUG=718198
TEST=OverviewButtonTrayTest.HideAnimationAlwaysCompletes* and
ScreenRotationAnimator[Smooth|Slow]AnimationTest.OverviewButtonTrayHideAnimation*
==========

to

==========
Add more tests to test OverviewButtonTray and ScreenRotationAnimator.

Add more tests to test the hide animation of OverviewButtonTray and interaction
with ScreenRotationAnimator.

BUG=718198
TEST=OverviewButtonTrayTest.HideAnimationAlwaysCompletes* and
ScreenRotationAnimator[Smooth|Slow]AnimationTest.OverviewButtonTrayHideAnimation*

Review-Url: https://codereview.chromium.org/2845123003
Cr-Commit-Position: refs/heads/master@{#476111}
Committed:
https://chromium.googlesource.com/chromium/src/+/310d861a8399fae1b082cd0d83db...
==========

[commit-bot: I haz the power, 2017-06-01 00:37:22]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/310d861a8399fae1b082cd0d83db...