Wires up WorkspaceLayoutManager in mash

Wires up WorkspaceLayoutManager in mash

Along with enabling a portion of the tests.

BUG=603369
TEST=none
R=jamescook@chromium.org

Committed: https://crrev.com/383d818f80838a60c4e9655836f1b243f83ea6ce
Cr-Commit-Position: refs/heads/master@{#394887}

[sky, 2016-05-18 21:10:21]

Description was changed from

==========
Wires up WorkspaceLayoutManager in mash

Along with enabling a portion of the tests.

BUG=603369
TEST=none
R=jamescook@chromium.org
==========

to

==========
Wires up WorkspaceLayoutManager in mash

Along with enabling a portion of the tests.

BUG=603369
TEST=none
R=jamescook@chromium.org
==========

[sky, 2016-05-18 21:10:21]

sky@chromium.org changed reviewers:
+ tsepez@chromium.org

[sky, 2016-05-18 21:10:44]

tsepez: window_manager.mojom
jamescook: the rest

[Tom Sepez, 2016-05-18 21:43:16]

mojom lgtm

[James Cook, 2016-05-19 05:08:22]

sky, I'm not quite done going through the tests, but I wanted to send you
initial comments in case you got to work before me. I'll finish in the morning.

https://codereview.chromium.org/1994763002/diff/20001/mash/test/mash_test_sui...
File mash/test/mash_test_suite.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/test/mash_test_sui...
mash/test/mash_test_suite.cc:31:
path.Append(FILE_PATH_LITERAL("mash_wm_resources.pak"));
Should there be a direct dep on //mash/wm:resources for this? Or a data dep? It
looks like it might be getting it transitively via mash/wm/unittests -> wm ->
resources, but I'm not sure.

https://codereview.chromium.org/1994763002/diff/20001/mash/test/mash_test_sui...
File mash/test/mash_test_suite.h (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/test/mash_test_sui...
mash/test/mash_test_suite.h:10: #include "base/compiler_specific.h"
nit: base/macros.h

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/bridge/wm_windo...
File mash/wm/bridge/wm_window_mus.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/bridge/wm_windo...
mash/wm/bridge/wm_window_mus.cc:591: } else if (name ==
mus::mojom::WindowManager::kAlwaysOnTop_Property) {
nit: else not needed

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/bridge/wm_windo...
mash/wm/bridge/wm_window_mus.cc:594: this,
ash::wm::WmWindowProperty::ALWAYS_ON_TOP, 0u));
Should this be 0 instead of 0u and/or an old value?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/root_window_con...
File mash/wm/root_window_controller.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/root_window_con...
mash/wm/root_window_controller.cc:66: ~WorkspaceLayoutManagerDelegateImpl()
override = default;
Just for my edification: Is there an advantage to "= default" for virtual
destructors over {} ?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/root_window_con...
mash/wm/root_window_controller.cc:304:
layout_managers_.erase(user_private_windows);
This section reads a little oddly... maybe a comment about ownership (I presume
this is because all the other layout managers need to be owned here, but
WorkspaceLayoutManager is owned below)?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
File mash/wm/test/wm_test_base.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:69: parts[1], "+", base::TRIM_WHITESPACE,
base::SPLIT_WANT_ALL);
parts[0]?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:70: DCHECK_EQ(2u, origin_parts.size());
maybe CHECK_EQ, since you're going to access origin_parts[1] below

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:81: DCHECK_EQ(2u, size_parts.size());
ditto

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:114: work_area.set_height(std::max(0,
work_area.height() - 2));
Comment where the -2 comes from, or use a constant?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:124: return roots.empty() ? nullptr :
roots[0]->root();
Does this have to support the case where there are no roots/displays? Naively I
would expect tests to always have at least one.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:181: window->SetBounds(bounds);
This one doesn't need kInitialBounds_Property?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
File mash/wm/test/wm_test_base.h (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.h:72: // coorelates with creation order).
nit: correlates

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
File mash/wm/test/wm_test_helper.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
mash/wm/test/wm_test_helper.cc:1: // Copyright 2015 The Chromium Authors. All
rights reserved.
nit: 2016

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
mash/wm/test/wm_test_helper.cc:39: display_bounds.height() - 20);
optional: maybe a constant for the magic -20?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
File mash/wm/test/wm_test_helper.h (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
mash/wm/test/wm_test_helper.h:13: #include "ui/gfx/native_widget_types.h"
Is this needed?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
mash/wm/test/wm_test_helper.h:14: #include "ui/wm/public/window_types.h"
ditto

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/window_manager_...
File mash/wm/window_manager_application.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/window_manager_...
mash/wm/window_manager_application.cc:69: if (!screen_) {
I'm a little confused here. This if() implies to me that this can be called
multiple times (presumably because there could be multiple root controllers for
multiple displays). But the function above looks like it would also be called
multiple times if there were multiple roots, which would reset globals_
repeatedly. Are they both supposed to tolerate being called multiple times?

Or is this supposed to be "if (connector_)"?

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
File mash/wm/workspace/workspace_layout_manager_unittest.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
mash/wm/workspace/workspace_layout_manager_unittest.cc:199: 
optional: I think it would be a little clearer if you commented out each test in
its own /* */ block. Jumping into the middle of this file it's not readily
apparently that some of these tests are not compiled. If you think you're going
to uncomment them soon then this is fine, but if it's going to be a while I
think marking them at function start/end would be clearer.

[sky, 2016-05-19 15:14:25]

https://codereview.chromium.org/1994763002/diff/20001/mash/test/mash_test_sui...
File mash/test/mash_test_suite.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/test/mash_test_sui...
mash/test/mash_test_suite.cc:31:
path.Append(FILE_PATH_LITERAL("mash_wm_resources.pak"));
On 2016/05/19 05:08:21, James Cook wrote:
> Should there be a direct dep on //mash/wm:resources for this? Or a data dep?
It
> looks like it might be getting it transitively via mash/wm/unittests -> wm ->
> resources, but I'm not sure.

It does get it transitively. But the target containing this file should also
directly contain the dep too. Updated.

https://codereview.chromium.org/1994763002/diff/20001/mash/test/mash_test_sui...
File mash/test/mash_test_suite.h (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/test/mash_test_sui...
mash/test/mash_test_suite.h:10: #include "base/compiler_specific.h"
On 2016/05/19 05:08:21, James Cook wrote:
> nit: base/macros.h

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/bridge/wm_windo...
File mash/wm/bridge/wm_window_mus.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/bridge/wm_windo...
mash/wm/bridge/wm_window_mus.cc:591: } else if (name ==
mus::mojom::WindowManager::kAlwaysOnTop_Property) {
On 2016/05/19 05:08:21, James Cook wrote:
> nit: else not needed

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/bridge/wm_windo...
mash/wm/bridge/wm_window_mus.cc:594: this,
ash::wm::WmWindowProperty::ALWAYS_ON_TOP, 0u));
On 2016/05/19 05:08:21, James Cook wrote:
> Should this be 0 instead of 0u

Not sure on that. I went with 0u as the type is intptr_t. The compilers seem
happy.

> and/or an old value?

It can't be old value as the types mismatch. I will nuke the old value from the
observer separately.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/root_window_con...
File mash/wm/root_window_controller.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/root_window_con...
mash/wm/root_window_controller.cc:66: ~WorkspaceLayoutManagerDelegateImpl()
override = default;
On 2016/05/19 05:08:21, James Cook wrote:
> Just for my edification: Is there an advantage to "= default" for virtual
> destructors over {} ?

Good question. I'm not sure. I copied this code from root_window_controller.cc.
I added the code there too and originally did not have '= default'. Oshima asked
me to add:
https://codereview.chromium.org/1888153002/diff/1/ash/root_window_controller....
. I suspect it doesn't make a difference, but I'm not positive.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/root_window_con...
mash/wm/root_window_controller.cc:304:
layout_managers_.erase(user_private_windows);
On 2016/05/19 05:08:21, James Cook wrote:
> This section reads a little oddly... maybe a comment about ownership (I
presume
> this is because all the other layout managers need to be owned here, but
> WorkspaceLayoutManager is owned below)?

layout_managers_ is a different type of LayoutManager (yes, confusing). I'll add
a comment.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
File mash/wm/test/wm_test_base.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:69: parts[1], "+", base::TRIM_WHITESPACE,
base::SPLIT_WANT_ALL);
On 2016/05/19 05:08:22, James Cook wrote:
> parts[0]?

origin is first. Format supported is on line 60, which is [origin]size.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:70: DCHECK_EQ(2u, origin_parts.size());
On 2016/05/19 05:08:22, James Cook wrote:
> maybe CHECK_EQ, since you're going to access origin_parts[1] below

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:81: DCHECK_EQ(2u, size_parts.size());
On 2016/05/19 05:08:22, James Cook wrote:
> ditto

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:114: work_area.set_height(std::max(0,
work_area.height() - 2));
On 2016/05/19 05:08:22, James Cook wrote:
> Comment where the -2 comes from, or use a constant?

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:124: return roots.empty() ? nullptr :
roots[0]->root();
On 2016/05/19 05:08:22, James Cook wrote:
> Does this have to support the case where there are no roots/displays? Naively
I
> would expect tests to always have at least one.

In theory tests that shutdown all the roots could see no displays. But since
we're not doing that now I'll remove the conditional and make it a DCHECK.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:181: window->SetBounds(bounds);
On 2016/05/19 05:08:22, James Cook wrote:
> This one doesn't need kInitialBounds_Property?

Top level windows are routed through the windowmanager, where the windowmanager
may impose sizing. So, we need kInitialBounds_Property to indicate the
windowmanager should use the supplied size. Child windows don't go through the
wm, so no need for initial bounds.
The test calling this function may not be all that relevant in the mash world, I
just got it running.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
File mash/wm/test/wm_test_base.h (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.h:72: // coorelates with creation order).
On 2016/05/19 05:08:22, James Cook wrote:
> nit: correlates 

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
File mash/wm/test/wm_test_helper.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
mash/wm/test/wm_test_helper.cc:1: // Copyright 2015 The Chromium Authors. All
rights reserved.
On 2016/05/19 05:08:22, James Cook wrote:
> nit: 2016

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
mash/wm/test/wm_test_helper.cc:39: display_bounds.height() - 20);
On 2016/05/19 05:08:22, James Cook wrote:
> optional: maybe a constant for the magic -20?

I added a comment. I don't think it's worth the constant.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
File mash/wm/test/wm_test_helper.h (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
mash/wm/test/wm_test_helper.h:13: #include "ui/gfx/native_widget_types.h"
On 2016/05/19 05:08:22, James Cook wrote:
> Is this needed?

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_he...
mash/wm/test/wm_test_helper.h:14: #include "ui/wm/public/window_types.h"
On 2016/05/19 05:08:22, James Cook wrote:
> ditto

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/window_manager_...
File mash/wm/window_manager_application.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/window_manager_...
mash/wm/window_manager_application.cc:69: if (!screen_) {
On 2016/05/19 05:08:22, James Cook wrote:
> I'm a little confused here. This if() implies to me that this can be called
> multiple times (presumably because there could be multiple root controllers
for
> multiple displays). But the function above looks like it would also be called
> multiple times if there were multiple roots, which would reset globals_
> repeatedly. Are they both supposed to tolerate being called multiple times?
> 
> Or is this supposed to be "if (connector_)"?

Good catch. I made OnRootWindowControllerGotRoot early out if globals_ is
non-null.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
File mash/wm/workspace/workspace_layout_manager_unittest.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
mash/wm/workspace/workspace_layout_manager_unittest.cc:199: 
On 2016/05/19 05:08:22, James Cook wrote:
> optional: I think it would be a little clearer if you commented out each test
in
> its own /* */ block. Jumping into the middle of this file it's not readily
> apparently that some of these tests are not compiled. If you think you're
going
> to uncomment them soon then this is fine, but if it's going to be a while I
> think marking them at function start/end would be clearer.

I will revisit this soon.

[James Cook, 2016-05-19 15:17:07]

Last set of comments. Whew!

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
File mash/wm/test/wm_test_base.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:62: gfx::Rect bounds(0, 0, 1366, 768);
Maybe start with just gfx::Rect since the size is always set? (I was briefly
confused about why this isn't 800x600.)

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
File mash/wm/workspace/workspace_layout_manager_unittest.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
mash/wm/workspace/workspace_layout_manager_unittest.cc:638:
UpdateDisplay("800x600");
I know it was like this before but this seems a little odd, since the display
started at 800x600 it's not really being "enlarged".

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
mash/wm/workspace/workspace_layout_manager_unittest.cc:863: mus::Window* window(
Do you still want to change these ones in the commented out section? Leaving
them in a partially converted state seems odd.

[James Cook, 2016-05-19 15:27:46]

Comments from patch set 3.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/bridge/wm_windo...
File mash/wm/bridge/wm_window_mus.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/bridge/wm_windo...
mash/wm/bridge/wm_window_mus.cc:594: this,
ash::wm::WmWindowProperty::ALWAYS_ON_TOP, 0u));
On 2016/05/19 15:14:24, sky wrote:
> On 2016/05/19 05:08:21, James Cook wrote:
> > Should this be 0 instead of 0u
> 
> Not sure on that. I went with 0u as the type is intptr_t. The compilers seem
> happy.

Huh. intptr_t is signed. But I guess 0 is 0. <shrug>

> > and/or an old value?
> 
> It can't be old value as the types mismatch. I will nuke the old value from
the
> observer separately.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/root_window_con...
File mash/wm/root_window_controller.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/root_window_con...
mash/wm/root_window_controller.cc:66: ~WorkspaceLayoutManagerDelegateImpl()
override = default;
On 2016/05/19 15:14:24, sky wrote:
> On 2016/05/19 05:08:21, James Cook wrote:
> > Just for my edification: Is there an advantage to "= default" for virtual
> > destructors over {} ?
> 
> Good question. I'm not sure. I copied this code from
root_window_controller.cc.
> I added the code there too and originally did not have '= default'. Oshima
asked
> me to add:
>
https://codereview.chromium.org/1888153002/diff/1/ash/root_window_controller....
> . I suspect it doesn't make a difference, but I'm not positive.

Acknowledged.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
File mash/wm/test/wm_test_base.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:69: parts[1], "+", base::TRIM_WHITESPACE,
base::SPLIT_WANT_ALL);
On 2016/05/19 15:14:24, sky wrote:
> On 2016/05/19 05:08:22, James Cook wrote:
> > parts[0]?
> 
> origin is first. Format supported is on line 60, which is [origin]size.

Huh? It looks to me like 1+2-800x600 will split into parts[0]="1+2" and
parts[1]="800x600", so to compute origin you want parts[0]. What am I missing?

[sky, 2016-05-19 16:12:39]

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
File mash/wm/test/wm_test_base.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:62: gfx::Rect bounds(0, 0, 1366, 768);
On 2016/05/19 15:17:07, James Cook wrote:
> Maybe start with just gfx::Rect since the size is always set? (I was briefly
> confused about why this isn't 800x600.)

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/test/wm_test_ba...
mash/wm/test/wm_test_base.cc:69: parts[1], "+", base::TRIM_WHITESPACE,
base::SPLIT_WANT_ALL);
On 2016/05/19 15:27:46, James Cook wrote:
> On 2016/05/19 15:14:24, sky wrote:
> > On 2016/05/19 05:08:22, James Cook wrote:
> > > parts[0]?
> > 
> > origin is first. Format supported is on line 60, which is [origin]size.
> 
> Huh? It looks to me like 1+2-800x600 will split into parts[0]="1+2" and
> parts[1]="800x600", so to compute origin you want parts[0]. What am I missing?

D'OH! You are clearly right. It should be parts[0]. Updated.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
File mash/wm/workspace/workspace_layout_manager_unittest.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
mash/wm/workspace/workspace_layout_manager_unittest.cc:638:
UpdateDisplay("800x600");
On 2016/05/19 15:17:07, James Cook wrote:
> I know it was like this before but this seems a little odd, since the display
> started at 800x600 it's not really being "enlarged".

Done.

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
mash/wm/workspace/workspace_layout_manager_unittest.cc:863: mus::Window* window(
On 2016/05/19 15:17:07, James Cook wrote:
> Do you still want to change these ones in the commented out section? Leaving
> them in a partially converted state seems odd.

I did a global replace of a handful of things. If it bothers you, I can revert
them.

[James Cook, 2016-05-19 16:37:24]

LGTM

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
File mash/wm/workspace/workspace_layout_manager_unittest.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
mash/wm/workspace/workspace_layout_manager_unittest.cc:863: mus::Window* window(
On 2016/05/19 16:12:38, sky wrote:
> On 2016/05/19 15:17:07, James Cook wrote:
> > Do you still want to change these ones in the commented out section? Leaving
> > them in a partially converted state seems odd.
> 
> I did a global replace of a handful of things. If it bothers you, I can revert
> them.

Yes, please revert them.

[sky, 2016-05-19 16:45:35]

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
File mash/wm/workspace/workspace_layout_manager_unittest.cc (right):

https://codereview.chromium.org/1994763002/diff/20001/mash/wm/workspace/works...
mash/wm/workspace/workspace_layout_manager_unittest.cc:863: mus::Window* window(
On 2016/05/19 16:37:24, James Cook wrote:
> On 2016/05/19 16:12:38, sky wrote:
> > On 2016/05/19 15:17:07, James Cook wrote:
> > > Do you still want to change these ones in the commented out section?
Leaving
> > > them in a partially converted state seems odd.
> > 
> > I did a global replace of a handful of things. If it bothers you, I can
revert
> > them.
> 
> Yes, please revert them.

Done.

[sky, 2016-05-19 16:46:47]

The CQ bit was checked by sky@chromium.org

[sky, 2016-05-19 16:46:48]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/1994763002/#ps80001
(title: "revert parts of test")

[commit-bot: I haz the power, 2016-05-19 16:47:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1994763002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1994763002/80001

[commit-bot: I haz the power, 2016-05-19 17:53:06]

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

[commit-bot: I haz the power, 2016-05-19 17:53:08]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[sky, 2016-05-19 18:06:26]

The CQ bit was checked by sky@chromium.org

[sky, 2016-05-19 18:06:27]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/1994763002/#ps100001
(title: "data deps")

[commit-bot: I haz the power, 2016-05-19 18:06:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1994763002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1994763002/100001

[commit-bot: I haz the power, 2016-05-19 20:37:35]

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

[commit-bot: I haz the power, 2016-05-19 20:37:36]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[sky, 2016-05-19 20:58:57]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-05-19 21:00:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1994763002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1994763002/100001

[commit-bot: I haz the power, 2016-05-19 22:34:18]

Description was changed from

==========
Wires up WorkspaceLayoutManager in mash

Along with enabling a portion of the tests.

BUG=603369
TEST=none
R=jamescook@chromium.org
==========

to

==========
Wires up WorkspaceLayoutManager in mash

Along with enabling a portion of the tests.

BUG=603369
TEST=none
R=jamescook@chromium.org
==========

[commit-bot: I haz the power, 2016-05-19 22:34:19]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-19 22:36:01]

Description was changed from

==========
Wires up WorkspaceLayoutManager in mash

Along with enabling a portion of the tests.

BUG=603369
TEST=none
R=jamescook@chromium.org
==========

to

==========
Wires up WorkspaceLayoutManager in mash

Along with enabling a portion of the tests.

BUG=603369
TEST=none
R=jamescook@chromium.org

Committed: https://crrev.com/383d818f80838a60c4e9655836f1b243f83ea6ce
Cr-Commit-Position: refs/heads/master@{#394887}
==========

[commit-bot: I haz the power, 2016-05-19 22:36:03]

Patchset 6 (id:??) landed as
https://crrev.com/383d818f80838a60c4e9655836f1b243f83ea6ce
Cr-Commit-Position: refs/heads/master@{#394887}