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

chrome/browser/chromeos/options/network_config_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/options/network_config_view.h"
 
 #include <algorithm>
 
 #include "ash/shell.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/options/network_property_ui_data.h"
 #include "chrome/browser/chromeos/options/vpn_config_view.h"
 #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"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/locale_settings.h"
 #include "chromeos/login/login_state.h"
 #include "chromeos/network/network_state.h"
 #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"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/vector_icons_public.h"
 #include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/controls/image_view.h"
 #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 {
 
 namespace {
 
 gfx::NativeWindow GetParentForUnhostedDialog() {
   if (LoginDisplayHost::default_host()) {
     return LoginDisplayHost::default_host()->GetNativeWindow();
   } else {
     Browser* browser = chrome::FindTabbedBrowser(
@@ skipping 60 matching lines @@
   return child_config_view_ != nullptr;
 }
 
 NetworkConfigView::~NetworkConfigView() {
   DCHECK(GetActiveDialog() == this);
   SetActiveDialog(nullptr);
 }
 
 // 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();
   const NetworkState* network = NetworkHandler::Get()->network_state_handler()->
       GetNetworkState(service_path);
   if (!network) {
     LOG(ERROR) << "NetworkConfigView::Show called with invalid service_path";
     return;
   }
   if (!view->InitWithNetworkState(network)) {
     LOG(ERROR) << "NetworkConfigView::Show called with invalid network type: "
                << network->type();
     delete view;
     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();
   if (!view->InitWithType(type)) {
     LOG(ERROR) << "NetworkConfigView::ShowForType called with invalid type: "
                << type;
     delete view;
     return;
   }
   NET_LOG(USER) << "NetworkConfigView::ShowForType: " << type;
-  view->ShowDialog(parent);
+  view->ShowDialog(widget_parent);
 }
 
 gfx::NativeWindow NetworkConfigView::GetNativeWindow() const {
   return GetWidget()->GetNativeWindow();
 }
 
 base::string16 NetworkConfigView::GetDialogButtonLabel(
     ui::DialogButton button) const {
   if (button == ui::DIALOG_BUTTON_OK) {
     if (child_config_view_->IsConfigureDialog())
@@ skipping 100 matching lines @@
 void NetworkConfigView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   // Can't init before we're inserted into a Container, because we require
   // a HWND to parent native child controls to.
   views::DialogDelegateView::ViewHierarchyChanged(details);
   if (details.is_add && details.child == this) {
     AddChildView(child_config_view_);
   }
 }
 
-void NetworkConfigView::ShowDialog(gfx::NativeWindow parent) {
-  if (parent == nullptr)
-    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);
+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.
+  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
 
 ChildNetworkConfigView::ChildNetworkConfigView(
     NetworkConfigView* parent,
     const std::string& service_path)
     : parent_(parent),
       service_path_(service_path) {
 }
 
 ChildNetworkConfigView::~ChildNetworkConfigView() {
@@ skipping 28 matching lines @@
 }
 
 ControlledSettingIndicatorView::~ControlledSettingIndicatorView() {}
 
 gfx::Size ControlledSettingIndicatorView::GetPreferredSize() const {
   return (managed_ && visible()) ? image_view_->GetPreferredSize()
                                  : gfx::Size();
 }
 
 }  // namespace chromeos