Remove persisted docked windows

Remove persisted docked windows

Disables restoring windows to a docked state if the #ash-enable-docked-windows
flag is disabled. This change was meant to be in M57 right after CL:
https://codereview.chromium.org/2617733002 had landed, but it didn't make it
before the branch point.

BUG=668355
Review-Url: https://codereview.chromium.org/2652043004
Cr-Commit-Position: refs/heads/master@{#446754}
Committed: https://chromium.googlesource.com/chromium/src/+/25a6b95c34d5318dbb2cf46545a6f0bf07d2a6ef

[afakhry, 2017-01-25 18:27:35]

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

[commit-bot: I haz the power, 2017-01-25 18:28:04]

Dry run: 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-01-25 18:54:28]

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

[commit-bot: I haz the power, 2017-01-25 18:54:29]

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

[afakhry, 2017-01-26 20:17:40]

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

[commit-bot: I haz the power, 2017-01-26 20:18:12]

Dry run: 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-01-26 21:13:58]

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

[commit-bot: I haz the power, 2017-01-26 21:13:59]

Dry run: This issue passed the CQ dry run.

[afakhry, 2017-01-26 21:39:55]

Description was changed from

==========
[WIP] Remove persisted docked windows

BUG=668355
==========

to

==========
Remove persisted docked windows

Disables restoring windows to a docked state if the #ash-enable-docked-windows
flag is disabled.

BUG=668355
==========

[afakhry, 2017-01-26 21:40:49]

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

[commit-bot: I haz the power, 2017-01-26 21:41:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[afakhry, 2017-01-26 21:43:39]

afakhry@chromium.org changed reviewers:
+ varkha@chromium.org

[afakhry, 2017-01-26 21:43:39]

varkha, can you please review this CL? Thanks!

[varkha, 2017-01-26 22:34:53]

LGTM with a few small comments.

You may also want to add "_DEPRECATED" to PERSISTED_SHOW_STATE_DOCKED here:

https://cs.chromium.org/chromium/src/components/sessions/core/session_service...

Also if you are auditing all uses of SHOW_STATE_DOCKED then there are probably
more places to add a TODO comment.

https://codereview.chromium.org/2652043004/diff/20001/ui/views/widget/native_...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.chromium.org/2652043004/diff/20001/ui/views/widget/native_...
ui/views/widget/native_widget_aura.cc:539: // TODO(afakhry): Remove in M58.
I would move this comment to the line above to make sure we remove just the
DOCKED state and not this if block contents.

https://codereview.chromium.org/2652043004/diff/20001/ui/views/widget/native_...
ui/views/widget/native_widget_aura.cc:997: // TODO(afakhry): Remove in M58.
Again, would move this comment above the method signature.

https://codereview.chromium.org/2652043004/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
(right):

https://codereview.chromium.org/2652043004/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc:237:
aura::client::kShowStateKey) == ui::SHOW_STATE_DOCKED ||
nit: We should not really be getting here anymore if the property is never
restored, should we? Not that this hurts though.

[commit-bot: I haz the power, 2017-01-26 22:45:24]

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

[commit-bot: I haz the power, 2017-01-26 22:45:24]

Dry run: This issue passed the CQ dry run.

[afakhry, 2017-01-27 02:27:52]

Description was changed from

==========
Remove persisted docked windows

Disables restoring windows to a docked state if the #ash-enable-docked-windows
flag is disabled.

BUG=668355
==========

to

==========
Remove persisted docked windows

Disables restoring windows to a docked state if the #ash-enable-docked-windows
flag is disabled. This change was meant to be in M57 right after CL:
https://codereview.chromium.org/2617733002 had landed, but it didn't make it
before the branch point.

BUG=668355
==========

[afakhry, 2017-01-27 02:29:03]

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

[commit-bot: I haz the power, 2017-01-27 02:29:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[afakhry, 2017-01-27 02:36:58]

Patchset #4 (id:60001) has been deleted

[afakhry, 2017-01-27 02:40:45]

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

[commit-bot: I haz the power, 2017-01-27 02:41:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[afakhry, 2017-01-27 02:43:43]

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

[afakhry, 2017-01-27 02:43:44]

Thanks, varkha. Done for the _DEPRECATED comment.

+sky@, Can you please take a look? Thanks.

https://codereview.chromium.org/2652043004/diff/20001/ui/views/widget/native_...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.chromium.org/2652043004/diff/20001/ui/views/widget/native_...
ui/views/widget/native_widget_aura.cc:539: // TODO(afakhry): Remove in M58.
On 2017/01/26 22:34:53, varkha wrote:
> I would move this comment to the line above to make sure we remove just the
> DOCKED state and not this if block contents.

Done.

https://codereview.chromium.org/2652043004/diff/20001/ui/views/widget/native_...
ui/views/widget/native_widget_aura.cc:997: // TODO(afakhry): Remove in M58.
On 2017/01/26 22:34:53, varkha wrote:
> Again, would move this comment above the method signature.

Done.

https://codereview.chromium.org/2652043004/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
(right):

https://codereview.chromium.org/2652043004/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc:237:
aura::client::kShowStateKey) == ui::SHOW_STATE_DOCKED ||
On 2017/01/26 22:34:53, varkha wrote:
> nit: We should not really be getting here anymore if the property is never
> restored, should we? Not that this hurts though.

We should be able to get here if the flag is disabled, correct?

[varkha, 2017-01-27 03:24:43]

https://codereview.chromium.org/2652043004/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
(right):

https://codereview.chromium.org/2652043004/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc:237:
aura::client::kShowStateKey) == ui::SHOW_STATE_DOCKED ||
On 2017/01/27 02:43:44, afakhry wrote:
> On 2017/01/26 22:34:53, varkha wrote:
> > nit: We should not really be getting here anymore if the property is never
> > restored, should we? Not that this hurts though.
> 
> We should be able to get here if the flag is disabled, correct?

I thought if DockedWindowsEnabled returns true then code before and after this
CL lets you through. If it returns false then the property would never be set to
DOCKED since you don't restore it and don't have runtime paths that set it
(after your previous CL) so it shouldn't matter.

[commit-bot: I haz the power, 2017-01-27 04:18:59]

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

[commit-bot: I haz the power, 2017-01-27 04:19:00]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[sky, 2017-01-27 18:52:53]

LGTM

[afakhry, 2017-01-27 20:01:07]

The CQ bit was checked by afakhry@chromium.org

[afakhry, 2017-01-27 20:01:08]

The patchset sent to the CQ was uploaded after l-g-t-m from varkha@chromium.org
Link to the patchset: https://codereview.chromium.org/2652043004/#ps80001
(title: "varkha's comments")

[commit-bot: I haz the power, 2017-01-27 20:02:43]

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-01-27 20:36:58]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485547267633280,
"parent_rev": "d43eecb7ed3d688f344cc64bc4d09b7fb7e88d7a", "commit_rev":
"25a6b95c34d5318dbb2cf46545a6f0bf07d2a6ef"}

[commit-bot: I haz the power, 2017-01-27 20:37:30]

Description was changed from

==========
Remove persisted docked windows

Disables restoring windows to a docked state if the #ash-enable-docked-windows
flag is disabled. This change was meant to be in M57 right after CL:
https://codereview.chromium.org/2617733002 had landed, but it didn't make it
before the branch point.

BUG=668355
==========

to

==========
Remove persisted docked windows

Disables restoring windows to a docked state if the #ash-enable-docked-windows
flag is disabled. This change was meant to be in M57 right after CL:
https://codereview.chromium.org/2617733002 had landed, but it didn't make it
before the branch point.

BUG=668355

Review-Url: https://codereview.chromium.org/2652043004
Cr-Commit-Position: refs/heads/master@{#446754}
Committed:
https://chromium.googlesource.com/chromium/src/+/25a6b95c34d5318dbb2cf46545a6...
==========

[commit-bot: I haz the power, 2017-01-27 20:37:39]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/25a6b95c34d5318dbb2cf46545a6...