saved_desks: Fix DCHECK when closing ARC window in overview

We have code in OverviewGrid to determine if there are windows that the
saved desk features do not support. This code is run when a window is
added to overview mode, and when it is removed. The issue here is a race
between ARC task and aura window destruction.

We call out to full_restore::GetAppId to get the application ID of a
window. In some cases when an ARC window is being destroyed, this
function will return an empty application ID. The fix here, which is
somewhat temporary, is to also check the window's kAppIDKey property.

BUG=b:270429246

Change-Id: I915e0b0917b0f551dab53e81f8d25425bdbfedd6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4628703
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Daniel Andersson <dandersson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1160353}

ash/wm/desks/templates/restore_data_collector.cc

@@ -13,7 +13,6 @@
 #include "ash/wm/window_restore/window_restore_util.h"
 #include "base/uuid.h"
 #include "components/app_restore/app_launch_info.h"
-#include "components/app_restore/full_restore_utils.h"
 #include "components/app_restore/restore_data.h"
 #include "components/app_restore/window_info.h"
 #include "ui/wm/core/window_util.h"
@@ -65,7 +64,7 @@ void RestoreDataCollector::CaptureActiveDeskAsSavedDesk(
     }
 
     // Skip windows that do not associate with a full restore app id.
-    const std::string app_id = full_restore::GetAppId(window);
+    const std::string app_id = saved_desk_util::GetAppId(window);
     if (!Shell::Get()
              ->overview_controller()
              ->disable_app_id_check_for_saved_desks() &&

ash/wm/desks/templates/saved_desk_unittest.cc

@@ -68,7 +68,6 @@
 #include "cc/test/pixel_comparator.h"
 #include "components/app_constants/constants.h"
 #include "components/app_restore/app_launch_info.h"
-#include "components/app_restore/full_restore_utils.h"
 #include "components/app_restore/window_info.h"
 #include "components/app_restore/window_properties.h"
 #include "components/desks_storage/core/local_desk_data_manager.h"
@@ -2127,7 +2126,7 @@ TEST_F(SavedDeskTest, AllUnsupportedAppsDisablesSaveDeskButtons) {
   auto no_app_id_window = CreateAppWindow();
   auto* delegate = Shell::Get()->saved_desk_delegate();
   ASSERT_TRUE(delegate->IsWindowSupportedForSavedDesk(no_app_id_window.get()));
-  ASSERT_TRUE(full_restore::GetAppId(no_app_id_window.get()).empty());
+  ASSERT_TRUE(saved_desk_util::GetAppId(no_app_id_window.get()).empty());
 
   // Open overview.
   ToggleOverview();

ash/wm/desks/templates/saved_desk_util.cc

@@ -7,11 +7,13 @@
 #include "ash/constants/ash_features.h"
 #include "ash/constants/ash_pref_names.h"
 #include "ash/public/cpp/session/session_types.h"
+#include "ash/public/cpp/window_properties.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "ash/wm/desks/templates/saved_desk_constants.h"
 #include "ash/wm/overview/overview_controller.h"
 #include "ash/wm/overview/overview_session.h"
+#include "components/app_restore/full_restore_utils.h"
 #include "components/app_restore/window_properties.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
@@ -84,6 +86,23 @@ SavedDeskPresenter* GetSavedDeskPresenter() {
   return presenter;
 }
 
+std::string GetAppId(aura::Window* window) {
+  // Check with full restore first.
+  if (std::string app_id = full_restore::GetAppId(window); !app_id.empty()) {
+    return app_id;
+  }
+
+  // Otherwise, check if there's a window property.
+  // TODO(b/288153585): Figure out if the kAppIDKey property is always going to
+  // be set for window types that we care about. If that is the case, we should
+  // be able to drop the call to full restore.
+  if (const std::string* app_id_property = window->GetProperty(kAppIDKey)) {
+    return *app_id_property;
+  }
+
+  return "";
+}
+
 bool IsAdminTemplateWindow(aura::Window* window) {
   const int32_t* activation_index =
       window->GetProperty(app_restore::kActivationIndexKey);

ash/wm/desks/templates/saved_desk_util.h

@@ -5,6 +5,8 @@
 #ifndef ASH_WM_DESKS_TEMPLATES_SAVED_DESK_UTIL_H_
 #define ASH_WM_DESKS_TEMPLATES_SAVED_DESK_UTIL_H_
 
+#include <string>
+
 #include "ash/ash_export.h"
 
 class PrefRegistrySimple;
@@ -34,6 +36,10 @@ ASH_EXPORT SavedDeskDialogController* GetSavedDeskDialogController();
 // Will return null if overview mode is not active.
 ASH_EXPORT SavedDeskPresenter* GetSavedDeskPresenter();
 
+// Returns the app ID of the window, if present. Returns an empty string
+// otherwise.
+ASH_EXPORT std::string GetAppId(aura::Window* window);
+
 // Returns true if `window` was launched from an admin template.
 bool IsAdminTemplateWindow(aura::Window* window);
 

ash/wm/overview/overview_grid.cc

@@ -66,7 +66,6 @@
 #include "base/ranges/algorithm.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "chromeos/ui/base/window_properties.h"
-#include "components/app_restore/full_restore_utils.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/chromeos/styles/cros_tokens_color_mappings.h"
@@ -2729,7 +2728,7 @@ void OverviewGrid::UpdateNumSavedDeskUnsupportedWindows(aura::Window* window,
                               (Shell::Get()
                                    ->overview_controller()
                                    ->disable_app_id_check_for_saved_desks() ||
-                               !full_restore::GetAppId(window).empty());
+                               !saved_desk_util::GetAppId(window).empty());
   int addend = increment ? 1 : -1;
   if (!DeskTemplate::IsAppTypeSupported(window) || !has_restore_id) {
     num_unsupported_windows_ += addend;