Exit overview mode when a new activity is opened in athena

Exit overview mode when a new activity is opened in athena.

This has the side effect of fixing the crash in 396368

BUG=396368
TEST=Manual, see bug

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285393

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285763

[pkotwicz, 2014-07-23 03:48:31]

Rob, PTAL

[flackr, 2014-07-23 17:33:39]

lgtm, sadrul wdyt? I don't believe we'd have windows added during overview
except for new activity creation, it's almost certainly the case today.

[sadrul-g, 2014-07-23 17:42:41]

What is the stacktrace from the crash?

[pkotwicz, 2014-07-23 17:58:14]

We crash in WindowOverviewModeImpl::DoScroll(), when accessing the new
activity's kWindowOverviewState in particular. (The returned
WindowOverviewState* is NULL)

Overview mode currently does not handle adding new windows while in overview
mode. In particular, it does not transform the newly added activity. So it looks
like an activity was selected. However, the user is still in overview mode.

[sadrul, 2014-07-23 18:30:51]

On 2014/07/23 17:58:14, pkotwicz wrote:
> We crash in WindowOverviewModeImpl::DoScroll(), when accessing the new
> activity's kWindowOverviewState in particular. (The returned
> WindowOverviewState* is NULL)
> 
> Overview mode currently does not handle adding new windows while in overview
> mode. In particular, it does not transform the newly added activity. So it
looks
> like an activity was selected. However, the user is still in overview mode.

Good catch. LGTM. (nit: maybe do SetInOverview(false) in ::Layout() instead, so
we also get out of the overview mode in case one of the windows is closed in
some way?)

For a better fix, we would have OnWindowAddedToLayout/OnWindowRemovedFromLayout
plumb through the information to the WindowOverviewMode instance, and it will
update the windows accordingly. But we should have this fix for now.

[pkotwicz, 2014-07-24 13:58:03]

mukai@ for OWNERS

[Jun Mukai, 2014-07-24 17:03:26]

lgtm

https://codereview.chromium.org/411813002/diff/60001/athena/home/home_card_un...
File athena/home/home_card_unittest.cc (left):

https://codereview.chromium.org/411813002/diff/60001/athena/home/home_card_un...
athena/home/home_card_unittest.cc:90:
WindowManager::GetInstance()->ToggleOverview();
also remove this

[pkotwicz, 2014-07-24 20:21:17]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2014-07-24 20:21:36]

The CQ bit was unchecked by pkotwicz@chromium.org

[pkotwicz, 2014-07-24 20:21:54]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-07-24 20:22:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/411813002/80001

[commit-bot: I haz the power, 2014-07-24 22:23:48]

Change committed as 285393

[pkotwicz, 2014-07-25 02:58:10]

This CL does not fix the crash when the test is exited when in overview. :(
mukai@ can you please take another look?

[pkotwicz, 2014-07-25 04:35:26]

Ok, I have fixed the test crashing when the test is exited when in overview.

The destructor of WindowOverviewModeImpl::scoped_targeter_ was segfaulting
because the ScopedWindowTargeter destructor was trying to access a deleted
window.

[pkotwicz, 2014-07-25 04:49:45]

I have noticed that ScopedWindowTargeter has unused code... I will fix that in a
separate CL. WindowOverviewModeImpl's destructor uses
WindowOverviewModeImpl::container_ a lot, so this is still the correct fix

[pkotwicz, 2014-07-25 22:56:45]

mukai@ re-approved the CL offline

[pkotwicz, 2014-07-25 22:56:49]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-07-25 22:57:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/411813002/120001

[commit-bot: I haz the power, 2014-07-26 00:11:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win8_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win8_chromium_rel/bui...)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-26 18:04:36]

Change committed as 285763