mash: Introduce ShowWebDialogWithContainer for webui dialogs

mash: Introduce ShowWebDialogWithContainer for webui dialogs

Several webui dialogs spawned by the ash system tray can be shown either as
children of the general settings webui window or by themselves at the login
screen. In the latter case we need to place them in the right ash window
container. Under mash, chrome is not aware of the ash window hierarchy, so
it needs to use a container ID rather than a native window parent.

Wire up the "set time" dialog as an example.

BUG=657021
TEST=manual, spawn set time dialog from settings and from lock screen
TBR=stevenjb@chromium.org for function rename touching c/b/ui/webui/chromeos

Committed: https://crrev.com/e36a90e2b15864b46c1d4ab862fd38590877b8d0
Cr-Commit-Position: refs/heads/master@{#426953}

[James Cook, 2016-10-21 21:21:48]

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

[James Cook, 2016-10-21 21:21:49]

sky, please take a look.

https://codereview.chromium.org/2441133002/diff/1/chrome/browser/ui/ash/syste...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.cc (right):

https://codereview.chromium.org/2441133002/diff/1/chrome/browser/ui/ash/syste...
chrome/browser/ui/ash/system_tray_delegate_chromeos.cc:354:
SystemTrayClient::GetDialogParentContainerId());
This is the core reason for this change. "GetNativeWindow" is an aura window
container. SystemTrayClient does not provide GetNativeWindow (intentionally so).
I'll move it over when I mojo-ify the ash->chrome communication for SetTime.

https://codereview.chromium.org/2441133002/diff/1/chrome/browser/ui/ash/web_d...
File chrome/browser/ui/ash/web_dialog_util.cc (right):

https://codereview.chromium.org/2441133002/diff/1/chrome/browser/ui/ash/web_d...
chrome/browser/ui/ash/web_dialog_util.cc:53:
extensions::ChromeExtensionWebContentsObserver::CreateForWebContents(
This line of code could arguably be refactored into an InitWebDialogView()
method that is shared with chrome::ShowDialogView(), or I could introduce some
kind of delegate, but it didn't seem worth it. We create these explicitly in
many places that create web contents.

[sky, 2016-10-21 23:11:15]

LGTM

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/set_time_dialog.cc (right):

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/set_time_dialog.cc:34: DCHECK(container_id !=
ash::kShellWindowId_Invalid);
DCHECK_NE

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/set_time_dialog.h (right):

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/set_time_dialog.h:27: static void
ShowDialogInContainer(int container_id);
Document what container_id is/means? I suspect someone not familiar with ash
names won't know what container_id means.

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/ui/ash/w...
File chrome/browser/ui/ash/web_dialog_util.cc (right):

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/ui/ash/w...
chrome/browser/ui/ash/web_dialog_util.cc:39:
mojo::ConvertTo<std::vector<uint8_t>>(container_id);
uint8_t -> int32_t.

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/ui/ash/w...
File chrome/browser/ui/ash/web_dialog_util.h (right):

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/ui/ash/w...
chrome/browser/ui/ash/web_dialog_util.h:23: // a |parent| or valid
|container_id| must be provided. The window is destroyed
Agaun, document what container_id is. A reference to where to get more
information is fine.

[sky, 2016-10-21 23:22:59]

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/ui/ash/w...
File chrome/browser/ui/ash/web_dialog_util.cc (right):

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/ui/ash/w...
chrome/browser/ui/ash/web_dialog_util.cc:39:
mojo::ConvertTo<std::vector<uint8_t>>(container_id);
On 2016/10/21 23:11:15, sky wrote:
> uint8_t -> int32_t.

As you pointed out, I have this wrong. What you have is right.

[James Cook, 2016-10-21 23:28:13]

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/set_time_dialog.cc (right):

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/set_time_dialog.cc:34: DCHECK(container_id !=
ash::kShellWindowId_Invalid);
On 2016/10/21 23:11:15, sky wrote:
> DCHECK_NE

Done.

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/set_time_dialog.h (right):

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/set_time_dialog.h:27: static void
ShowDialogInContainer(int container_id);
On 2016/10/21 23:11:15, sky wrote:
> Document what container_id is/means? I suspect someone not familiar with ash
> names won't know what container_id means.

Done.

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/ui/ash/w...
File chrome/browser/ui/ash/web_dialog_util.h (right):

https://codereview.chromium.org/2441133002/diff/20001/chrome/browser/ui/ash/w...
chrome/browser/ui/ash/web_dialog_util.h:23: // a |parent| or valid
|container_id| must be provided. The window is destroyed
On 2016/10/21 23:11:15, sky wrote:
> Agaun, document what container_id is. A reference to where to get more
> information is fine.

Done.

[James Cook, 2016-10-21 23:29:46]

Description was changed from

==========
mash: Introduce ShowWebDialogWithContainer for webui dialogs

Several webui dialogs spawned by the ash system tray can be shown either as
children of the general settings webui window or by themselves at the login
screen. In the latter case we need to place them in the right ash window
container. Under mash, chrome is not aware of the ash window hierarchy, so
it needs to use a container ID rather than a native window parent.

Wire up the "set time" dialog as an example.

BUG=657021
TEST=manual, spawn set time dialog from settings and from lock screen
==========

to

==========
mash: Introduce ShowWebDialogWithContainer for webui dialogs

Several webui dialogs spawned by the ash system tray can be shown either as
children of the general settings webui window or by themselves at the login
screen. In the latter case we need to place them in the right ash window
container. Under mash, chrome is not aware of the ash window hierarchy, so
it needs to use a container ID rather than a native window parent.

Wire up the "set time" dialog as an example.

BUG=657021
TEST=manual, spawn set time dialog from settings and from lock screen
TBR=stevenjb@chromium.org for function rename touching c/b/ui/webui/chromeos
==========

[James Cook, 2016-10-21 23:30:09]

jamescook@chromium.org changed reviewers:
+ stevenjb@chromium.org

[James Cook, 2016-10-21 23:30:10]

TBR=stevenjb@chromium.org for function rename touching 1 line in
c/b/ui/webui/chromeos

[James Cook, 2016-10-21 23:30:14]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-10-21 23:30:14]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2441133002/#ps40001
(title: "review comments")

[commit-bot: I haz the power, 2016-10-21 23:31:04]

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, 2016-10-22 02:22:32]

Description was changed from

==========
mash: Introduce ShowWebDialogWithContainer for webui dialogs

Several webui dialogs spawned by the ash system tray can be shown either as
children of the general settings webui window or by themselves at the login
screen. In the latter case we need to place them in the right ash window
container. Under mash, chrome is not aware of the ash window hierarchy, so
it needs to use a container ID rather than a native window parent.

Wire up the "set time" dialog as an example.

BUG=657021
TEST=manual, spawn set time dialog from settings and from lock screen
TBR=stevenjb@chromium.org for function rename touching c/b/ui/webui/chromeos
==========

to

==========
mash: Introduce ShowWebDialogWithContainer for webui dialogs

Several webui dialogs spawned by the ash system tray can be shown either as
children of the general settings webui window or by themselves at the login
screen. In the latter case we need to place them in the right ash window
container. Under mash, chrome is not aware of the ash window hierarchy, so
it needs to use a container ID rather than a native window parent.

Wire up the "set time" dialog as an example.

BUG=657021
TEST=manual, spawn set time dialog from settings and from lock screen
TBR=stevenjb@chromium.org for function rename touching c/b/ui/webui/chromeos
==========

[commit-bot: I haz the power, 2016-10-22 02:22:33]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-22 02:26:15]

Description was changed from

==========
mash: Introduce ShowWebDialogWithContainer for webui dialogs

Several webui dialogs spawned by the ash system tray can be shown either as
children of the general settings webui window or by themselves at the login
screen. In the latter case we need to place them in the right ash window
container. Under mash, chrome is not aware of the ash window hierarchy, so
it needs to use a container ID rather than a native window parent.

Wire up the "set time" dialog as an example.

BUG=657021
TEST=manual, spawn set time dialog from settings and from lock screen
TBR=stevenjb@chromium.org for function rename touching c/b/ui/webui/chromeos
==========

to

==========
mash: Introduce ShowWebDialogWithContainer for webui dialogs

Several webui dialogs spawned by the ash system tray can be shown either as
children of the general settings webui window or by themselves at the login
screen. In the latter case we need to place them in the right ash window
container. Under mash, chrome is not aware of the ash window hierarchy, so
it needs to use a container ID rather than a native window parent.

Wire up the "set time" dialog as an example.

BUG=657021
TEST=manual, spawn set time dialog from settings and from lock screen
TBR=stevenjb@chromium.org for function rename touching c/b/ui/webui/chromeos

Committed: https://crrev.com/e36a90e2b15864b46c1d4ab862fd38590877b8d0
Cr-Commit-Position: refs/heads/master@{#426953}
==========

[commit-bot: I haz the power, 2016-10-22 02:26:16]

Patchset 3 (id:??) landed as
https://crrev.com/e36a90e2b15864b46c1d4ab862fd38590877b8d0
Cr-Commit-Position: refs/heads/master@{#426953}