Hide ARC++ Windows with the same logical_window_id

Windows are considered one "logical window" if they are created
with the same |shelf_group_id| and |logical_window_id|.
The consequence: only one window per logical window is shown in
the shelf, the alt-tab menu, and the desk mini view.

The use case are windows across multiple screen, which are not possible
in Chrome OS. So the app creates one window per screen and marks them
as one logical window.

This is done by the |ArcAppWindowInfo|, which stores the IDs mentioned
above. It is setting the |HideInDeskMiniView| property, the
|HideInOverview| property and the new |HideInShelf| property on a
window. The AppWindowLauncherItemController pays attention to the
HideInShelf property and will not use hidden windows as
|last_active_window|s, nor return them via |GetAppMenuItems|.

When a window closes, it might have been the visible window of a
logical window, so the |AppServiceAppWindowArcTracker| checks this and
tells a different app of the same logical window to become visible.

browser_tests --gtest_filter=ArcAppLauncherBrowserTest.LogicalWindow

Test: 
Bug: b/148875131
Change-Id: Ide57ac8dbc77137d4678763bdc88806f3937c0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207316Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Commit-Queue: Philipp Weiß <phweiss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774043}

ash/public/cpp/window_properties.cc

@@ -31,6 +31,7 @@ DEFINE_UI_CLASS_PROPERTY_KEY(bool, kCanAttachToAnotherWindowKey, true)
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kCanConsumeSystemKeysKey, false)
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kExcludeInMruKey, false)
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kHideInOverviewKey, false)
+DEFINE_UI_CLASS_PROPERTY_KEY(bool, kHideInShelfKey, false)
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kHideShelfWhenFullscreenKey, true)
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kImmersiveImpliedByFullscreen, true)
 DEFINE_UI_CLASS_PROPERTY_KEY(bool, kImmersiveIsActive, false)

ash/public/cpp/window_properties.h

@@ -73,6 +73,10 @@ ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
 ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
     kHideInOverviewKey;
 
+// A property key to indicate whether we should hide this window in the shelf.
+ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const
+    kHideInShelfKey;
+
 // Whether the shelf should be hidden when this window is put into fullscreen.
 // Exposed because some windows want to explicitly opt-out of this.
 ASH_PUBLIC_EXPORT extern const aura::WindowProperty<bool>* const

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

@@ -115,6 +115,9 @@ void AppServiceAppWindowArcTracker::OnTaskCreated(
       arc::ArcAppShelfId::FromIntentAndAppId(intent, arc_app_id);
   task_id_to_arc_app_window_info_[task_id] = std::make_unique<ArcAppWindowInfo>(
       arc_app_shelf_id, intent, package_name);
+  // Hide from shelf if there already is some task representing the window.
+  if (GetTaskIdSharingLogicalWindow(task_id) != arc::kNoTaskId)
+    task_id_to_arc_app_window_info_[task_id]->set_hidden_from_shelf(true);
 
   CheckAndAttachControllers();
 
@@ -164,6 +167,12 @@ void AppServiceAppWindowArcTracker::OnTaskDestroyed(int task_id) {
   if (it == task_id_to_arc_app_window_info_.end())
     return;
 
+  if (!it->second->logical_window_id().empty()) {
+    const int other_id = GetTaskIdSharingLogicalWindow(task_id);
+    if (other_id != arc::kNoTaskId)
+      task_id_to_arc_app_window_info_[other_id]->set_hidden_from_shelf(false);
+  }
+
   aura::Window* const window = it->second.get()->window();
   if (window) {
     // For ARC apps, window may be recreated in some cases, and OnTaskSetActive
@@ -407,6 +416,25 @@ void AppServiceAppWindowArcTracker::HandlePlayStoreLaunch(
   }
 }
 
+int AppServiceAppWindowArcTracker::GetTaskIdSharingLogicalWindow(int task_id) {
+  auto fixed_it = task_id_to_arc_app_window_info_.find(task_id);
+  if (fixed_it == task_id_to_arc_app_window_info_.end())
+    return arc::kNoTaskId;
+  if (fixed_it->second->logical_window_id().empty())
+    return arc::kNoTaskId;
+  for (auto it = task_id_to_arc_app_window_info_.begin();
+       it != task_id_to_arc_app_window_info_.end(); it++) {
+    if (task_id == it->first)
+      continue;
+    if (fixed_it->second->logical_window_id() ==
+            it->second->logical_window_id() &&
+        fixed_it->second->shelf_id() == it->second->shelf_id()) {
+      return it->first;
+    }
+  }
+  return arc::kNoTaskId;
+}
+
 std::vector<int> AppServiceAppWindowArcTracker::GetTaskIdsForApp(
     const std::string& app_id) const {
   std::vector<int> task_ids;

chrome/browser/ui/ash/launcher/app_service/app_service_app_window_arc_tracker.h

@@ -110,6 +110,11 @@ class AppServiceAppWindowArcTracker : public ArcAppListPrefs::Observer,
 
   void HandlePlayStoreLaunch(ArcAppWindowInfo* app_window_info);
 
+  // Returns a task ID different from |task_id| that is part of the same
+  // logical window. Return arc::kNoTaskId if there is no such window.
+  // For consistency, always return the lowest such task ID.
+  int GetTaskIdSharingLogicalWindow(int task_id);
+
   std::vector<int> GetTaskIdsForApp(const std::string& arc_app_id) const;
 
   Profile* const observed_profile_;

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

@@ -6,6 +6,7 @@
 #include <vector>
 
 #include "ash/public/cpp/shelf_model.h"
+#include "ash/public/cpp/window_properties.h"
 #include "ash/shell.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_feature_list.h"
@@ -100,6 +101,15 @@ std::vector<arc::mojom::AppInfoPtr> GetTestAppsList(
   return apps;
 }
 
+std::string CreateIntentUriWithShelfGroupAndLogicalWindow(
+    const std::string& shelf_group_id,
+    const std::string& logical_window_id) {
+  return base::StringPrintf(
+      "#Intent;S.org.chromium.arc.logical_window_id=%s;"
+      "S.org.chromium.arc.shelf_group_id=%s;end",
+      logical_window_id.c_str(), shelf_group_id.c_str());
+}
+
 }  // namespace
 
 class AppServiceAppWindowBrowserTest
@@ -630,6 +640,75 @@ IN_PROC_BROWSER_TEST_F(AppServiceAppWindowArcAppBrowserTest, ArcAppsWindow) {
   StopInstance();
 }
 
+// Test what happens when the logical window ID is provided, and some window
+// might be hidden in the shelf.
+IN_PROC_BROWSER_TEST_F(AppServiceAppWindowArcAppBrowserTest, LogicalWindowId) {
+  // Install app to remember existing apps.
+  StartInstance();
+  InstallTestApps(kTestAppPackage, true);
+  SendPackageAdded(kTestAppPackage, false);
+
+  // Create the windows for the app.
+  views::Widget* arc_window1 = CreateArcWindow("org.chromium.arc.1");
+  views::Widget* arc_window2 = CreateArcWindow("org.chromium.arc.2");
+
+  // Simulate task creation so the app is marked as running/open.
+  const std::string app_id = GetTestApp1Id(kTestAppPackage);
+  std::unique_ptr<ArcAppListPrefs::AppInfo> info = app_prefs()->GetApp(app_id);
+  app_host()->OnTaskCreated(1, info->package_name, info->activity, info->name,
+                            CreateIntentUriWithShelfGroupAndLogicalWindow(
+                                "shelf_group_1", "logical_window_1"));
+  app_host()->OnTaskCreated(2, info->package_name, info->activity, info->name,
+                            CreateIntentUriWithShelfGroupAndLogicalWindow(
+                                "shelf_group_1", "logical_window_1"));
+
+  // Both windows should show up in the instance registry.
+  auto windows = app_service_proxy_->InstanceRegistry().GetWindows(app_id);
+  EXPECT_EQ(2u, windows.size());
+
+  // Of those two, one should be hidden.
+  auto is_hidden = [](aura::Window* w) {
+    return w->GetProperty(ash::kHideInShelfKey);
+  };
+  EXPECT_EQ(1u, std::count_if(windows.begin(), windows.end(), is_hidden));
+
+  // The hidden window should be task_id 2.
+  aura::Window* window1 =
+      *(std::find_if_not(windows.begin(), windows.end(), is_hidden));
+  aura::Window* window2 =
+      *(std::find_if(windows.begin(), windows.end(), is_hidden));
+
+  apps::InstanceState latest_state =
+      app_service_proxy_->InstanceRegistry().GetState(window1);
+  EXPECT_EQ(apps::InstanceState::kStarted | apps::InstanceState::kRunning,
+            latest_state);
+  latest_state = app_service_proxy_->InstanceRegistry().GetState(window2);
+  EXPECT_EQ(apps::InstanceState::kStarted | apps::InstanceState::kRunning,
+            latest_state);
+
+  // If the user focuses window 2, it should become active, but still hidden in
+  // the shelf.
+  app_host()->OnTaskSetActive(2);
+  latest_state = app_service_proxy_->InstanceRegistry().GetState(window2);
+  EXPECT_EQ(apps::InstanceState::kStarted | apps::InstanceState::kRunning |
+                apps::InstanceState::kActive | apps::InstanceState::kVisible,
+            latest_state);
+  EXPECT_TRUE(window2->GetProperty(ash::kHideInShelfKey));
+
+  // Close first window. No window should be hidden anymore.
+  arc_window1->CloseNow();
+  app_host()->OnTaskDestroyed(1);
+  windows = app_service_proxy_->InstanceRegistry().GetWindows(app_id);
+  EXPECT_EQ(1u, windows.size());
+  EXPECT_EQ(0u, std::count_if(windows.begin(), windows.end(), is_hidden));
+
+  // Close second window.
+  app_host()->OnTaskDestroyed(2);
+  arc_window2->CloseNow();
+  windows = app_service_proxy_->InstanceRegistry().GetWindows(app_id);
+  EXPECT_EQ(0u, windows.size());
+}
+
 INSTANTIATE_TEST_SUITE_P(All,
                          AppServiceAppWindowWebAppBrowserTest,
                          ::testing::Values(web_app::ProviderType::kBookmarkApps,

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

@@ -121,7 +121,8 @@ void AppServiceAppWindowLauncherItemController::OnWindowTitleChanged(
   if (!IsChromeApp())
     return;
 
-  ui::BaseWindow* const base_window = GetAppWindow(window);
+  ui::BaseWindow* const base_window =
+      GetAppWindow(window, true /*include_hidden*/);
 
   // For Chrome apps, use the window title (if set) to differentiate
   // show_in_shelf window shelf items instead of the default behavior of using

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

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "ash/public/cpp/shelf_types.h"
+#include "ash/public/cpp/window_properties.h"
 #include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/launcher_controller_helper.h"
@@ -23,16 +24,20 @@ AppWindowLauncherItemController::AppWindowLauncherItemController(
 AppWindowLauncherItemController::~AppWindowLauncherItemController() {}
 
 void AppWindowLauncherItemController::AddWindow(ui::BaseWindow* app_window) {
-  windows_.push_front(app_window);
   aura::Window* window = app_window->GetNativeWindow();
   if (window)
     observed_windows_.Add(window);
+  if (window && window->GetProperty(ash::kHideInShelfKey))
+    hidden_windows_.push_front(app_window);
+  else
+    windows_.push_front(app_window);
   UpdateShelfItemIcon();
 }
 
 AppWindowLauncherItemController::WindowList::iterator
-AppWindowLauncherItemController::GetFromNativeWindow(aura::Window* window) {
-  return std::find_if(windows_.begin(), windows_.end(),
+AppWindowLauncherItemController::GetFromNativeWindow(aura::Window* window,
+                                                     WindowList& list) {
+  return std::find_if(list.begin(), list.end(),
                       [window](ui::BaseWindow* base_window) {
                         return base_window->GetNativeWindow() == window;
                       });
@@ -46,24 +51,38 @@ void AppWindowLauncherItemController::RemoveWindow(ui::BaseWindow* app_window) {
   if (app_window == last_active_window_)
     last_active_window_ = nullptr;
   auto iter = std::find(windows_.begin(), windows_.end(), app_window);
-  if (iter == windows_.end()) {
-    NOTREACHED();
-    return;
+  if (iter != windows_.end()) {
+    windows_.erase(iter);
+  } else {
+    iter =
+        std::find(hidden_windows_.begin(), hidden_windows_.end(), app_window);
+    if (iter != hidden_windows_.end()) {
+      hidden_windows_.erase(iter);
+    } else {
+      NOTREACHED();
+      return;
+    }
   }
-  windows_.erase(iter);
   UpdateShelfItemIcon();
 }
 
 ui::BaseWindow* AppWindowLauncherItemController::GetAppWindow(
-    aura::Window* window) {
-  const auto iter = GetFromNativeWindow(window);
+    aura::Window* window,
+    bool include_hidden) {
+  auto iter = GetFromNativeWindow(window, windows_);
   if (iter != windows_.end())
     return *iter;
+  if (include_hidden) {
+    iter = GetFromNativeWindow(window, hidden_windows_);
+    if (iter != hidden_windows_.end())
+      return *iter;
+  }
   return nullptr;
 }
 
 void AppWindowLauncherItemController::SetActiveWindow(aura::Window* window) {
-  ui::BaseWindow* app_window = GetAppWindow(window);
+  // If the window is hidden, do not set it as last_active_window
+  ui::BaseWindow* app_window = GetAppWindow(window, false);
   if (app_window)
     last_active_window_ = app_window;
   UpdateShelfItemIcon();
@@ -132,6 +151,8 @@ void AppWindowLauncherItemController::GetContextMenu(
 void AppWindowLauncherItemController::Close() {
   for (auto* window : windows_)
     window->Close();
+  for (auto* window : hidden_windows_)
+    window->Close();
 }
 
 void AppWindowLauncherItemController::ActivateIndexedApp(size_t index) {
@@ -158,6 +179,8 @@ void AppWindowLauncherItemController::OnWindowPropertyChanged(
     ChromeLauncherController::instance()->SetItemStatus(shelf_id(), status);
   } else if (key == aura::client::kAppIconKey) {
     UpdateShelfItemIcon();
+  } else if (key == ash::kHideInShelfKey) {
+    UpdateWindowInLists(window);
   }
 }
 
@@ -219,6 +242,27 @@ void AppWindowLauncherItemController::UpdateShelfItemIcon() {
   }
 }
 
+void AppWindowLauncherItemController::UpdateWindowInLists(
+    aura::Window* window) {
+  if (window->GetProperty(ash::kHideInShelfKey)) {
+    // Hide Window:
+    auto it = GetFromNativeWindow(window, windows_);
+    if (it != windows_.end()) {
+      hidden_windows_.push_front(*it);
+      windows_.erase(it);
+      UpdateShelfItemIcon();
+    }
+  } else {
+    // Unhide window:
+    auto it = GetFromNativeWindow(window, hidden_windows_);
+    if (it != hidden_windows_.end()) {
+      windows_.push_front(*it);
+      hidden_windows_.erase(it);
+      UpdateShelfItemIcon();
+    }
+  }
+}
+
 void AppWindowLauncherItemController::ExecuteCommand(bool from_context_menu,
                                                      int64_t command_id,
                                                      int32_t event_flags,

chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h

@@ -39,7 +39,7 @@ class AppWindowLauncherItemController : public ash::ShelfItemDelegate,
   void RemoveWindow(ui::BaseWindow* window);
 
   void SetActiveWindow(aura::Window* window);
-  ui::BaseWindow* GetAppWindow(aura::Window* window);
+  ui::BaseWindow* GetAppWindow(aura::Window* window, bool include_hidden);
 
   // ash::ShelfItemDelegate overrides:
   AppWindowLauncherItemController* AsAppWindowLauncherItemController() override;
@@ -87,15 +87,24 @@ class AppWindowLauncherItemController : public ash::ShelfItemDelegate,
   ash::ShelfAction ActivateOrAdvanceToNextAppWindow(
       ui::BaseWindow* window_to_show);
 
-  WindowList::iterator GetFromNativeWindow(aura::Window* window);
+  WindowList::iterator GetFromNativeWindow(aura::Window* window,
+                                           WindowList& list);
 
   // Handles the case when the app window in this controller has been changed,
   // and sets the new controller icon based on the currently active window.
   void UpdateShelfItemIcon();
 
-  // List of associated app windows
+  // Move a window between windows_ and hidden_windows_ list, depending on
+  // changes in the ash::kHideInShelfKey property.
+  void UpdateWindowInLists(aura::Window* window);
+
+  // List of visible associated app windows
   WindowList windows_;
 
+  // List of hidden associated app windows. These windows will not appear in
+  // the UI.
+  WindowList hidden_windows_;
+
   // Pointer to the most recently active app window
   // TODO(khmel): Get rid of |last_active_window_| and provide more reliable
   // way to determine active window.

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

@@ -4,12 +4,31 @@
 
 #include "chrome/browser/ui/ash/launcher/arc_app_window_info.h"
 
+#include "ash/public/cpp/window_properties.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
 
 namespace {
 
+// Prefix in intent that specifies a logical window. Among a group of windows
+// belonging to the same logical window, only one will be represented in the
+// shelf and in the alt-tab menu. S. means string type.
+constexpr char kLogicalWindowIntentPrefix[] =
+    "S.org.chromium.arc.logical_window_id=";
+
 constexpr size_t kMaxIconPngSize = 64 * 1024;  // 64 kb
 
+std::string GetLogicalWindowIdFromIntent(const std::string& launch_intent) {
+  arc::Intent intent;
+  if (!arc::ParseIntent(launch_intent, &intent))
+    return std::string();
+  const std::string prefix(kLogicalWindowIntentPrefix);
+  for (const auto& param : intent.extra_params()) {
+    if (base::StartsWith(param, prefix, base::CompareCase::SENSITIVE))
+      return param.substr(prefix.length());
+  }
+  return std::string();
+}
+
 }  // namespace
 
 ArcAppWindowInfo::ArcAppWindowInfo(const arc::ArcAppShelfId& app_shelf_id,
@@ -17,7 +36,8 @@ ArcAppWindowInfo::ArcAppWindowInfo(const arc::ArcAppShelfId& app_shelf_id,
                                    const std::string& package_name)
     : app_shelf_id_(app_shelf_id),
       launch_intent_(launch_intent),
-      package_name_(package_name) {}
+      package_name_(package_name),
+      logical_window_id_(GetLogicalWindowIdFromIntent(launch_intent)) {}
 
 ArcAppWindowInfo::~ArcAppWindowInfo() = default;
 
@@ -36,8 +56,25 @@ void ArcAppWindowInfo::SetDescription(
     VLOG(1) << "Task icon size is too big " << icon_data_png.size() << ".";
 }
 
+void ArcAppWindowInfo::set_hidden_from_shelf(bool hidden) {
+  if (hidden_from_shelf_ != hidden) {
+    hidden_from_shelf_ = hidden;
+    UpdateWindowProperties();
+  }
+}
+
+void ArcAppWindowInfo::UpdateWindowProperties() {
+  aura::Window* const win = window();
+  if (!win)
+    return;
+  win->SetProperty(ash::kHideInDeskMiniViewKey, hidden_from_shelf_);
+  win->SetProperty(ash::kHideInOverviewKey, hidden_from_shelf_);
+  win->SetProperty(ash::kHideInShelfKey, hidden_from_shelf_);
+}
+
 void ArcAppWindowInfo::set_window(aura::Window* window) {
   window_ = window;
+  UpdateWindowProperties();
 }
 
 aura::Window* ArcAppWindowInfo::ArcAppWindowInfo::window() {
@@ -67,3 +104,7 @@ const std::string& ArcAppWindowInfo::title() const {
 const std::vector<uint8_t>& ArcAppWindowInfo::icon_data_png() const {
   return icon_data_png_;
 }
+
+const std::string& ArcAppWindowInfo::logical_window_id() const {
+  return logical_window_id_;
+}

chrome/browser/ui/ash/launcher/arc_app_window_info.h

@@ -29,6 +29,8 @@ class ArcAppWindowInfo {
 
   void set_window(aura::Window* window);
 
+  void set_hidden_from_shelf(bool hidden);
+
   aura::Window* window();
 
   const arc::ArcAppShelfId& app_shelf_id() const;
@@ -43,10 +45,17 @@ class ArcAppWindowInfo {
 
   const std::vector<uint8_t>& icon_data_png() const;
 
+  const std::string& logical_window_id() const;
+
  private:
+  // Updates window properties depending on the hidden_from_shelf_ setting.
+  void UpdateWindowProperties();
+
   const arc::ArcAppShelfId app_shelf_id_;
   const std::string launch_intent_;
   const std::string package_name_;
+  const std::string logical_window_id_;
+  bool hidden_from_shelf_ = false;
   // Keeps overridden window title.
   std::string title_;
   // Keeps overridden window icon.

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

@@ -230,7 +230,8 @@ void ExtensionAppWindowLauncherController::UnregisterApp(aura::Window* window) {
   ExtensionAppWindowLauncherItemController* controller =
       app_controller_iter->second;
 
-  controller->RemoveWindow(controller->GetAppWindow(window));
+  controller->RemoveWindow(
+      controller->GetAppWindow(window, true /*include_hidden*/));
   if (controller->window_count() == 0) {
     // If this is the last window associated with the app window shelf id,
     // close the shelf item.

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

@@ -73,7 +73,7 @@ void ExtensionAppWindowLauncherItemController::ExecuteCommand(
 
 void ExtensionAppWindowLauncherItemController::OnWindowTitleChanged(
     aura::Window* window) {
-  ui::BaseWindow* base_window = GetAppWindow(window);
+  ui::BaseWindow* base_window = GetAppWindow(window, true /*include_hidden*/);
   extensions::AppWindowRegistry* app_window_registry =
       extensions::AppWindowRegistry::Get(
           ChromeLauncherController::instance()->profile());