Open windows from shelf/launcher context menu on same display

Bug: 1083825
Change-Id: I9272bb8031e73b48b23b72822d86844a157b9cce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210173Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770848}

chrome/browser/ui/app_list/app_context_menu_unittest.cc

@@ -41,6 +41,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/ui_base_features.h"
+#include "ui/display/test/test_screen.h"
 
 using web_app::ProviderType;
 
@@ -162,6 +163,7 @@ class AppContextMenuTest : public AppListTestBase,
 
   void SetUp() override {
     AppListTestBase::SetUp();
+    display::Screen::SetScreenInstance(&test_screen_);
     extensions::MenuManagerFactory::GetInstance()->SetTestingFactory(
         profile(), base::BindRepeating(&MenuManagerFactory));
     controller_ = std::make_unique<FakeAppListControllerDelegate>();
@@ -281,6 +283,7 @@ class AppContextMenuTest : public AppListTestBase,
 
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
+  display::test::TestScreen test_screen_;
   std::unique_ptr<KeyedService> menu_manager_;
   std::unique_ptr<FakeAppListControllerDelegate> controller_;
   std::unique_ptr<FakeAppContextMenuDelegate> menu_delegate_;

chrome/browser/ui/app_list/app_service/app_service_context_menu.cc

@@ -33,6 +33,7 @@
 #include "chrome/common/chrome_features.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/context_menu_params.h"
+#include "ui/display/scoped_display_for_new_windows.h"
 #include "ui/gfx/vector_icon_types.h"
 
 namespace {
@@ -91,6 +92,9 @@ void AppServiceContextMenu::GetMenuModel(GetMenuModelCallback callback) {
 }
 
 void AppServiceContextMenu::ExecuteCommand(int command_id, int event_flags) {
+  // Place new windows on the same display as the context menu.
+  display::ScopedDisplayForNewWindows scoped_display(
+      controller()->GetAppListDisplayId());
   switch (command_id) {
     case ash::LAUNCH_NEW:
       delegate()->ExecuteLaunchCommand(event_flags);

chrome/browser/ui/app_list/extension_app_context_menu.cc

@@ -19,6 +19,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/context_menu_params.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/display/scoped_display_for_new_windows.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/menu/menu_config.h"
 #include "ui/views/vector_icons.h"
@@ -180,6 +181,9 @@ bool ExtensionAppContextMenu::IsCommandIdEnabled(int command_id) const {
 }
 
 void ExtensionAppContextMenu::ExecuteCommand(int command_id, int event_flags) {
+  // Place new windows on the same display as the context menu.
+  display::ScopedDisplayForNewWindows scoped_display(
+      controller()->GetAppListDisplayId());
   if (command_id == ash::LAUNCH_NEW) {
     delegate()->ExecuteLaunchCommand(event_flags);
   } else if (command_id == ash::SHOW_APP_INFO) {

chrome/browser/ui/ash/launcher/app_service/app_service_shelf_context_menu.cc

@@ -44,6 +44,7 @@
 #include "chrome/common/chrome_features.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/context_menu_params.h"
+#include "ui/display/scoped_display_for_new_windows.h"
 #include "ui/gfx/vector_icon_types.h"
 
 namespace {
@@ -119,6 +120,8 @@ void AppServiceShelfContextMenu::GetMenuModel(GetMenuModelCallback callback) {
 
 void AppServiceShelfContextMenu::ExecuteCommand(int command_id,
                                                 int event_flags) {
+  // Place new windows on the same display as the context menu.
+  display::ScopedDisplayForNewWindows scoped_display(display_id());
   if (ExecuteCommonCommand(command_id, event_flags))
     return;
 

chrome/browser/ui/ash/launcher/extension_shelf_context_menu.cc

@@ -27,6 +27,7 @@
 #include "content/public/browser/context_menu_params.h"
 #include "extensions/browser/extension_prefs.h"
 #include "ui/base/models/image_model.h"
+#include "ui/display/scoped_display_for_new_windows.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/paint_vector_icon.h"
 
@@ -37,27 +38,6 @@ bool MenuItemHasLauncherContext(const extensions::MenuItem* item) {
   return item->contexts().Contains(extensions::MenuItem::LAUNCHER);
 }
 
-// Temporarily sets the display for new windows. Only use this when it's
-// guaranteed messages won't be received from ash to update the display.
-// For example, it's OK to use temporarily at function scope, but don't
-// heap-allocate one and hang on to it.
-class ScopedDisplayIdForNewWindows {
- public:
-  explicit ScopedDisplayIdForNewWindows(int64_t display_id)
-      : old_display_id_(display_id) {
-    display::Screen::GetScreen()->SetDisplayForNewWindows(display_id);
-  }
-
-  ~ScopedDisplayIdForNewWindows() {
-    display::Screen::GetScreen()->SetDisplayForNewWindows(old_display_id_);
-  }
-
- private:
-  const int64_t old_display_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedDisplayIdForNewWindows);
-};
-
 }  // namespace
 
 ExtensionShelfContextMenu::ExtensionShelfContextMenu(
@@ -176,7 +156,7 @@ void ExtensionShelfContextMenu::ExecuteCommand(int command_id,
     return;
 
   // Place new windows on the same display as the context menu.
-  ScopedDisplayIdForNewWindows scoped_display(display_id());
+  display::ScopedDisplayForNewWindows scoped_display(display_id());
 
   switch (static_cast<ash::CommandId>(command_id)) {
     case ash::SHOW_APP_INFO: