mash: Place views Wi-Fi network config dialogs in correct window parent

chrome/browser/chromeos/options/network_config_view.cc

Index: chrome/browser/chromeos/options/network_config_view.cc
diff --git a/chrome/browser/chromeos/options/network_config_view.cc b/chrome/browser/chromeos/options/network_config_view.cc
index 2208aa05b59c7ab110a99061fb86d4ff2bbae47d..85b47fd4da2d6cd8ac46648159638a58e61d6cdf 100644
--- a/chrome/browser/chromeos/options/network_config_view.cc
+++ b/chrome/browser/chromeos/options/network_config_view.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/chromeos/options/wifi_config_view.h"
 #include "chrome/browser/chromeos/options/wimax_config_view.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -25,7 +26,10 @@
 #include "chromeos/network/network_state_handler.h"
 #include "components/device_event_log/device_event_log.h"
 #include "components/user_manager/user.h"
+#include "services/ui/public/cpp/property_type_converters.h"
+#include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "ui/accessibility/ax_view_state.h"
+#include "ui/aura/mus/mus_util.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/color_palette.h"
@@ -38,8 +42,10 @@
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/widget/widget_parent.h"
 
 using views::Widget;
+using views::WidgetParent;
 
 namespace chromeos {
 
@@ -120,7 +126,7 @@ NetworkConfigView::~NetworkConfigView() {
 
 // static
 void NetworkConfigView::Show(const std::string& service_path,
-                             gfx::NativeWindow parent) {
+                             const views::WidgetParent& widget_parent) {
   if (GetActiveDialog() != nullptr)
     return;
   NetworkConfigView* view = new NetworkConfigView();
@@ -137,12 +143,12 @@ void NetworkConfigView::Show(const std::string& service_path,
     return;
   }
   NET_LOG(USER) << "NetworkConfigView::Show: " << service_path;
-  view->ShowDialog(parent);
+  view->ShowDialog(widget_parent);
 }
 
 // static
 void NetworkConfigView::ShowForType(const std::string& type,
-                                    gfx::NativeWindow parent) {
+                                    const views::WidgetParent& widget_parent) {
   if (GetActiveDialog() != nullptr)
     return;
   NetworkConfigView* view = new NetworkConfigView();
@@ -153,7 +159,7 @@ void NetworkConfigView::ShowForType(const std::string& type,
     return;
   }
   NET_LOG(USER) << "NetworkConfigView::ShowForType: " << type;
-  view->ShowDialog(parent);
+  view->ShowDialog(widget_parent);
 }
 
 gfx::NativeWindow NetworkConfigView::GetNativeWindow() const {
@@ -274,17 +280,62 @@ void NetworkConfigView::ViewHierarchyChanged(
   }
 }
 
-void NetworkConfigView::ShowDialog(gfx::NativeWindow parent) {
-  if (parent == nullptr)
-    parent = GetParentForUnhostedDialog();
+void NetworkConfigView::ShowDialog(const views::WidgetParent& widget_parent) {
+  Widget::InitParams params = GetDialogWidgetInitParams(widget_parent);
+  Widget* window = new Widget;
+  window->Init(params);
+  window->SetAlwaysOnTop(true);
+  window->Show();
+}
+
+Widget::InitParams NetworkConfigView::GetDialogWidgetInitParams(
+    const views::WidgetParent& widget_parent) {
+  // Start with the usual params for a dialog.
+  Widget::InitParams params = DialogDelegate::GetDialogWidgetInitParams(
+      this, nullptr, nullptr, gfx::Rect());
+
+  if (chrome::IsRunningInMash()) {
+    // If the native window maps to a mus window, use that.
+    if (widget_parent.native_parent) {
+      params.parent_mus = aura::GetMusWindow(widget_parent.native_parent);
+      if (params.parent_mus)
+        return params;
+    }
+
+    // Use ash container id if available.
+    if (widget_parent.container_id != WidgetParent::kInvalidContainerId) {
+      using ui::mojom::WindowManager;
+      params.mus_properties[WindowManager::kInitialContainerId_Property] =
+          mojo::ConvertTo<std::vector<uint8_t>>(widget_parent.container_id);
+      return params;
+    }
+
+    // Use a fallback window. |parent_mus| may still be null, which will result
+    // in the default window placement based on type and last active display.
+    params.parent_mus = aura::GetMusWindow(GetParentForUnhostedDialog());
+    return params;
+  }
+
+  if (widget_parent.native_parent) {
+    params.parent = widget_parent.native_parent;
+    return params;
+  }
+
+  // Use ash container id if available.
+  if (widget_parent.container_id != WidgetParent::kInvalidContainerId) {
+    params.parent = ash::Shell::GetContainer(ash::Shell::GetPrimaryRootWindow(),
+                                             widget_parent.container_id);
+    return params;
+  }
+
+  // Use a fallback parent.
+  params.parent = GetParentForUnhostedDialog();
   // Failed connections may result in a pop-up with no natural parent window,
   // so provide a fallback context on the primary display. This is necessary
   // becase one of parent or context must be non nullptr.
-  gfx::NativeWindow context =
-      parent ? nullptr : ash::Shell::GetPrimaryRootWindow();
-  Widget* window = DialogDelegate::CreateDialogWidget(this, context, parent);
-  window->SetAlwaysOnTop(true);
-  window->Show();
+  if (!params.parent)
+    params.context = ash::Shell::GetPrimaryRootWindow();
+  return params;
 }

[stevenjb, 2016/10/19 17:40:07]

This doesn't feel like it belongs here. I'm sure we have other dialogs that will
need to do the same thing?

[James Cook, 2016/10/19 18:05:27]

Yeah, I know. Copy/pasting what I said to sky in the initial review:

"In this case I was able to configure the dialog widget parent without
delegating
from src/ui back into src/chrome. I suspect we'll be able to avoid delegating
most (all?) of the time. However, I also expect we'll eventually want to
consolidate this "choose-parent-or-fallback" code in a central location. I'm
waiting to try to centralize it until I see how often this pattern emerges."

I'm working on the next one now.

[stevenjb, 2016/10/19 18:23:38]

Hrmm. Can we at least put this in an anonymous namespace instead of making it a
member? I don't see any member dependencies here.

 
 // ChildNetworkConfigView