Do not create workspace for fullscreen.

Do not create workspace for fullscreen.

This CL still keeps some of multi-workspace code, but they are
a bit tough to test.

BUG=259568
R=jamescook@chromium.org
TEST=ash_unittests covers

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

[James Cook, 2013-07-23 16:53:48]

Nice patch.  A few nits and a couple questions.

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
File ash/wm/workspace/workspace_manager.cc (right):

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager.cc:122: active_workspace_ = new
Workspace(this,contents_window_);
nit: space after comma

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager.cc:566:
MoveWorkspaceToPendingOrDelete(workspace, child,
nit: one parameter per line

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
File ash/wm/workspace/workspace_manager.h (left):

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager.h:253: Workspace* unminimizing_workspace_;
It's nice that we don't need this "unminimizing workspace" any more.

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
File ash/wm/workspace/workspace_manager_unittest.cc (left):

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:772: TEST_F(WorkspaceManagerTest,
PersistAcrossAllWorkspaces) {
Is the idea of "persist across all workspaces" removed, or is it just hard to
test now?

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:848: TEST_F(WorkspaceManagerTest,
ShowMinimizedPersistWindow) {
Should we keep this test?

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
File ash/wm/workspace/workspace_manager_unittest.cc (right):

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:364: TEST_F(WorkspaceManagerTest,
MultiWorkspaces) {
Thanks for adding a test for this.

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:594: // Switch to w1. |w1| should
be visible and |w2| hidden.
comment is wrong

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:808: // Transition it to tracked
by worskpace. It should end up in its own
comment is wrong

[Jun Mukai, 2013-07-23 18:04:19]

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
File ash/wm/workspace/workspace_manager.cc (right):

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager.cc:122: active_workspace_ = new
Workspace(this,contents_window_);
On 2013/07/23 16:53:48, James Cook (Chromium) wrote:
> nit: space after comma

Done.

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager.cc:566:
MoveWorkspaceToPendingOrDelete(workspace, child,
On 2013/07/23 16:53:48, James Cook (Chromium) wrote:
> nit: one parameter per line

Done.

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
File ash/wm/workspace/workspace_manager_unittest.cc (left):

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:772: TEST_F(WorkspaceManagerTest,
PersistAcrossAllWorkspaces) {
On 2013/07/23 16:53:48, James Cook (Chromium) wrote:
> Is the idea of "persist across all workspaces" removed, or is it just hard to
> test now?

The idea is removed although a bit of code remains. I think the concept of
persist-across-all-workspaces is not necessary anymore. That's used to share the
same v2 app window (or task manager or some other dialogs) among workspaces.

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:848: TEST_F(WorkspaceManagerTest,
ShowMinimizedPersistWindow) {
On 2013/07/23 16:53:48, James Cook (Chromium) wrote:
> Should we keep this test?

since I've removed PERSIST_ACROSS_ALL_WORKSPACES, this test is not necessary
anymore.

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
File ash/wm/workspace/workspace_manager_unittest.cc (right):

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:594: // Switch to w1. |w1| should
be visible and |w2| hidden.
On 2013/07/23 16:53:48, James Cook (Chromium) wrote:
> comment is wrong

Done.

https://codereview.chromium.org/19460014/diff/1/ash/wm/workspace/workspace_ma...
ash/wm/workspace/workspace_manager_unittest.cc:808: // Transition it to tracked
by worskpace. It should end up in its own
On 2013/07/23 16:53:48, James Cook (Chromium) wrote:
> comment is wrong

Done.

[James Cook, 2013-07-23 18:05:23]

LGTM

[commit-bot: I haz the power, 2013-07-23 18:25:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/19460014/5001

[commit-bot: I haz the power, 2013-07-23 23:15:58]

Change committed as 213259