Factor out the app type detection from exo

Intoduce WMHelper::AppPropertyResolver to populate window properties
for given |app_id| and |startup_id".

Move the code to identify AppType property to app service directory. EXO should be generic window/composition server and should
is a generic window server and should interpret the application id.

Bug: None
Test: covered by unittest

Change-Id: I134c4071a515a0b9bdee7fbb17d1487c2eb60b95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2526500
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Nic Hollingum <hollingum@google.com>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828062}

chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_browsertest.cc

@@ -11,6 +11,7 @@
 #include "base/feature_list.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/ash/launcher/app_service/exo_app_type_resolver.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chromeos/constants/chromeos_features.h"
@@ -60,6 +61,8 @@ class ArcAccessibilityHelperBridgeBrowserTest : public InProcessBrowserTest {
         browser()->profile());
 
     wm_helper_ = std::make_unique<exo::WMHelperChromeOS>();
+    wm_helper_->RegisterAppPropertyResolver(
+        std::make_unique<ExoAppTypeResolver>());
   }
 
   void TearDownOnMainThread() override {
@@ -78,6 +81,7 @@ class ArcAccessibilityHelperBridgeBrowserTest : public InProcessBrowserTest {
   ArcTestWindow MakeTestWindow(std::string name) {
     ArcTestWindow ret;
     exo::test::ExoTestHelper helper;
+
     ret.surface = std::make_unique<exo::Surface>();
     ret.buffer = std::make_unique<exo::Buffer>(
         helper.CreateGpuMemoryBuffer(gfx::Size(640, 480)));
@@ -87,8 +91,7 @@ class ArcAccessibilityHelperBridgeBrowserTest : public InProcessBrowserTest {
     ret.surface->Commit();
 
     // Forcefully set task_id for each window.
-    exo::SetShellApplicationId(
-        ret.shell_surface->GetWidget()->GetNativeWindow(), std::move(name));
+    ret.surface->SetApplicationId(name.c_str());
     return ret;
   }
 

chrome/browser/ui/BUILD.gn

@@ -1930,6 +1930,8 @@ static_library("ui") {
       "ash/launcher/app_service/app_service_instance_registry_helper.h",
       "ash/launcher/app_service/app_service_shelf_context_menu.cc",
       "ash/launcher/app_service/app_service_shelf_context_menu.h",
+      "ash/launcher/app_service/exo_app_type_resolver.cc",
+      "ash/launcher/app_service/exo_app_type_resolver.h",
       "ash/launcher/app_service/launcher_app_service_app_updater.cc",
       "ash/launcher/app_service/launcher_app_service_app_updater.h",
       "ash/launcher/app_shortcut_launcher_item_controller.cc",
@@ -2631,6 +2633,7 @@ static_library("ui") {
       "//chromeos/components/tether",
       "//chromeos/components/web_applications",
       "//chromeos/constants",
+      "//chromeos/crosapi/cpp",
       "//chromeos/cryptohome",
       "//chromeos/dbus",
       "//chromeos/dbus/audio",

chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc

@@ -33,6 +33,7 @@
 #include "chrome/browser/ui/ash/chrome_new_window_client.h"
 #include "chrome/browser/ui/ash/ime_controller_client.h"
 #include "chrome/browser/ui/ash/in_session_auth_dialog_client.h"
+#include "chrome/browser/ui/ash/launcher/app_service/exo_app_type_resolver.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/login_screen_client.h"
 #include "chrome/browser/ui/ash/media_client_impl.h"
@@ -179,6 +180,10 @@ void ChromeBrowserMainExtraPartsAsh::PreProfileInit() {
 
 #if BUILDFLAG(ENABLE_WAYLAND_SERVER)
   exo_parts_ = ExoParts::CreateIfNecessary();
+  if (exo_parts_) {
+    exo::WMHelper::GetInstance()->RegisterAppPropertyResolver(
+        std::make_unique<ExoAppTypeResolver>());
+  }
 #endif
 
   night_light_client_ = std::make_unique<NightLightClient>(

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

+// Copyright 2020 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/ui/ash/launcher/app_service/exo_app_type_resolver.h"
+
+#include "ash/public/cpp/app_types.h"
+#include "base/strings/string_piece.h"
+#include "chromeos/crosapi/cpp/crosapi_constants.h"
+#include "components/arc/arc_util.h"
+#include "ui/aura/client/aura_constants.h"
+#include "ui/base/class_property.h"
+
+namespace {
+
+// Returns true, if the given ID represents Lacros.
+bool IsLacrosAppId(base::StringPiece app_id) {
+  return base::StartsWith(app_id, crosapi::kLacrosAppIdPrefix);
+}
+
+}  // namespace
+
+void ExoAppTypeResolver::PopulateProperties(
+    const std::string& app_id,
+    const std::string& startup_id,
+    bool for_creation,
+    ui::PropertyHandler& out_properties_container) {
+  if (IsLacrosAppId(app_id)) {
+    out_properties_container.SetProperty(
+        aura::client::kAppType, static_cast<int>(ash::AppType::LACROS));
+  } else if (arc::GetTaskIdFromWindowAppId(app_id) != arc::kNoTaskId) {
+    out_properties_container.SetProperty(
+        aura::client::kAppType, static_cast<int>(ash::AppType::ARC_APP));
+  }
+}

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

+// Copyright 2020 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.
+
+#ifndef CHROME_BROWSER_UI_ASH_LAUNCHER_APP_SERVICE_EXO_APP_TYPE_RESOLVER_H_
+#define CHROME_BROWSER_UI_ASH_LAUNCHER_APP_SERVICE_EXO_APP_TYPE_RESOLVER_H_
+
+#include "components/exo/wm_helper.h"
+
+// This class populates the window property to identify the type of application
+// for exo's toplevel window based on |app_id| and |startup_id|.
+class ExoAppTypeResolver : public exo::WMHelper::AppPropertyResolver {
+ public:
+  ExoAppTypeResolver() = default;
+  ExoAppTypeResolver(const ExoAppTypeResolver&) = delete;
+  ExoAppTypeResolver& operator=(const ExoAppTypeResolver&) = delete;
+  ~ExoAppTypeResolver() override = default;
+
+  // exo::WMHelper::AppPropertyResolver:
+  void PopulateProperties(
+      const std::string& app_id,
+      const std::string& startup_id,
+      bool for_creation,
+      ui::PropertyHandler& out_properties_container) override;
+};
+
+#endif  // CHROME_BROWSER_UI_ASH_LAUNCHER_APP_SERVICE_EXO_APP_TYPE_RESOLVER_H_

components/exo/client_controlled_shell_surface.cc

@@ -1021,9 +1021,6 @@ gfx::Rect ClientControlledShellSurface::GetShadowBounds() const {
 
 void ClientControlledShellSurface::InitializeWindowState(
     ash::WindowState* window_state) {
-  // Set the relevant window properties for Arc apps.
-  SetArcAppType(window_state->window());
-
   // Allow the client to request bounds that do not fill the entire work area
   // when maximized, or the entire display when fullscreen.
   window_state->set_allow_set_bounds_direct(true);

components/exo/fullscreen_shell_surface.cc

@@ -288,7 +288,6 @@ void FullscreenShellSurface::CreateFullscreenShellSurfaceWidget(
   SetShellApplicationId(window, application_id_);
   SetShellStartupId(window, startup_id_);
   SetShellMainSurface(window, root_surface());
-  SetArcAppType(window);
 
   window->AddObserver(this);
 }

components/exo/notification_surface.cc

@@ -22,7 +22,6 @@ NotificationSurface::NotificationSurface(NotificationSurfaceManager* manager,
       notification_key_(notification_key) {
   surface->AddSurfaceObserver(this);
   SetRootSurface(surface);
-  SetArcAppType(host_window());
   host_window()->Show();
   host_window()->AddObserver(this);
 }

components/exo/shell_surface_base.cc

@@ -68,11 +68,6 @@ void SetSkipImeProcessingToDescendentSurfaces(aura::Window* window) {
     SetSkipImeProcessingToDescendentSurfaces(child);
 }
 
-// Returns true, if the given ID represents Lacros.
-bool IsLacrosAppId(base::StringPiece app_id) {
-  return base::StartsWith(app_id, crosapi::kLacrosAppIdPrefix);
-}
-
 // The accelerator keys used to close ShellSurfaces.
 const struct {
   ui::KeyboardCode keycode;
@@ -421,8 +416,11 @@ void ShellSurfaceBase::SetApplicationId(const char* application_id) {
 
   if (widget_ && widget_->GetNativeWindow()) {
     SetShellApplicationId(widget_->GetNativeWindow(), application_id_);
-    if (application_id_.has_value() && IsLacrosAppId(*application_id_))
-      SetLacrosAppType(widget_->GetNativeWindow());
+    ui::PropertyHandler& property_handler = *widget_->GetNativeWindow();
+    WMHelper::GetInstance()->PopulateAppProperties(
+        application_id_ ? *application_id_ : std::string(),
+        startup_id_ ? *startup_id_ : std::string(),
+        /*for_creation=*/false, property_handler);
   }
 }
 
@@ -887,6 +885,7 @@ void ShellSurfaceBase::CreateShellSurfaceWidget(
     activatable_ = false;
     DisableMovement();
   }
+
   views::Widget::InitParams params;
   params.type = emulate_x11_override_redirect
                     ? views::Widget::InitParams::TYPE_MENU
@@ -906,6 +905,12 @@ void ShellSurfaceBase::CreateShellSurfaceWidget(
         WMHelper::GetInstance()->GetRootWindowForNewWindows(), container_);
   }
   params.bounds = gfx::Rect(origin_, gfx::Size());
+
+  WMHelper::GetInstance()->PopulateAppProperties(
+      application_id_ ? *application_id_ : std::string(),
+      startup_id_ ? *startup_id_ : std::string(),
+      /*for_creation=*/true, params.init_properties_container);
+
   bool activatable = activatable_;
   if (container_ == ash::kShellWindowId_SystemModalContainer)
     activatable &= HasHitTestRegion();
@@ -932,8 +937,6 @@ void ShellSurfaceBase::CreateShellSurfaceWidget(
       aura::EventTargetingPolicy::kTargetAndDescendants);
   InstallCustomWindowTargeter();
   SetShellApplicationId(window, application_id_);
-  if (application_id_.has_value() && IsLacrosAppId(*application_id_))
-    SetLacrosAppType(window);
   SetShellStartupId(window, startup_id_);
   SetShellMainSurface(window, root_surface());
 

components/exo/shell_surface_unittest.cc

@@ -8,6 +8,7 @@
 #include "ash/frame/non_client_frame_view_ash.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/cpp/test/shell_test_api.h"
+#include "ash/public/cpp/window_properties.h"
 #include "ash/shell.h"
 #include "ash/wm/window_state.h"
 #include "ash/wm/wm_event.h"
@@ -1194,4 +1195,86 @@ TEST_F(ShellSurfaceTest, NotifyLeaveEnter) {
   EXPECT_EQ(secondary_id, old_display_id);
 }
 
+TEST_F(ShellSurfaceTest, PropertyResolverTest) {
+  class TestPropertyResolver : public exo::WMHelper::AppPropertyResolver {
+   public:
+    TestPropertyResolver() = default;
+    ~TestPropertyResolver() override = default;
+    void PopulateProperties(
+        const std::string& app_id,
+        const std::string& startup_id,
+        bool for_creation,
+        ui::PropertyHandler& out_properties_container) override {
+      if (expected_app_id == app_id) {
+        out_properties_container.AcquireAllPropertiesFrom(
+            std::move(for_creation ? properties_for_creation
+                                   : properties_after_creation));
+      }
+    }
+    std::string expected_app_id;
+    ui::PropertyHandler properties_for_creation;
+    ui::PropertyHandler properties_after_creation;
+  };
+  std::unique_ptr<TestPropertyResolver> resolver_holder =
+      std::make_unique<TestPropertyResolver>();
+  auto* resolver = resolver_holder.get();
+  WMHelper::GetInstance()->RegisterAppPropertyResolver(
+      std::move(resolver_holder));
+
+  resolver->properties_for_creation.SetProperty(ash::kShelfItemTypeKey, 1);
+  resolver->properties_after_creation.SetProperty(ash::kShelfItemTypeKey, 2);
+  resolver->expected_app_id = "test";
+
+  // Make sure that properties are properly populated for both
+  // "before widget creation", and "after widget creation".
+  {
+    // TODO(oshima): create a test API to create a shell surface.
+    gfx::Size buffer_size(256, 256);
+    auto buffer = std::make_unique<Buffer>(
+        exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+    auto surface = std::make_unique<Surface>();
+    auto shell_surface = std::make_unique<ShellSurface>(surface.get());
+
+    surface->SetApplicationId("test");
+    surface->Attach(buffer.get());
+    surface->Commit();
+    EXPECT_EQ(1, shell_surface->GetWidget()->GetNativeWindow()->GetProperty(
+                     ash::kShelfItemTypeKey));
+    surface->SetApplicationId("test");
+    EXPECT_EQ(2, shell_surface->GetWidget()->GetNativeWindow()->GetProperty(
+                     ash::kShelfItemTypeKey));
+  }
+
+  // Make sure that properties are will not be popluated when the app ids
+  // mismatch "before" and "after" widget is created.
+  resolver->properties_for_creation.SetProperty(ash::kShelfItemTypeKey, 1);
+  resolver->properties_after_creation.SetProperty(ash::kShelfItemTypeKey, 2);
+  {
+    gfx::Size buffer_size(256, 256);
+    auto buffer = std::make_unique<Buffer>(
+        exo_test_helper()->CreateGpuMemoryBuffer(buffer_size));
+    auto surface = std::make_unique<Surface>();
+    auto shell_surface = std::make_unique<ShellSurface>(surface.get());
+
+    surface->SetApplicationId("testx");
+    surface->Attach(buffer.get());
+    surface->Commit();
+    EXPECT_NE(1, shell_surface->GetWidget()->GetNativeWindow()->GetProperty(
+                     ash::kShelfItemTypeKey));
+
+    surface->SetApplicationId("testy");
+    surface->Attach(buffer.get());
+    surface->Commit();
+    EXPECT_NE(1, shell_surface->GetWidget()->GetNativeWindow()->GetProperty(
+                     ash::kShelfItemTypeKey));
+    EXPECT_NE(2, shell_surface->GetWidget()->GetNativeWindow()->GetProperty(
+                     ash::kShelfItemTypeKey));
+
+    // Updating to the matching |app_id| should set the window property.
+    surface->SetApplicationId("test");
+    EXPECT_EQ(2, shell_surface->GetWidget()->GetNativeWindow()->GetProperty(
+                     ash::kShelfItemTypeKey));
+  }
+}
+
 }  // namespace exo

components/exo/shell_surface_util.cc

@@ -6,14 +6,12 @@
 
 #include <memory>
 
-#include "ash/public/cpp/app_types.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "components/exo/permission.h"
 #include "components/exo/shell_surface_base.h"
 #include "components/exo/surface.h"
 #include "components/exo/wm_helper.h"
-#include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/capture_client.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_delegate.h"
@@ -92,16 +90,6 @@ const std::string* GetShellApplicationId(const aura::Window* window) {
   return window->GetProperty(kApplicationIdKey);
 }
 
-void SetArcAppType(aura::Window* window) {
-  window->SetProperty(aura::client::kAppType,
-                      static_cast<int>(ash::AppType::ARC_APP));
-}
-
-void SetLacrosAppType(aura::Window* window) {
-  window->SetProperty(aura::client::kAppType,
-                      static_cast<int>(ash::AppType::LACROS));
-}
-
 void SetShellStartupId(aura::Window* window,
                        const base::Optional<std::string>& id) {
   TRACE_EVENT1("exo", "SetStartupId", "startup_id", id ? *id : "null");

components/exo/wm_helper.cc

@@ -48,4 +48,20 @@ bool WMHelper::HasInstance() {
   return !!g_instance;
 }
 
+void WMHelper::RegisterAppPropertyResolver(
+    std::unique_ptr<AppPropertyResolver> resolver) {
+  resolver_list_.push_back(std::move(resolver));
+}
+
+void WMHelper::PopulateAppProperties(
+    const std::string& app_id,
+    const std::string& startup_id,
+    bool for_creation,
+    ui::PropertyHandler& out_properties_container) {
+  for (auto& resolver : resolver_list_) {
+    resolver->PopulateProperties(app_id, startup_id, for_creation,
+                                 out_properties_container);
+  }
+}
+
 }  // namespace exo

components/exo/wm_helper.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_EXO_WM_HELPER_H_
 
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "base/macros.h"
@@ -35,6 +36,7 @@ class ManagedDisplayMode;
 namespace ui {
 class EventHandler;
 class DropTargetEvent;
+class PropertyHandler;
 }  // namespace ui
 
 namespace wm {
@@ -82,6 +84,18 @@ class WMHelper : public aura::client::DragDropDelegate {
     DISALLOW_COPY_AND_ASSIGN(LifetimeManager);
   };
 
+  // Used to resolve the properties to be set to the window
+  // based on the |app_id| and |startup_id|.
+  class AppPropertyResolver {
+   public:
+    virtual ~AppPropertyResolver() = default;
+    virtual void PopulateProperties(
+        const std::string& app_id,
+        const std::string& startup_id,
+        bool for_creation,
+        ui::PropertyHandler& out_properties_container) = 0;
+  };
+
   WMHelper();
   ~WMHelper() override;
 
@@ -138,7 +152,23 @@ class WMHelper : public aura::client::DragDropDelegate {
   int OnPerformDrop(const ui::DropTargetEvent& event,
                     std::unique_ptr<ui::OSExchangeData> data) override = 0;
 
+  // Registers an AppPropertyResolver. Multiple resolver can be registered and
+  // all resolvers are called in the registration order by the method below.
+  void RegisterAppPropertyResolver(
+      std::unique_ptr<AppPropertyResolver> resolver);
+
+  // Populates window properties for given |app_id| and |startup_id|.
+  // |for_creation| == true means this is called before a widget gets
+  // created, and false means this is called when the application id is set
+  // after the widget is created.
+  void PopulateAppProperties(const std::string& app_id,
+                             const std::string& startup_id,
+                             bool for_creation,
+                             ui::PropertyHandler& out_properties_container);
+
  protected:
+  std::vector<std::unique_ptr<AppPropertyResolver>> resolver_list_;
+
   DISALLOW_COPY_AND_ASSIGN(WMHelper);
 };