Fix crash when dragging a half width window to split view from overivew mode

Fix crash when dragging a half width window to split view from overview mode

BUG=405957
TEST=SplitViewControllerTest.SplitModeActivation

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

[pkotwicz, 2014-08-21 15:46:12]

Sadrul, PTAL

https://codereview.chromium.org/488153003/diff/40001/athena/wm/split_view_con...
File athena/wm/split_view_controller.h (right):

https://codereview.chromium.org/488153003/diff/40001/athena/wm/split_view_con...
athena/wm/split_view_controller.h:51: gfx::Rect GetRightTargetBounds();
I made these methods public by design. When http://crbug.com/405964 is fixed,
AthenaContainerLayoutManager::OnWindowAddedToLayout() should use this method
instead of querying the left window's bounds.

[sadrul, 2014-08-21 16:04:04]

LGTM

https://codereview.chromium.org/488153003/diff/40001/athena/wm/split_view_con...
File athena/wm/split_view_controller_unittest.cc (right):

https://codereview.chromium.org/488153003/diff/40001/athena/wm/split_view_con...
athena/wm/split_view_controller_unittest.cc:31: container->AddChild(window);
There's now a AthenaTestBase::CreateTestWindow(). Can you use that instead to
create+parent the window?

https://codereview.chromium.org/488153003/diff/40001/athena/wm/split_view_con...
athena/wm/split_view_controller_unittest.cc:37: SplitViewController
controller(container, list_provider.get());
Instead of creating this new SVC here, perhaps you could expose
WindowManagerImplTestApi (from window_manager_unittest.cc) and use that to get
the SVC of the WMImpl instead (next patch, perhaps).

[pkotwicz, 2014-08-21 18:35:09]

oshima for OWNERS and for exposing of WindowManagerImplTestApi

Exposing WindowManagerImplTestApi turned out to be easy enough so I just did it
as part of this CL

[oshima, 2014-08-21 19:24:03]

https://codereview.chromium.org/488153003/diff/70001/athena/athena.gyp
File athena/athena.gyp (right):

https://codereview.chromium.org/488153003/diff/70001/athena/athena.gyp#newcod...
athena/athena.gyp:165: 'wm/window_manager_impl_test_api.h',
how about putting them in wm/test?

I'll change ones for activities.

https://codereview.chromium.org/488153003/diff/70001/athena/wm/window_manager...
File athena/wm/window_manager_impl_test_api.h (right):

https://codereview.chromium.org/488153003/diff/70001/athena/wm/window_manager...
athena/wm/window_manager_impl_test_api.h:16: 
can you put this in test namespace?

https://codereview.chromium.org/488153003/diff/70001/athena/wm/window_manager...
athena/wm/window_manager_impl_test_api.h:22: WindowManager* wm();
There is a WindowManager::Get() so no need to expose it here.

[pkotwicz, 2014-08-21 20:35:53]

Done. Oshima, can you please take another look?

[oshima, 2014-08-21 21:12:21]

lgtm

thanks!

[pkotwicz, 2014-08-21 21:30:36]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-08-21 21:32:15]

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

[commit-bot: I haz the power, 2014-08-21 22:10:38]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/44913)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/50095)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-21 22:15:01]

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

[commit-bot: I haz the power, 2014-08-21 22:15:02]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/44931)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/50115)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[pkotwicz, 2014-08-22 00:53:24]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-08-22 00:54:38]

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

[commit-bot: I haz the power, 2014-08-22 02:14:08]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[commit-bot: I haz the power, 2014-08-22 03:06:05]

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

[commit-bot: I haz the power, 2014-08-22 03:06:06]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[pkotwicz, 2014-08-22 12:48:49]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-08-22 12:49:00]

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

[commit-bot: I haz the power, 2014-08-22 13:33:14]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[commit-bot: I haz the power, 2014-08-22 14:27:08]

Committed patchset #4 (150001) as 291400