[Docking] Persists docked state for tab-less browser windows

[Docking] Persists docked state for tab-less browser windows.

Allow docked state to be saved and restored for non-tabstrip app browser windows (e.g. bookmarklets saved with chrome-menu->More Tools->Add to shelf... or web apps running in a window). For example https://www.google.com/calendar/gpcal works quite well when docked.
This CL builds on persistent docked state introduced for V2 apps in https://codereview.chromium.org/1056793006/.

BUG=271582
TEST=
Install Maps web app (https://chrome.google.com/webstore/detail/google-maps/lneaknkopdijkpnocmklfnjbeapigfbh).
Pin it to the launcher shelf and select to run it in a window (not as a tab).
Launch the maps web app in a window and dock it at screen edge (Alt+] twice).
Close it.
Click on a shelf icon for the app, verify that it opens in docked state.
Undock, close, relaunch - should open in normal state.

Repeat for a bookmarklet (https://www.google.com/calendar/gpcal saved as a shelf icon and set to run in a window).

Leave both those windows docked and shut down / restart a session. The docked state should get restored.

Open a web browser window, point it to some URL.
Dock the browser window (Alt+] twice).
Close it and reopen, should open normally (not docked).
Dock it again and restart session, should open normally.

Committed: https://crrev.com/86fc7fe26677116fc53d610ef7ee03c3ec2ca154
Cr-Commit-Position: refs/heads/master@{#338346}

[varkha, 2015-07-08 20:29:14]

Patchset #1 (id:1) has been deleted

[varkha, 2015-07-09 04:19:32]

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

[commit-bot: I haz the power, 2015-07-09 04:19:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1221193009/40001

[varkha, 2015-07-09 04:19:57]

The CQ bit was unchecked by varkha@chromium.org

[varkha, 2015-07-09 04:30:07]

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

[commit-bot: I haz the power, 2015-07-09 04:30:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1221193009/60001

[commit-bot: I haz the power, 2015-07-09 05:27:30]

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

[commit-bot: I haz the power, 2015-07-09 05:27:31]

Dry run: This issue passed the CQ dry run.

[varkha, 2015-07-09 18:49:03]

Patchset #2 (id:40001) has been deleted

[varkha, 2015-07-09 18:49:10]

Patchset #1 (id:20001) has been deleted

[varkha, 2015-07-09 18:49:19]

Patchset #1 (id:60001) has been deleted

[varkha, 2015-07-09 18:49:29]

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

[commit-bot: I haz the power, 2015-07-09 18:52:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1221193009/80001

[commit-bot: I haz the power, 2015-07-09 18:58:08]

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

[commit-bot: I haz the power, 2015-07-09 18:58:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

(exceeded global retry quota)

[varkha, 2015-07-09 19:27:17]

Patchset #2 (id:100001) has been deleted

[varkha, 2015-07-09 19:27:34]

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

[varkha, 2015-07-09 19:28:00]

Patchset #1 (id:80001) has been deleted

[commit-bot: I haz the power, 2015-07-09 19:30:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1221193009/120001

[commit-bot: I haz the power, 2015-07-09 21:12:13]

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

[commit-bot: I haz the power, 2015-07-09 21:12:14]

Dry run: This issue passed the CQ dry run.

[varkha, 2015-07-09 23:32:33]

Patchset #2 (id:140001) has been deleted

[varkha, 2015-07-09 23:41:21]

Patchset #2 (id:160001) has been deleted

[varkha, 2015-07-09 23:41:55]

Patchset #2 (id:180001) has been deleted

[varkha, 2015-07-09 23:45:19]

Patchset #2 (id:200001) has been deleted

[varkha, 2015-07-09 23:45:36]

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

[commit-bot: I haz the power, 2015-07-09 23:48:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1221193009/220001

[varkha, 2015-07-10 00:06:16]

Patchset #1 (id:120001) has been deleted

[varkha, 2015-07-10 00:13:54]

varkha@chromium.org changed reviewers:
+ pkasting@chromium.org, sadrul@chromium.org

[varkha, 2015-07-10 00:13:54]

+pkasting@, Can you please take a look? You can try it with the mobile calendar
bookmarklet (https://www.google.com/calendar/gpcal) or use
https://chrome.google.com/webstore/detail/google-maps/lneaknkopdijkpnocmklfnj...
running in a window if you'd like to try that.

+sadrul@ for ui/views/widget/*.

Thanks!

[commit-bot: I haz the power, 2015-07-10 00:28:38]

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

[commit-bot: I haz the power, 2015-07-10 00:28:39]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2015-07-10 06:09:01]

I'm sorry, I don't know what I'm trying here.  The bug says basically nothing
and is already marked "Verified" to boot, I have no idea what "docking" or
"docked state" is, and the fact that this touches _ash files makes me think this
is CrOS-only?

If you want me to test something you're going to need to give a complete
explain-like-I'm-5 description of what all this is about, what you're doing, how
to test, etc.  Otherwise I would select a tester who already has familiarity
with your work.

[varkha, 2015-07-10 13:00:41]

On 2015/07/10 06:09:01, Peter Kasting wrote:
> I'm sorry, I don't know what I'm trying here.  The bug says basically nothing
> and is already marked "Verified" to boot, I have no idea what "docking" or
> "docked state" is, and the fact that this touches _ash files makes me think
this
> is CrOS-only?
> 
> If you want me to test something you're going to need to give a complete
> explain-like-I'm-5 description of what all this is about, what you're doing,
how
> to test, etc.  Otherwise I would select a tester who already has familiarity
> with your work.

Fair enough. This is indeed only affecting Ash (Chrome OS and Windows Metro). 
Docked windows were added to Ash around M34 time. Here is a design doc
describing
it that is still mostly relevant. This CL just fills in persistency part and
session restore for docked state so when a window with a known app_name is
docked
it will retain docked state across launches and reboots.
I will reopen the bug, thanks it was left under-implemented (only V2 apps were
saved / restored).

I'll add a manual test section in the CL description (there's a unit test for
most
scenarios in this CL).

Also +oshima@ who was shadowing most of this work since forever for Ash-specific
bits.

[varkha, 2015-07-10 13:07:51]

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

[Peter Kasting, 2015-07-10 16:05:34]

It sounds like you're implementing something desirable, so I would assume what
you really need is an actual code review rather than someone to test that your
code does indeed work.

[varkha, 2015-07-10 16:20:44]

On 2015/07/10 16:05:34, Peter Kasting wrote:
> It sounds like you're implementing something desirable, so I would assume what
> you really need is an actual code review rather than someone to test that your
> code does indeed work.

Yes, sorry if I got you confused. I am looking for an actual code review.

[Peter Kasting, 2015-07-10 16:40:56]

LGTM, though I'd still prefer this to be reviewed by someone who knows anything
about docked windows.

https://codereview.webrtc.org/1221193009/diff/220001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc (right):

https://codereview.webrtc.org/1221193009/diff/220001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc:57: LOG(INFO)
<< "test: " << (test_app ? "APP" : "TABBED");
Try to avoid LOG statements, even in tests.

https://codereview.webrtc.org/1221193009/diff/220001/ui/views/widget/native_w...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.webrtc.org/1221193009/diff/220001/ui/views/widget/native_w...
ui/views/widget/native_widget_aura.cc:485: state == ui::SHOW_STATE_DOCKED) {
Nit: No {}

[oshima, 2015-07-10 17:43:56]

lgtm with nits

https://codereview.webrtc.org/1221193009/diff/220001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc (right):

https://codereview.webrtc.org/1221193009/diff/220001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc:46: class
BrowserViewTestP : public BrowserViewTest,
nit: please put this in anonymous namespace to avoid conflict with other tests.

https://codereview.webrtc.org/1221193009/diff/220001/ui/views/widget/native_w...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.webrtc.org/1221193009/diff/220001/ui/views/widget/native_w...
ui/views/widget/native_widget_aura.cc:369: // to return the current window
bounds if the window is not in either state.
can you update the comment?

[sadrul, 2015-07-10 18:00:23]

lgtm

[varkha, 2015-07-10 19:38:25]

Thanks!

https://codereview.chromium.org/1221193009/diff/220001/chrome/browser/ui/view...
File chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc (right):

https://codereview.chromium.org/1221193009/diff/220001/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc:46: class
BrowserViewTestP : public BrowserViewTest,
On 2015/07/10 17:43:56, oshima wrote:
> nit: please put this in anonymous namespace to avoid conflict with other
tests.

Done.

https://codereview.chromium.org/1221193009/diff/220001/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc:57: LOG(INFO)
<< "test: " << (test_app ? "APP" : "TABBED");
On 2015/07/10 16:40:56, Peter Kasting wrote:
> Try to avoid LOG statements, even in tests.

Done. Leftover that presubmit doesn't catch.

https://codereview.chromium.org/1221193009/diff/220001/ui/views/widget/native...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.chromium.org/1221193009/diff/220001/ui/views/widget/native...
ui/views/widget/native_widget_aura.cc:369: // to return the current window
bounds if the window is not in either state.
On 2015/07/10 17:43:56, oshima wrote:
> can you update the comment?

Done.

https://codereview.chromium.org/1221193009/diff/220001/ui/views/widget/native...
ui/views/widget/native_widget_aura.cc:485: state == ui::SHOW_STATE_DOCKED) {
On 2015/07/10 16:40:56, Peter Kasting wrote:
> Nit: No {}

Doesn't multiline condition suggest braces?

[varkha, 2015-07-10 19:38:26]

Thanks!

https://codereview.chromium.org/1221193009/diff/220001/chrome/browser/ui/view...
File chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc (right):

https://codereview.chromium.org/1221193009/diff/220001/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc:46: class
BrowserViewTestP : public BrowserViewTest,
On 2015/07/10 17:43:56, oshima wrote:
> nit: please put this in anonymous namespace to avoid conflict with other
tests.

Done.

https://codereview.chromium.org/1221193009/diff/220001/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc:57: LOG(INFO)
<< "test: " << (test_app ? "APP" : "TABBED");
On 2015/07/10 16:40:56, Peter Kasting wrote:
> Try to avoid LOG statements, even in tests.

Done. Leftover that presubmit doesn't catch.

https://codereview.chromium.org/1221193009/diff/220001/ui/views/widget/native...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.chromium.org/1221193009/diff/220001/ui/views/widget/native...
ui/views/widget/native_widget_aura.cc:369: // to return the current window
bounds if the window is not in either state.
On 2015/07/10 17:43:56, oshima wrote:
> can you update the comment?

Done.

https://codereview.chromium.org/1221193009/diff/220001/ui/views/widget/native...
ui/views/widget/native_widget_aura.cc:485: state == ui::SHOW_STATE_DOCKED) {
On 2015/07/10 16:40:56, Peter Kasting wrote:
> Nit: No {}

Doesn't multiline condition suggest braces?

[varkha, 2015-07-10 19:39:18]

The CQ bit was checked by varkha@chromium.org

[varkha, 2015-07-10 19:39:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org, oshima@chromium.org, sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/1221193009/#ps240001
(title: "[Docking] Persists docked state for tab-less browser windows (nits)")

[commit-bot: I haz the power, 2015-07-10 19:40:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1221193009/240001

[commit-bot: I haz the power, 2015-07-10 20:36:56]

Committed patchset #2 (id:240001)

[commit-bot: I haz the power, 2015-07-10 20:37:57]

Patchset 2 (id:??) landed as
https://crrev.com/86fc7fe26677116fc53d610ef7ee03c3ec2ca154
Cr-Commit-Position: refs/heads/master@{#338346}