chromeos: Make network enrollment and SIM unlock dialogs work with mash

chromeos: Make network enrollment and SIM unlock dialogs work with mash

Chrome running in mash cannot directly access the ash aura window hierarchy.
Instead it must pass an ash window container id when a new window is being
opened.

* Convert dialogs to use container id when no explicit parent is available
* Remove ash dependencies from NetworkConnectDelegateChromeos, including
GetNativeWindow()
* Add DEPS to restrict access to ash from chrome/browser/chromeos/net

Also do some cleanup:
* Add class comments for captive portal and HaTS dialogs, which I audited.
They do not need to be fixed for mash because they are always displayed in the
default container.
* Fix include-what-you-use.

BUG=657021
TEST=chrome browser_tests

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

[James Cook, 2016-10-27 18:17:24]

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

[commit-bot: I haz the power, 2016-10-27 18:18:09]

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

[James Cook, 2016-10-27 18:20:36]

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

[James Cook, 2016-10-27 18:20:37]

stevenjb, please take a look.

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/enr...
File chrome/browser/chromeos/enrollment_dialog_view.cc (right):

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/enr...
chrome/browser/chromeos/enrollment_dialog_view.cc:122: int container_id =
SystemTrayClient::GetDialogParentContainerId();
As I noted in the last CL: I would like to move GetDialogParentContainerId() to
a utility file, but that would require moving GetUserLoginState() as well, and
I'd prefer to wait on that until the login service is better defined.

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/hat...
File chrome/browser/chromeos/hats/hats_dialog.h (right):

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/hat...
chrome/browser/chromeos/hats/hats_dialog.h:16: // user how satisfied they are
with their Chromebook.
I added this because it wasn't obvious to me at first reading what "HaTS" meant.

[stevenjb, 2016-10-27 18:45:13]

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/enr...
File chrome/browser/chromeos/enrollment_dialog_view.cc (right):

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/enr...
chrome/browser/chromeos/enrollment_dialog_view.cc:122: int container_id =
SystemTrayClient::GetDialogParentContainerId();
On 2016/10/27 18:20:37, James Cook wrote:
> As I noted in the last CL: I would like to move GetDialogParentContainerId()
to
> a utility file, but that would require moving GetUserLoginState() as well, and
> I'd prefer to wait on that until the login service is better defined.

The added dependencies are somewhat unfortunate.

Would it be reasonable to put this into SystemTrayClient for now instead, e.g.
SystemTrayClient::CreateUnownedDialogWidget()?

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/net...
File chrome/browser/chromeos/net/network_connect_delegate_chromeos.cc (right):

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/net...
chrome/browser/chromeos/net/network_connect_delegate_chromeos.cc:14: namespace
chromeos {
nit: blank line

[commit-bot: I haz the power, 2016-10-27 19:05:43]

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

[commit-bot: I haz the power, 2016-10-27 19:05:44]

Dry run: This issue passed the CQ dry run.

[James Cook, 2016-10-27 22:28:13]

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

[James Cook, 2016-10-27 22:28:27]

stevenjb, please take another look.

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/enr...
File chrome/browser/chromeos/enrollment_dialog_view.cc (right):

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/enr...
chrome/browser/chromeos/enrollment_dialog_view.cc:122: int container_id =
SystemTrayClient::GetDialogParentContainerId();
On 2016/10/27 18:45:13, stevenjb wrote:
> On 2016/10/27 18:20:37, James Cook wrote:
> > As I noted in the last CL: I would like to move GetDialogParentContainerId()
> to
> > a utility file, but that would require moving GetUserLoginState() as well,
and
> > I'd prefer to wait on that until the login service is better defined.
> 
> The added dependencies are somewhat unfortunate.
> 
> Would it be reasonable to put this into SystemTrayClient for now instead, e.g.
> SystemTrayClient::CreateUnownedDialogWidget()?

Done. In a follow-up CL I'll migrate NetworkConfigView to use the new function.

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/net...
File chrome/browser/chromeos/net/network_connect_delegate_chromeos.cc (right):

https://codereview.chromium.org/2452283003/diff/1/chrome/browser/chromeos/net...
chrome/browser/chromeos/net/network_connect_delegate_chromeos.cc:14: namespace
chromeos {
On 2016/10/27 18:45:13, stevenjb wrote:
> nit: blank line

Done.

https://codereview.chromium.org/2452283003/diff/20001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_client.h (right):

https://codereview.chromium.org/2452283003/diff/20001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_client.h:43: static views::Widget*
CreateUnownedDialogWidget(
The next CL will need this to return a Widget, even though the current CL does
not.

[commit-bot: I haz the power, 2016-10-27 22:29:14]

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

[stevenjb, 2016-10-27 22:48:31]

Thanks, this makes a lot more sense to me.
LGTM

[commit-bot: I haz the power, 2016-10-27 23:00:04]

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

[commit-bot: I haz the power, 2016-10-27 23:00:05]

Dry run: This issue passed the CQ dry run.

[James Cook, 2016-10-27 23:02:48]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-10-27 23:02:49]

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

[commit-bot: I haz the power, 2016-10-27 23:03:29]

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-27 23:55:37]

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

[commit-bot: I haz the power, 2016-10-27 23:55:38]

Failed to apply patch for chrome/browser/ui/ash/system_tray_client.cc:
While running git apply --index -3 -p1;
  error: patch failed: chrome/browser/ui/ash/system_tray_client.cc:33
  Falling back to three-way merge...
  Applied patch to 'chrome/browser/ui/ash/system_tray_client.cc' with conflicts.
  U chrome/browser/ui/ash/system_tray_client.cc

Patch:       chrome/browser/ui/ash/system_tray_client.cc
Index: chrome/browser/ui/ash/system_tray_client.cc
diff --git a/chrome/browser/ui/ash/system_tray_client.cc
b/chrome/browser/ui/ash/system_tray_client.cc
index
946fda056617dfe7d8ec68e20b4aa0622b780200..698434c70015d52b5210e922bb9a61dfa0d14809
100644
--- a/chrome/browser/ui/ash/system_tray_client.cc
+++ b/chrome/browser/ui/ash/system_tray_client.cc
@@ -8,6 +8,7 @@
 #include "ash/common/session/session_state_delegate.h"
 #include "ash/common/wm_shell.h"
 #include "ash/public/cpp/shell_window_ids.h"
+#include "ash/shell.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
@@ -33,11 +34,16 @@
 #include "content/public/common/service_manager_connection.h"
 #include "net/base/escape.h"
 #include "services/service_manager/public/cpp/connector.h"
+#include "services/ui/public/cpp/property_type_converters.h"
+#include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/views/widget/widget.h"
+#include "ui/views/window/dialog_delegate.h"
 
 using chromeos::DBusThreadManager;
 using chromeos::LoginState;
+using views::Widget;
 
 namespace {
 
@@ -131,6 +137,28 @@ int SystemTrayClient::GetDialogParentContainerId() {
   return ash::kShellWindowId_SystemModalContainer;
 }
 
+// static
+Widget* SystemTrayClient::CreateUnownedDialogWidget(
+    views::WidgetDelegate* widget_delegate) {
+  DCHECK(widget_delegate);
+  Widget::InitParams params = views::DialogDelegate::GetDialogWidgetInitParams(
+      widget_delegate, nullptr, nullptr, gfx::Rect());
+  // Place the dialog in the appropriate modal dialog container, either above
+  // or below the lock screen, based on the login state.
+  int container_id = GetDialogParentContainerId();
+  if (chrome::IsRunningInMash()) {
+    using ui::mojom::WindowManager;
+    params.mus_properties[WindowManager::kInitialContainerId_Property] =
+        mojo::ConvertTo<std::vector<uint8_t>>(container_id);
+  } else {
+    params.parent =
ash::Shell::GetContainer(ash::Shell::GetPrimaryRootWindow(),
+                                             container_id);
+  }
+  Widget* widget = new Widget;  // Owned by native widget.
+  widget->Init(params);
+  return widget;
+}
+

////////////////////////////////////////////////////////////////////////////////
 // ash::mojom::SystemTrayClient:

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

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-10-29 00:23:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2452283003/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-10-29 00:24:14]

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-29 00:52:37]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-29 01:00:56]

Description was changed from

==========
chromeos: Make network enrollment and SIM unlock dialogs work with mash

Chrome running in mash cannot directly access the ash aura window hierarchy.
Instead it must pass an ash window container id when a new window is being
opened.

* Convert dialogs to use container id when no explicit parent is available
* Remove ash dependencies from NetworkConnectDelegateChromeos, including
GetNativeWindow()
* Add DEPS to restrict access to ash from chrome/browser/chromeos/net

Also do some cleanup:
* Add class comments for captive portal and HaTS dialogs, which I audited.
They do not need to be fixed for mash because they are always displayed in the
default container.
* Fix include-what-you-use.

BUG=657021
TEST=chrome browser_tests
==========

to

==========
chromeos: Make network enrollment and SIM unlock dialogs work with mash

Chrome running in mash cannot directly access the ash aura window hierarchy.
Instead it must pass an ash window container id when a new window is being
opened.

* Convert dialogs to use container id when no explicit parent is available
* Remove ash dependencies from NetworkConnectDelegateChromeos, including
GetNativeWindow()
* Add DEPS to restrict access to ash from chrome/browser/chromeos/net

Also do some cleanup:
* Add class comments for captive portal and HaTS dialogs, which I audited.
They do not need to be fixed for mash because they are always displayed in the
default container.
* Fix include-what-you-use.

BUG=657021
TEST=chrome browser_tests

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

[commit-bot: I haz the power, 2016-10-29 01:00:57]

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