cros: Centralize Widget setup for ash containers

Widget::InitParams setup is different in classic ash vs. mash.
Centralize the boilerplate code. This eliminates some ash/shell.h
includes, making it clearer that there's only one issue here.
It also reduces the chance of weird compiler errors if you
copy/paste the boilerplate but forget the type converters header.

Bug: 756057
Test: browser_tests
Change-Id: I3f1c76f2b174ef305697f8551a6923534a73dbaf
Reviewed-on: https://chromium-review.googlesource.com/999813
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548875}

chrome/browser/chromeos/accessibility/chromevox_panel.cc

@@ -7,21 +7,18 @@
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/interfaces/accessibility_controller.mojom.h"
 #include "ash/public/interfaces/constants.mojom.h"
-#include "ash/shell.h"  // mash-ok
 #include "base/macros.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/ash_config.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/data_use_measurement/data_use_web_contents_observer.h"
 #include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
+#include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/service_manager_connection.h"
 #include "extensions/browser/view_type_utils.h"
-#include "mojo/public/cpp/bindings/type_converter.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 "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/views/controls/webview/webview.h"
@@ -106,20 +103,9 @@ ChromeVoxPanel::ChromeVoxPanel(content::BrowserContext* browser_context,
       views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
   // Placing the panel in the accessibility panel container allows ash to manage
   // both the window bounds and display work area.
-  const int container_id = ash::kShellWindowId_AccessibilityPanelContainer;
-  const display::Display primary_display =
-      display::Screen::GetScreen()->GetPrimaryDisplay();
-  if (chromeos::GetAshConfig() == ash::Config::MASH) {
-    using ui::mojom::WindowManager;
-    params.mus_properties[WindowManager::kContainerId_InitProperty] =
-        mojo::ConvertTo<std::vector<uint8_t>>(container_id);
-    params.mus_properties[WindowManager::kDisplayId_InitProperty] =
-        mojo::ConvertTo<std::vector<uint8_t>>(primary_display.id());
-  } else {
-    params.parent = ash::Shell::GetContainer(ash::Shell::GetPrimaryRootWindow(),
-                                             container_id);
-  }
-  params.bounds = primary_display.bounds();
+  ash_util::SetupWidgetInitParamsForContainer(
+      &params, ash::kShellWindowId_AccessibilityPanelContainer);
+  params.bounds = display::Screen::GetScreen()->GetPrimaryDisplay().bounds();
   params.delegate = this;
   params.activatable = views::Widget::InitParams::ACTIVATABLE_NO;
   params.name = "ChromeVoxPanel";

chrome/browser/chromeos/lock_screen_apps/toast_dialog_view.cc

@@ -7,14 +7,10 @@
 #include <memory>
 #include <utility>
 
-#include "ash/public/cpp/config.h"
 #include "ash/public/cpp/shell_window_ids.h"
-#include "ash/shell.h"
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/grit/generated_resources.h"
-#include "services/ui/public/cpp/property_type_converters.h"
-#include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "ui/aura/window.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/geometry/rect.h"
@@ -80,15 +76,8 @@ void ToastDialogView::Show() {
   views::Widget::InitParams params =
       GetDialogWidgetInitParams(this, nullptr, nullptr, gfx::Rect());
 
-  const int container_id = ash::kShellWindowId_SettingBubbleContainer;
-  if (ash_util::IsRunningInMash()) {
-    using ui::mojom::WindowManager;
-    params.mus_properties[WindowManager::kContainerId_InitProperty] =
-        mojo::ConvertTo<std::vector<uint8_t>>(container_id);
-  } else {
-    params.parent = ash::Shell::GetContainer(ash::Shell::GetPrimaryRootWindow(),
-                                             container_id);
-  }
+  ash_util::SetupWidgetInitParamsForContainer(
+      &params, ash::kShellWindowId_SettingBubbleContainer);
 
   views::Widget* widget = new views::Widget;  // owned by native widget
   widget->Init(params);

chrome/browser/chromeos/login/ui/gaia_dialog_delegate.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/chromeos/login/ui/gaia_dialog_delegate.h"
 
 #include "ash/public/cpp/shell_window_ids.h"
-#include "ash/shell.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host_views.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
@@ -13,8 +12,6 @@
 #include "chrome/browser/ui/webui/chrome_web_contents_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 #include "content/public/browser/web_contents.h"
-#include "services/ui/public/cpp/property_type_converters.h"
-#include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
@@ -52,16 +49,8 @@ void GaiaDialogDelegate::Init() {
   views::Widget::InitParams params(
       views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
   params.delegate = dialog_view_;
-  if (!ash_util::IsRunningInMash()) {
-    params.parent =
-        ash::Shell::GetContainer(ash::Shell::GetPrimaryRootWindow(),
-                                 ash::kShellWindowId_LockSystemModalContainer);
-  } else {
-    using ui::mojom::WindowManager;
-    params.mus_properties[WindowManager::kContainerId_InitProperty] =
-        mojo::ConvertTo<std::vector<uint8_t>>(
-            static_cast<int32_t>(ash::kShellWindowId_LockSystemModalContainer));
-  }
+  ash_util::SetupWidgetInitParamsForContainer(
+      &params, ash::kShellWindowId_LockSystemModalContainer);
 
   dialog_widget_ = new views::Widget;
   dialog_widget_->Init(params);

chrome/browser/ui/ash/ash_util.cc

@@ -7,14 +7,20 @@
 #include "ash/accelerators/accelerator_controller.h"
 #include "ash/mojo_interface_factory.h"
 #include "ash/public/cpp/config.h"
+#include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/interfaces/event_properties.mojom.h"
 #include "ash/shell.h"
 #include "base/macros.h"
 #include "chrome/browser/chromeos/ash_config.h"
+#include "mojo/public/cpp/bindings/type_converter.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/cpp/service.h"
 #include "services/service_manager/public/mojom/interface_provider_spec.mojom.h"
+#include "services/ui/public/cpp/property_type_converters.h"
+#include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "ui/aura/window_event_dispatcher.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
 
 namespace ash_util {
 
@@ -74,4 +80,22 @@ bool WillAshProcessAcceleratorForEvent(const ui::KeyEvent& key_event) {
              ash::mojom::kWillProcessAccelerator_KeyEventProperty);
 }
 
+void SetupWidgetInitParamsForContainer(views::Widget::InitParams* params,
+                                       int container_id) {
+  DCHECK_GE(container_id, ash::kShellWindowId_MinContainer);
+  DCHECK_LE(container_id, ash::kShellWindowId_MaxContainer);
+
+  if (chromeos::GetAshConfig() == ash::Config::MASH) {
+    using ui::mojom::WindowManager;
+    params->mus_properties[WindowManager::kContainerId_InitProperty] =
+        mojo::ConvertTo<std::vector<uint8_t>>(container_id);
+    params->mus_properties[WindowManager::kDisplayId_InitProperty] =
+        mojo::ConvertTo<std::vector<uint8_t>>(
+            display::Screen::GetScreen()->GetPrimaryDisplay().id());
+  } else {
+    params->parent = ash::Shell::GetContainer(
+        ash::Shell::GetPrimaryRootWindow(), container_id);
+  }
+}
+
 }  // namespace ash_util

chrome/browser/ui/ash/ash_util.h

@@ -10,6 +10,7 @@
 #include "ash/public/cpp/config.h"
 #include "base/memory/ref_counted.h"
 #include "base/single_thread_task_runner.h"
+#include "ui/views/widget/widget.h"
 
 namespace service_manager {
 class Service;
@@ -41,6 +42,13 @@ bool IsAcceleratorDeprecated(const ui::Accelerator& accelerator);
 // Returns true if ash has an accelerator for |key_event| that is enabled.
 bool WillAshProcessAcceleratorForEvent(const ui::KeyEvent& key_event);
 
+// Sets up |params| to place the widget in an ash shell window container on
+// the primary display. See ash/public/cpp/shell_window_ids.h for |container_id|
+// values.
+// TODO(jamescook): Extend to take a display_id.
+void SetupWidgetInitParamsForContainer(views::Widget::InitParams* params,
+                                       int container_id);
+
 }  // namespace ash_util
 
 #endif  // CHROME_BROWSER_UI_ASH_ASH_UTIL_H_

chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc

@@ -168,18 +168,7 @@ void ChromeNativeAppWindowViewsAuraAsh::OnBeforeWidgetInit(
     int container_id = create_params.is_ime_window
                            ? ash::kShellWindowId_ImeWindowParentContainer
                            : ash::kShellWindowId_LockActionHandlerContainer;
-    if (ash_util::IsRunningInMash()) {
-      init_params->mus_properties
-          [ui::mojom::WindowManager::kContainerId_InitProperty] =
-          mojo::ConvertTo<std::vector<uint8_t>>(container_id);
-      int display_id = display::Screen::GetScreen()->GetPrimaryDisplay().id();
-      init_params
-          ->mus_properties[ui::mojom::WindowManager::kDisplayId_InitProperty] =
-          mojo::ConvertTo<std::vector<uint8_t>>(display_id);
-    } else {
-      init_params->parent = ash::Shell::GetContainer(
-          ash::Shell::GetPrimaryRootWindow(), container_id);
-    }
+    ash_util::SetupWidgetInitParamsForContainer(init_params, container_id);
   }
   DCHECK_NE(AppWindow::WINDOW_TYPE_PANEL, create_params.window_type);
   init_params->mus_properties

chrome/browser/ui/views/chrome_web_dialog_view.cc

@@ -9,12 +9,8 @@
 #include "ui/views/widget/widget.h"
 
 #if defined(OS_CHROMEOS)
-// gn check complains on Linux Ozone.
-#include "ash/public/cpp/shell_window_ids.h"  // nogncheck
-#include "ash/shell.h"
+#include "ash/public/cpp/shell_window_ids.h"
 #include "chrome/browser/ui/ash/ash_util.h"
-#include "services/ui/public/cpp/property_type_converters.h"
-#include "services/ui/public/interfaces/window_manager.mojom.h"
 #endif  // defined(OS_CHROMEOS)
 
 namespace chrome {
@@ -59,14 +55,7 @@ gfx::NativeWindow ShowWebDialogInContainer(int container_id,
       new views::WebDialogView(context, delegate, new ChromeWebContentsHandler);
   views::Widget::InitParams params;
   params.delegate = view;
-  if (ash_util::IsRunningInMash()) {
-    using ui::mojom::WindowManager;
-    params.mus_properties[WindowManager::kContainerId_InitProperty] =
-        mojo::ConvertTo<std::vector<uint8_t>>(container_id);
-  } else {
-    params.parent = ash::Shell::GetContainer(ash::Shell::GetPrimaryRootWindow(),
-                                             container_id);
-  }
+  ash_util::SetupWidgetInitParamsForContainer(&params, container_id);
   return ShowWebDialogWidget(params, view);
 }
 #endif  // defined(OS_CHROMEOS)