collect windows from mru_list in workspace_controller.

collect windows from mru_list in workspace_controller.

BUG=b:38505617
TEST=
  ash_unittests on ShelfLayoutManagerTest.AutohideShelfForAutohideWhenActiveWindow
  ash unittests on AppListPresenterDelegateTest.ShelfBackgroundRespondsToAppListBeingShown

Review-Url: https://codereview.chromium.org/2967543003
Cr-Commit-Position: refs/heads/master@{#486462}
Committed: https://chromium.googlesource.com/chromium/src/+/754ce9484c33d957a31a4a6aa822c92abc62694a

[Muyuan, 2017-06-29 22:16:58]

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

[oshima, 2017-06-29 22:57:08]

can you add unit tests?

[Muyuan, 2017-06-29 23:40:45]

Description was changed from

==========
collect windows from mru_list in workspace_controller.

BUG=b:38505617
==========

to

==========
collect windows from mru_list in workspace_controller.

BUG=b:38505617
TEST=ash_unittests 
  --gtest_filter=ShelfLayoutManagerTest.AutohideShelfForAutohideWhenActiveWindow
==========

[Muyuan, 2017-06-29 23:44:58]

On 2017/06/29 22:57:08, oshima (OOO back on July 5th) wrote:
> can you add unit tests?

done

[oshima, 2017-06-30 00:00:08]

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

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

Dry run: CQ is trying da patch.

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

[oshima, 2017-06-30 00:04:09]

please add following checks, then lgtm

https://codereview.chromium.org/2967543003/diff/20001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager_unittest.cc (right):

https://codereview.chromium.org/2967543003/diff/20001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager_unittest.cc:1565:
window_two->SetProperty(aura::client::kAlwaysOnTopKey, true);
please test if the window is in the always on container.

https://codereview.chromium.org/2967543003/diff/20001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager_unittest.cc:1567: EXPECT_EQ(SHELF_AUTO_HIDE,
shelf->GetVisibilityState());
Can you also check if the workspace state is in maximized?

[Muyuan, 2017-06-30 00:28:23]

https://codereview.chromium.org/2967543003/diff/20001/ash/shelf/shelf_layout_...
File ash/shelf/shelf_layout_manager_unittest.cc (right):

https://codereview.chromium.org/2967543003/diff/20001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager_unittest.cc:1565:
window_two->SetProperty(aura::client::kAlwaysOnTopKey, true);
On 2017/06/30 00:04:08, oshima (OOO back on July 5th) wrote:
> please test if the window is in the always on container.

Done.

https://codereview.chromium.org/2967543003/diff/20001/ash/shelf/shelf_layout_...
ash/shelf/shelf_layout_manager_unittest.cc:1567: EXPECT_EQ(SHELF_AUTO_HIDE,
shelf->GetVisibilityState());
On 2017/06/30 00:04:08, oshima (OOO back on July 5th) wrote:
> Can you also check if the workspace state is in maximized?

Done.

[Muyuan, 2017-06-30 00:28:30]

The CQ bit was checked by muyuanli@chromium.org

[Muyuan, 2017-06-30 00:28:31]

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/2967543003/#ps40001
(title: "add more verification to test")

[commit-bot: I haz the power, 2017-06-30 00:28:42]

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-30 01:40:00]

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

[commit-bot: I haz the power, 2017-06-30 01:40:01]

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

[Muyuan, 2017-07-05 23:59:23]

Patchset #4 (id:60001) has been deleted

[Muyuan, 2017-07-13 00:13:57]

Description was changed from

==========
collect windows from mru_list in workspace_controller.

BUG=b:38505617
TEST=ash_unittests 
  --gtest_filter=ShelfLayoutManagerTest.AutohideShelfForAutohideWhenActiveWindow
==========

to

==========
collect windows from mru_list in workspace_controller.

BUG=b:38505617
TEST=
  ash_unittests on
ShelfLayoutManagerTest.AutohideShelfForAutohideWhenActiveWindow
  ash unittests on
AppListPresenterDelegateTest.ShelfBackgroundRespondsToAppListBeingShown
==========

[Muyuan, 2017-07-13 00:14:02]

The CQ bit was checked by muyuanli@chromium.org

[Muyuan, 2017-07-13 00:14:02]

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/2967543003/#ps80001
(title: "address applist tests")

[commit-bot: I haz the power, 2017-07-13 00:14:24]

CQ is trying da patch.

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

[oshima, 2017-07-13 00:21:54]

https://codereview.chromium.org/2967543003/diff/80001/ash/app_list/app_list_p...
File ash/app_list/app_list_presenter_delegate.cc (right):

https://codereview.chromium.org/2967543003/diff/80001/ash/app_list/app_list_p...
ash/app_list/app_list_presenter_delegate.cc:125:
->set_ignored_by_shelf(!IsSideShelf(root_window));
we should always set this to true, to keep the same behavior even for current
app list.

[commit-bot: I haz the power, 2017-07-13 01:02:39]

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

[commit-bot: I haz the power, 2017-07-13 01:02:40]

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

[Muyuan, 2017-07-13 01:04:40]

The CQ bit was checked by muyuanli@chromium.org

[commit-bot: I haz the power, 2017-07-13 01:04:57]

CQ is trying da patch.

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

[Muyuan, 2017-07-13 01:12:57]

The CQ bit was unchecked by muyuanli@chromium.org

[Muyuan, 2017-07-13 01:41:41]

https://codereview.chromium.org/2967543003/diff/80001/ash/app_list/app_list_p...
File ash/app_list/app_list_presenter_delegate.cc (right):

https://codereview.chromium.org/2967543003/diff/80001/ash/app_list/app_list_p...
ash/app_list/app_list_presenter_delegate.cc:125:
->set_ignored_by_shelf(!IsSideShelf(root_window));
On 2017/07/13 00:21:54, oshima wrote:
> we should always set this to true, to keep the same behavior even for current
> app list.

Done.

[Muyuan, 2017-07-13 01:41:48]

The CQ bit was checked by muyuanli@chromium.org

[Muyuan, 2017-07-13 01:41:48]

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/2967543003/#ps100001
(title: "address review comment")

[commit-bot: I haz the power, 2017-07-13 01:42:08]

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-13 02:51:56]

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

[commit-bot: I haz the power, 2017-07-13 02:51:58]

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

[oshima, 2017-07-13 04:41:32]

https://codereview.chromium.org/2967543003/diff/100001/ash/app_list/app_list_...
File ash/app_list/app_list_presenter_delegate_unittest.cc (left):

https://codereview.chromium.org/2967543003/diff/100001/ash/app_list/app_list_...
ash/app_list/app_list_presenter_delegate_unittest.cc:703:
SHELF_BACKGROUND_DEFAULT);
This should stay right?

[Muyuan, 2017-07-13 18:46:01]

https://codereview.chromium.org/2967543003/diff/100001/ash/app_list/app_list_...
File ash/app_list/app_list_presenter_delegate_unittest.cc (left):

https://codereview.chromium.org/2967543003/diff/100001/ash/app_list/app_list_...
ash/app_list/app_list_presenter_delegate_unittest.cc:703:
SHELF_BACKGROUND_DEFAULT);
On 2017/07/13 04:41:32, oshima wrote:
> This should stay right?

It should be OVERLAP per UX requirement. The test itself is not setup properly..
I put down a todo and created a bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=742461 to track the issue
separately.

[Muyuan, 2017-07-13 18:46:06]

The CQ bit was checked by muyuanli@chromium.org

[Muyuan, 2017-07-13 18:46:07]

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/2967543003/#ps120001
(title: "write down todo for test")

[commit-bot: I haz the power, 2017-07-13 18:46:23]

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-13 19:56:44]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1499971566684970,
"parent_rev": "d4f5c197b9d01fd820038bd4a85945c55a1c8d4f", "commit_rev":
"754ce9484c33d957a31a4a6aa822c92abc62694a"}

[commit-bot: I haz the power, 2017-07-13 19:56:59]

Description was changed from

==========
collect windows from mru_list in workspace_controller.

BUG=b:38505617
TEST=
  ash_unittests on
ShelfLayoutManagerTest.AutohideShelfForAutohideWhenActiveWindow
  ash unittests on
AppListPresenterDelegateTest.ShelfBackgroundRespondsToAppListBeingShown
==========

to

==========
collect windows from mru_list in workspace_controller.

BUG=b:38505617
TEST=
  ash_unittests on
ShelfLayoutManagerTest.AutohideShelfForAutohideWhenActiveWindow
  ash unittests on
AppListPresenterDelegateTest.ShelfBackgroundRespondsToAppListBeingShown

Review-Url: https://codereview.chromium.org/2967543003
Cr-Commit-Position: refs/heads/master@{#486462}
Committed:
https://chromium.googlesource.com/chromium/src/+/754ce9484c33d957a31a4a6aa822...
==========

[commit-bot: I haz the power, 2017-07-13 19:57:02]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/754ce9484c33d957a31a4a6aa822...