CrOS Tablet Window management - Split Screen part I

CrOS Tablet Window management - Split Screen part I

Implement the split view controller.

Changes in this CL:
1. Introduce a command switch (--enable-tablet-splitview) to enable
split view in tablet mode (maximized mode).
2. Add unit tests for the split view controller.

I'll wire the split view controller with overview mode in a following
CL.

BUG=725683
Review-Url: https://codereview.chromium.org/2960843004
Cr-Commit-Position: refs/heads/master@{#484397}
Committed: https://chromium.googlesource.com/chromium/src/+/d491376f80629a946673f618e05d2f1ffa821c2f

[xdai1, 2017-06-27 19:41:38]

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

[commit-bot: I haz the power, 2017-06-27 19:41:51]

Dry run: CQ is trying da patch.

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

[xdai1, 2017-06-27 23:39:19]

xdai@chromium.org changed reviewers:
+ oshima@chromium.org, varkha@chromium.org

[xdai1, 2017-06-27 23:39:19]

+cc sadrul@ FYI

oshima@, varkha@, as suggested by sadrul@, I split the original CL
(https://codereview.chromium.org/2918403006/) into two smaller CLs and added
tests accordingly. This first part only contains SplitViewController and its
unit tests. I'll wire it with the overview mode in a following CL. Note that the
codes are almost the same as in the original CL.

Please help take a look at the CL, thanks!

[xdai1, 2017-06-27 23:39:38]

Patchset #2 (id:20001) has been deleted

[xdai1, 2017-06-27 23:39:43]

Patchset #2 (id:40001) has been deleted

[oshima, 2017-06-28 17:36:36]

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
File ash/wm/splitview/split_view_controller.cc (right):

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:29: return
wm::GetWindowState(window)->CanSnap();
nit: return CanActivate ? CanSnap : false;

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:63: void
SplitViewController::SetLeftWindow(aura::Window* left_window) {
This method looks pretty much same as SetRightWindow except that left/right is
swapped. 
Maybe it's ok, but may want to try somethign like

pass "the other window" and state, and reset the "other window" if the
left_window matches the other window's reference, and so forth.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:65: return;
can this happen? If it shouldn't happen, then DCHECK is better option

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:76: left_window_ = nullptr;
not necessary?

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:117: right_window_ = nullptr;
ditto

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:181:
root_window->GetChildById(kShellWindowId_DefaultContainer));
just return?

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:256: DCHECK(state_ == BOTH_SNAPPED);
DCHECK_EQ

but is this true? What if I enter the overview mode when only one window is
snapped?

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:277: for (auto* window : windows) {
It's probably faster to simply skip !CanSnap in the loop.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
File ash/wm/splitview/split_view_controller.h (right):

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.h:28: enum State { NOSNAP, LEFT_SNAPPED,
RIGHT_SNAPPED, BOTH_SNAPPED };
nit: NO_SNAP

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.h:115: // The x position of the divider
between |left_window_| and |right_window_|.
in the root window coordinates?

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
File ash/wm/splitview/split_view_controller_unittest.cc (right):

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller_unittest.cc:21: void SetUp() override {
test::AshTestBase::SetUp(); }
not necessary?

[xdai1, 2017-06-28 18:48:17]

oshima@, I've addressed your comments. Please take another look, thanks for the
review!

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
File ash/wm/splitview/split_view_controller.cc (right):

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:29: return
wm::GetWindowState(window)->CanSnap();
On 2017/06/28 17:36:36, oshima wrote:
> nit: return CanActivate ? CanSnap : false;

Done.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:63: void
SplitViewController::SetLeftWindow(aura::Window* left_window) {
On 2017/06/28 17:36:36, oshima wrote:
> This method looks pretty much same as SetRightWindow except that left/right is
> swapped. 
> Maybe it's ok, but may want to try somethign like
> 
> pass "the other window" and state, and reset the "other window" if the
> left_window matches the other window's reference, and so forth.

I tried to merge these two functions into one function, but it doesn't seem it
can reuse codes and simplify the function. Instead I needed to introduce more
if-clause which increased the function complexity. As such I prefer to keep the
two functions the way it looks in the old patch.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:65: return;
On 2017/06/28 17:36:36, oshima wrote:
> can this happen? If it shouldn't happen, then DCHECK is better option

Done.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:76: left_window_ = nullptr;
On 2017/06/28 17:36:36, oshima wrote:
> not necessary?

Done.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:117: right_window_ = nullptr;
On 2017/06/28 17:36:36, oshima wrote:
> ditto

Done.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:181:
root_window->GetChildById(kShellWindowId_DefaultContainer));
On 2017/06/28 17:36:36, oshima wrote:
> just return?

Done.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:256: DCHECK(state_ == BOTH_SNAPPED);
On 2017/06/28 17:36:36, oshima wrote:
> DCHECK_EQ
> 
> but is this true? What if I enter the overview mode when only one window is
> snapped?

Yes, theoretically the state must be BOTH_SNAPPED. If there is only one window
snapped, which means MRU windows list only contains one window, and we won't
enter the overview mode in this case. However, your question reminds me that we
can have windows/ARC apps that could not be snapped thus it's possible that we
only have 1 snapped window while MRU windows list contains more than 1 window.
Removed the DCHECK here.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:277: for (auto* window : windows) {
On 2017/06/28 17:36:36, oshima wrote:
> It's probably faster to simply skip !CanSnap in the loop.

Done.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
File ash/wm/splitview/split_view_controller.h (right):

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.h:28: enum State { NOSNAP, LEFT_SNAPPED,
RIGHT_SNAPPED, BOTH_SNAPPED };
On 2017/06/28 17:36:36, oshima wrote:
> nit: NO_SNAP

Done.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.h:115: // The x position of the divider
between |left_window_| and |right_window_|.
On 2017/06/28 17:36:36, oshima wrote:
> in the root window coordinates?

in screen coordinates. Added it here.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
File ash/wm/splitview/split_view_controller_unittest.cc (right):

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller_unittest.cc:21: void SetUp() override {
test::AshTestBase::SetUp(); }
On 2017/06/28 17:36:36, oshima wrote:
> not necessary?

Done.

[oshima, 2017-06-28 19:47:47]

one more q. mru list may contain a window in always on top, which IIRC, can't be
in splitscreen mode?

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
File ash/wm/splitview/split_view_controller.cc (right):

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:63: void
SplitViewController::SetLeftWindow(aura::Window* left_window) {
On 2017/06/28 18:48:16, xdai1 wrote:
> On 2017/06/28 17:36:36, oshima wrote:
> > This method looks pretty much same as SetRightWindow except that left/right
is
> > swapped. 
> > Maybe it's ok, but may want to try somethign like
> > 
> > pass "the other window" and state, and reset the "other window" if the
> > left_window matches the other window's reference, and so forth.
> 
> I tried to merge these two functions into one function, but it doesn't seem it
> can reuse codes and simplify the function. Instead I needed to introduce more
> if-clause which increased the function complexity. As such I prefer to keep
the
> two functions the way it looks in the old patch.

Thanks for trying. If it didn't help, then this is fine as it's cleaner.

https://codereview.chromium.org/2960843004/diff/60001/ash/wm/splitview/split_...
ash/wm/splitview/split_view_controller.cc:256: DCHECK(state_ == BOTH_SNAPPED);
On 2017/06/28 18:48:16, xdai1 wrote:
> On 2017/06/28 17:36:36, oshima wrote:
> > DCHECK_EQ
> > 
> > but is this true? What if I enter the overview mode when only one window is
> > snapped?
> 
> Yes, theoretically the state must be BOTH_SNAPPED. If there is only one window
> snapped, which means MRU windows list only contains one window, and we won't
> enter the overview mode in this case. However, your question reminds me that
we
> can have windows/ARC apps that could not be snapped thus it's possible that we
> only have 1 snapped window while MRU windows list contains more than 1 window.
> Removed the DCHECK here.

Ack.

[xdai1, 2017-06-28 20:34:46]

On 2017/06/28 19:47:47, oshima wrote:
> one more q. mru list may contain a window in always on top, which IIRC, can't
be
> in splitscreen mode?
> 

There are many kinds of windows that can be in MRU list but can't be in
splitscreen mode. Generally speaking, the windows in MRU list must be
activatable while the windows in splitscreen mode must be both activatable and
snapable (decided by WindowState::CanSnap()).

[oshima, 2017-06-28 22:24:24]

On 2017/06/28 20:34:46, xdai1 wrote:
> On 2017/06/28 19:47:47, oshima wrote:
> > one more q. mru list may contain a window in always on top, which IIRC,
can't
> be
> > in splitscreen mode?
> > 
> 
> There are many kinds of windows that can be in MRU list but can't be in
> splitscreen mode. Generally speaking, the windows in MRU list must be
> activatable while the windows in splitscreen mode must be both activatable and
> snapable (decided by WindowState::CanSnap()).

nvm. It was my misunderstanding of the other issue.

lgtm

[xdai1, 2017-06-29 17:00:49]

varkha@, kindly ping? I assume this split smaller CL looks good to you since the
codes are almost the same as in the original CL?

[varkha, 2017-06-30 14:29:35]

lgtm with a suggestion and a few nits.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
File ash/wm/maximize_mode/maximize_mode_window_manager.cc (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
ash/wm/maximize_mode/maximize_mode_window_manager.cc:90: // Maximize All windows
upon exit split view mode.
nits: All -> all, exit -> exiting

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
File ash/wm/maximize_mode/maximize_mode_window_state.cc (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
ash/wm/maximize_mode/maximize_mode_window_state.cc:73: // If split view mode is
not allowed in maximized mode, do not allow snap
nit: 'do not snap the window' or 'do not allow snapping the window'

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
File ash/wm/maximize_mode/maximize_mode_window_state.h (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
ash/wm/maximize_mode/maximize_mode_window_state.h:66: // return |target_state|.
Otherwise depending on the capabilities of the
nit: returns

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
ash/wm/maximize_mode/maximize_mode_window_state.h:67: // window we either return
|WINDOW_STATE_TYPE_MAXIMIZED| or
nit: drop 'we' and return -> returns

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
File ash/wm/splitview/split_view_controller.cc (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
ash/wm/splitview/split_view_controller.cc:101: void
SplitViewController::SetRightWindow(aura::Window* right_window) {
nit: Those methods look quite similar, maybe avoid duplication by having a
helper method or just use something like SetAdjacentWindow(window,
left_or_right).

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
File ash/wm/splitview/split_view_controller.h (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
ash/wm/splitview/split_view_controller.h:70: // aura::WindowObserver overrides:
nit: feel free to ignore here and below - I think the preferred style now is
just '// aura::WindowObserver:'.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
ash/wm/splitview/split_view_controller.h:124: // i.e., all the other windows
will open snapped on the right side - And vice
nit: And -> and

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
File ash/wm/splitview/split_view_controller_unittest.cc (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
ash/wm/splitview/split_view_controller_unittest.cc:105: // screened, end the
split view mode.
nit: end the split view mode. -> the split view mode ends.

Nice tests!

[xdai1, 2017-07-05 20:17:53]

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

[commit-bot: I haz the power, 2017-07-05 20:18:09]

Dry run: CQ is trying da patch.

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

[xdai1, 2017-07-05 20:18:23]

varkha@, I've addressed your comments. Thanks for the review!

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
File ash/wm/maximize_mode/maximize_mode_window_manager.cc (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
ash/wm/maximize_mode/maximize_mode_window_manager.cc:90: // Maximize All windows
upon exit split view mode.
On 2017/06/30 14:29:34, varkha (OOO on July 3) wrote:
> nits: All -> all, exit -> exiting

Done.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
File ash/wm/maximize_mode/maximize_mode_window_state.cc (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
ash/wm/maximize_mode/maximize_mode_window_state.cc:73: // If split view mode is
not allowed in maximized mode, do not allow snap
On 2017/06/30 14:29:34, varkha (OOO on July 3) wrote:
> nit: 'do not snap the window' or 'do not allow snapping the window'

Done.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
File ash/wm/maximize_mode/maximize_mode_window_state.h (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
ash/wm/maximize_mode/maximize_mode_window_state.h:66: // return |target_state|.
Otherwise depending on the capabilities of the
On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> nit: returns

Done.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
ash/wm/maximize_mode/maximize_mode_window_state.h:67: // window we either return
|WINDOW_STATE_TYPE_MAXIMIZED| or
On 2017/06/30 14:29:34, varkha (OOO on July 3) wrote:
> nit: drop 'we' and return -> returns

Done.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
File ash/wm/splitview/split_view_controller.cc (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
ash/wm/splitview/split_view_controller.cc:101: void
SplitViewController::SetRightWindow(aura::Window* right_window) {
On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> nit: Those methods look quite similar, maybe avoid duplication by having a
> helper method or just use something like SetAdjacentWindow(window,
> left_or_right).

I merged these two methods into one.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
File ash/wm/splitview/split_view_controller.h (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
ash/wm/splitview/split_view_controller.h:70: // aura::WindowObserver overrides:
On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> nit: feel free to ignore here and below - I think the preferred style now is
> just '// aura::WindowObserver:'.

Done.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
ash/wm/splitview/split_view_controller.h:124: // i.e., all the other windows
will open snapped on the right side - And vice
On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> nit: And -> and

Done.

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
File ash/wm/splitview/split_view_controller_unittest.cc (right):

https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
ash/wm/splitview/split_view_controller_unittest.cc:105: // screened, end the
split view mode.
On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> nit: end the split view mode. -> the split view mode ends.
> 
> Nice tests!

Done.

[commit-bot: I haz the power, 2017-07-05 20:56:45]

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

[commit-bot: I haz the power, 2017-07-05 20:56:46]

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_...)

[xdai1, 2017-07-05 21:05:37]

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

[commit-bot: I haz the power, 2017-07-05 21:06:08]

Dry run: CQ is trying da patch.

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

[varkha, 2017-07-05 21:06:51]

On 2017/07/05 20:18:23, xdai1 wrote:
> varkha@, I've addressed your comments. Thanks for the review!
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
> File ash/wm/maximize_mode/maximize_mode_window_manager.cc (right):
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
> ash/wm/maximize_mode/maximize_mode_window_manager.cc:90: // Maximize All
windows
> upon exit split view mode.
> On 2017/06/30 14:29:34, varkha (OOO on July 3) wrote:
> > nits: All -> all, exit -> exiting
> 
> Done.
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
> File ash/wm/maximize_mode/maximize_mode_window_state.cc (right):
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
> ash/wm/maximize_mode/maximize_mode_window_state.cc:73: // If split view mode
is
> not allowed in maximized mode, do not allow snap
> On 2017/06/30 14:29:34, varkha (OOO on July 3) wrote:
> > nit: 'do not snap the window' or 'do not allow snapping the window'
> 
> Done.
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
> File ash/wm/maximize_mode/maximize_mode_window_state.h (right):
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
> ash/wm/maximize_mode/maximize_mode_window_state.h:66: // return
|target_state|.
> Otherwise depending on the capabilities of the
> On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> > nit: returns
> 
> Done.
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/maximize_mode/m...
> ash/wm/maximize_mode/maximize_mode_window_state.h:67: // window we either
return
> |WINDOW_STATE_TYPE_MAXIMIZED| or
> On 2017/06/30 14:29:34, varkha (OOO on July 3) wrote:
> > nit: drop 'we' and return -> returns
> 
> Done.
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
> File ash/wm/splitview/split_view_controller.cc (right):
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
> ash/wm/splitview/split_view_controller.cc:101: void
> SplitViewController::SetRightWindow(aura::Window* right_window) {
> On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> > nit: Those methods look quite similar, maybe avoid duplication by having a
> > helper method or just use something like SetAdjacentWindow(window,
> > left_or_right).
> 
> I merged these two methods into one.
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
> File ash/wm/splitview/split_view_controller.h (right):
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
> ash/wm/splitview/split_view_controller.h:70: // aura::WindowObserver
overrides:
> On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> > nit: feel free to ignore here and below - I think the preferred style now is
> > just '// aura::WindowObserver:'.
> 
> Done.
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
> ash/wm/splitview/split_view_controller.h:124: // i.e., all the other windows
> will open snapped on the right side - And vice
> On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> > nit: And -> and
> 
> Done.
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
> File ash/wm/splitview/split_view_controller_unittest.cc (right):
> 
>
https://codereview.chromium.org/2960843004/diff/100001/ash/wm/splitview/split...
> ash/wm/splitview/split_view_controller_unittest.cc:105: // screened, end the
> split view mode.
> On 2017/06/30 14:29:35, varkha (OOO on July 3) wrote:
> > nit: end the split view mode. -> the split view mode ends.
> > 
> > Nice tests!
> 
> Done.

Thanks, still LGTM.

[commit-bot: I haz the power, 2017-07-05 23:07:00]

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

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

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_TIMED_OUT, build has not
started yet; builder either lacks capacity or does not exist (misspelled?))
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[xdai1, 2017-07-05 23:23:31]

The CQ bit was checked by xdai@chromium.org

[xdai1, 2017-07-05 23:23:32]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2960843004/#ps140001
(title: "Fix failed tests.")

[commit-bot: I haz the power, 2017-07-05 23:23:56]

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-07-06 00:17:24]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1499297011671900,
"parent_rev": "0bfd541aad9959f26bc5790d559add1edf3b3de2", "commit_rev":
"8fd6c029fa94e969cda476bf4bc17ab447d107c7"}

[commit-bot: I haz the power, 2017-07-06 00:17:41]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1499297011671900,
"parent_rev": "94f71bbfcc06fa93196c743f90b08484d931e2b2", "commit_rev":
"d491376f80629a946673f618e05d2f1ffa821c2f"}

[commit-bot: I haz the power, 2017-07-06 00:17:55]

Description was changed from

==========
CrOS Tablet Window management - Split Screen part I

Implement the split view controller.

Changes in this CL:
1. Introduce a command switch (--enable-tablet-splitview) to enable
split view in tablet mode (maximized mode).
2. Add unit tests for the split view controller.

I'll wire the split view controller with overview mode in a following
CL.

BUG=725683
==========

to

==========
CrOS Tablet Window management - Split Screen part I

Implement the split view controller.

Changes in this CL:
1. Introduce a command switch (--enable-tablet-splitview) to enable
split view in tablet mode (maximized mode).
2. Add unit tests for the split view controller.

I'll wire the split view controller with overview mode in a following
CL.

BUG=725683

Review-Url: https://codereview.chromium.org/2960843004
Cr-Commit-Position: refs/heads/master@{#484397}
Committed:
https://chromium.googlesource.com/chromium/src/+/d491376f80629a946673f618e05d...
==========

[commit-bot: I haz the power, 2017-07-06 00:17:57]

Committed patchset #6 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/d491376f80629a946673f618e05d...