Lacros: Allow Perfetto for ash in LO mode for dev debugging.

This cl implements the following:
1. Allow perfetto extension to run in ash in LO mode if ash
debug switch is enabled.
2. Refactor the code so that the profile migrator will not
maintain its own allowlist for extensions/apps running in both
ash and lacros, instead, it will use the allowlist maintained
by ash extension keeplist. This helps to avoid the potential
inconsistency by maintaining two allowlists in different
components.

Bug: 1472954
Change-Id: I832e24a84c04fa1341b2ab7f2becbcf6c71a0ff4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4795108
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Yuta Hijikata <ythjkt@chromium.org>
Commit-Queue: Jenny Zhang <jennyz@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1186645}

chrome/browser/ash/crosapi/browser_data_back_migrator.cc

@@ -30,6 +30,7 @@
 #include "chrome/browser/ash/crosapi/browser_data_back_migrator_metrics.h"
 #include "chrome/browser/ash/crosapi/browser_data_migrator_util.h"
 #include "chrome/browser/ash/profiles/profile_helper.h"
+#include "chrome/browser/extensions/extension_keeplist_chromeos.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
@@ -253,10 +254,10 @@ BrowserDataBackMigrator::TaskResult BrowserDataBackMigrator::MergeSplitItems(
   }
 
   // Merge IndexedDB.
-  for (const char* extension_id :
-       browser_data_migrator_util::kExtensionsBothChromes) {
+  for (const auto& extension_id :
+       extensions::GetExtensionsAndAppsRunInOSAndStandaloneBrowser()) {
     if (!MergeCommonIndexedDB(ash_profile_dir, lacros_default_profile_dir,
-                              extension_id)) {
+                              extension_id.data())) {
       return {TaskStatus::kMergeSplitItemsMergeIndexedDBFailed, errno};
     }
   }
@@ -749,8 +750,8 @@ bool BrowserDataBackMigrator::MergeCommonExtensionsDataFiles(
       return false;
     }
 
-    for (const char* extension_id :
-         browser_data_migrator_util::kExtensionsBothChromes) {
+    for (const auto& extension_id :
+         extensions::GetExtensionsAndAppsRunInOSAndStandaloneBrowser()) {
       base::FilePath lacros_target_path =
           lacros_target_dir.Append(extension_id);
 
@@ -779,8 +780,8 @@ bool BrowserDataBackMigrator::RemoveAshCommonExtensionsDataFiles(
   const base::FilePath ash_target_dir = ash_profile_dir.Append(target_dir);
 
   if (base::PathExists(ash_target_dir)) {
-    for (const char* extension_id :
-         browser_data_migrator_util::kExtensionsBothChromes) {
+    for (const auto& extension_id :
+         extensions::GetExtensionsAndAppsRunInOSAndStandaloneBrowser()) {
       base::FilePath ash_target_path = ash_target_dir.Append(extension_id);
 
       if (!base::DeletePathRecursively(ash_target_path)) {
@@ -1023,8 +1024,9 @@ bool BrowserDataBackMigrator::IsLacrosOnlyExtension(
     const base::StringPiece extension_id) {
   return !base::Contains(browser_data_migrator_util::kExtensionsAshOnly,
                          extension_id) &&
-         !base::Contains(browser_data_migrator_util::kExtensionsBothChromes,
-                         extension_id);
+         !base::Contains(
+             extensions::GetExtensionsAndAppsRunInOSAndStandaloneBrowser(),
+             extension_id);
 }
 
 // static

chrome/browser/ash/crosapi/browser_data_back_migrator_unittest.cc

@@ -20,6 +20,7 @@
 #include "chrome/browser/ash/crosapi/browser_data_back_migrator_metrics.h"
 #include "chrome/browser/ash/crosapi/browser_data_migrator_util.h"
 #include "chrome/browser/ash/crosapi/browser_util.h"
+#include "chrome/browser/extensions/extension_keeplist_chromeos.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/policy_constants.h"
 #include "components/prefs/testing_pref_service.h"
@@ -44,8 +45,15 @@ constexpr size_t kLacrosDataSize = sizeof(kLacrosDataContent);
 // so we can be sure that it won't be included in `kExtensionsAshOnly`.
 constexpr char kLacrosOnlyExtensionId[] = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
 
-const char* kBothExtensionId =
-    browser_data_migrator_util::kExtensionsBothChromes[0];
+// ID of an extension that runs in both Lacros and Ash chrome.
+std::string_view GetBothChromesExtensionId() {
+  // Any id from the Ash allowlist works for tests. Pick the first
+  // element of the allowlist for convenience.
+  DCHECK(
+      !extensions::GetExtensionsAndAppsRunInOSAndStandaloneBrowser().empty());
+  return extensions::GetExtensionsAndAppsRunInOSAndStandaloneBrowser()[0];
+}
+
 const char* kAshOnlyExtensionId =
     browser_data_migrator_util::kExtensionsAshOnly[0];
 
@@ -117,18 +125,19 @@ void SetUpExtensions(const base::FilePath& ash_profile_dir,
         lacros_extensions_path.Append(kLacrosOnlyExtensionId),
         kLacrosDataFilePath, kLacrosDataContent, kLacrosDataSize);
     // Generate Lacros data for an extension existing in both Chromes.
-    CreateDirectoryAndFile(lacros_extensions_path.Append(kBothExtensionId),
-                           kLacrosDataFilePath, kLacrosDataContent,
-                           kLacrosDataSize);
+    CreateDirectoryAndFile(
+        lacros_extensions_path.Append(GetBothChromesExtensionId()),
+        kLacrosDataFilePath, kLacrosDataContent, kLacrosDataSize);
   }
 
   if (setup != FilesSetup::kLacrosOnly) {
     // Generate data for an Ash-only extension.
     CreateDirectoryAndFile(ash_extensions_path.Append(kAshOnlyExtensionId),
                            kAshDataFilePath, kAshDataContent, kAshDataSize);
     // Generate Ash data for an extension existing in both Chromes.
-    CreateDirectoryAndFile(ash_extensions_path.Append(kBothExtensionId),
-                           kAshDataFilePath, kAshDataContent, kAshDataSize);
+    CreateDirectoryAndFile(
+        ash_extensions_path.Append(GetBothChromesExtensionId()),
+        kAshDataFilePath, kAshDataContent, kAshDataSize);
   }
 }
 
@@ -184,7 +193,7 @@ void SetUpIndexedDB(const base::FilePath& ash_profile_dir,
   if (setup != FilesSetup::kAshOnly) {
     const auto& [lacros_blob_path, lacros_leveldb_path] =
         browser_data_migrator_util::GetIndexedDBPaths(
-            lacros_default_profile_dir, kBothExtensionId);
+            lacros_default_profile_dir, GetBothChromesExtensionId().data());
 
     CreateDirectoryAndFile(lacros_blob_path, kLacrosDataFilePath,
                            kLacrosDataContent, kLacrosDataSize);
@@ -194,8 +203,8 @@ void SetUpIndexedDB(const base::FilePath& ash_profile_dir,
 
   if (setup != FilesSetup::kLacrosOnly) {
     const auto& [ash_blob_path, ash_leveldb_path] =
-        browser_data_migrator_util::GetIndexedDBPaths(ash_profile_dir,
-                                                      kBothExtensionId);
+        browser_data_migrator_util::GetIndexedDBPaths(
+            ash_profile_dir, GetBothChromesExtensionId().data());
     CreateDirectoryAndFile(ash_blob_path, kAshDataFilePath, kAshDataContent,
                            kAshDataSize);
     CreateDirectoryAndFile(ash_leveldb_path, kAshDataFilePath, kAshDataContent,
@@ -297,13 +306,15 @@ class BrowserDataBackMigratorTest : public testing::Test {
     kLacrosOnlyMetaKey = kMetaPrefix + std::string(kLacrosOnlyExtensionId);
     kLacrosOnlyValueKey =
         kKeyPrefix + std::string(kLacrosOnlyExtensionId) + "\x00key"s;
-    kBothChromesMetaKey = kMetaPrefix + std::string(kBothExtensionId);
+    kBothChromesMetaKey =
+        kMetaPrefix + std::string(GetBothChromesExtensionId());
     kBothChromesValueKey =
-        kKeyPrefix + std::string(kBothExtensionId) + "\x00key"s;
+        kKeyPrefix + std::string(GetBothChromesExtensionId()) + "\x00key"s;
 
     kAshOnlyStateStoreKey = std::string(kAshOnlyExtensionId) + ".key";
     kLacrosOnlyStateStoreKey = std::string(kLacrosOnlyExtensionId) + ".key";
-    kBothChromesStateStoreKey = std::string(kBothExtensionId) + ".key";
+    kBothChromesStateStoreKey =
+        std::string(GetBothChromesExtensionId()) + ".key";
   }
 
   void SetUp() override {
@@ -533,12 +544,14 @@ TEST_F(BrowserDataBackMigratorTest, MergeCommonExtensionsDataFiles) {
                                     .Append(kAshDataFilePath)));
 
   // The Ash version of the both-Chromes extension does not exist.
-  ASSERT_FALSE(base::PathExists(
-      tmp_extensions_path.Append(kBothExtensionId).Append(kAshDataFilePath)));
+  ASSERT_FALSE(
+      base::PathExists(tmp_extensions_path.Append(GetBothChromesExtensionId())
+                           .Append(kAshDataFilePath)));
 
   // The Lacros version of the both-Chromes extension exists.
   base::FilePath lacros_tmp_file_path =
-      tmp_extensions_path.Append(kBothExtensionId).Append(kLacrosDataFilePath);
+      tmp_extensions_path.Append(GetBothChromesExtensionId())
+          .Append(kLacrosDataFilePath);
   ASSERT_TRUE(base::PathExists(lacros_tmp_file_path));
 
   // The contents of the file in the temporary directory are the same as the
@@ -561,8 +574,7 @@ TEST_P(BrowserDataBackMigratorFilesSetupTest, MergeCommonIndexedDB) {
   auto files_setup = GetParam();
   SetUpIndexedDB(ash_profile_dir_, lacros_default_profile_dir_, files_setup);
 
-  const char* extension_id =
-      browser_data_migrator_util::kExtensionsBothChromes[0];
+  const char* extension_id = GetBothChromesExtensionId().data();
 
   ASSERT_TRUE(BrowserDataBackMigrator::MergeCommonIndexedDB(
       ash_profile_dir_, lacros_default_profile_dir_, extension_id));
@@ -773,8 +785,8 @@ TEST_F(BrowserDataBackMigratorTest,
   base::Value::Dict ash_split_pref_dict;
   ash_split_pref_dict.SetByDottedPath(
       browser_data_migrator_util::kExtensionsAshOnly[0], kAshPrefValue);
-  ash_split_pref_dict.SetByDottedPath(
-      browser_data_migrator_util::kExtensionsBothChromes[0], kAshPrefValue);
+  ash_split_pref_dict.SetByDottedPath(GetBothChromesExtensionId(),
+                                      kAshPrefValue);
   ash_split_pref_dict.SetByDottedPath(kLacrosOnlyExtensionId, kAshPrefValue);
   ash_prefs.SetByDottedPath(
       browser_data_migrator_util::kSplitPreferencesKeys[0],
@@ -784,8 +796,8 @@ TEST_F(BrowserDataBackMigratorTest,
   base::Value::Dict lacros_split_pref_dict;
   lacros_split_pref_dict.SetByDottedPath(
       browser_data_migrator_util::kExtensionsAshOnly[0], kLacrosPrefValue);
-  lacros_split_pref_dict.SetByDottedPath(
-      browser_data_migrator_util::kExtensionsBothChromes[0], kLacrosPrefValue);
+  lacros_split_pref_dict.SetByDottedPath(GetBothChromesExtensionId(),
+                                         kLacrosPrefValue);
   lacros_split_pref_dict.SetByDottedPath(kLacrosOnlyExtensionId,
                                          kLacrosPrefValue);
   lacros_prefs.SetByDottedPath(
@@ -818,8 +830,8 @@ TEST_F(BrowserDataBackMigratorTest,
       browser_data_migrator_util::kExtensionsAshOnly[0]);
   ASSERT_TRUE(ash_extension_value);
   ASSERT_EQ(ash_extension_value->GetInt(), kAshPrefValue);
-  const base::Value* common_extension_value = split_pref_dict->FindByDottedPath(
-      browser_data_migrator_util::kExtensionsBothChromes[0]);
+  const base::Value* common_extension_value =
+      split_pref_dict->FindByDottedPath(GetBothChromesExtensionId());
   ASSERT_TRUE(common_extension_value);
   ASSERT_EQ(common_extension_value->GetInt(), kAshPrefValue);
   const base::Value* lacros_extension_value =
@@ -850,8 +862,7 @@ TEST_F(BrowserDataBackMigratorTest,
   base::Value::Dict ash_prefs;
   base::Value::List ash_split_pref_list;
   ash_split_pref_list.Append(browser_data_migrator_util::kExtensionsAshOnly[0]);
-  ash_split_pref_list.Append(
-      browser_data_migrator_util::kExtensionsBothChromes[0]);
+  ash_split_pref_list.Append(GetBothChromesExtensionId());
   ash_split_pref_list.Append(kLacrosOnlyExtensionId);
   ash_prefs.SetByDottedPath(
       browser_data_migrator_util::kSplitPreferencesKeys[0],
@@ -890,10 +901,9 @@ TEST_F(BrowserDataBackMigratorTest,
       CountStringInList(*split_pref_list,
                         browser_data_migrator_util::kExtensionsAshOnly[0]),
       1u);
-  ASSERT_EQ(
-      CountStringInList(*split_pref_list,
-                        browser_data_migrator_util::kExtensionsBothChromes[0]),
-      1u);
+  ASSERT_EQ(CountStringInList(*split_pref_list,
+                              std::string(GetBothChromesExtensionId())),
+            1u);
   ASSERT_EQ(CountStringInList(*split_pref_list, kLacrosOnlyExtensionId), 1u);
 }
 

chrome/browser/ash/crosapi/browser_data_migrator_util.cc

@@ -20,6 +20,7 @@
 #include "base/strings/string_util.h"
 #include "base/system/sys_info.h"
 #include "base/values.h"
+#include "chrome/browser/extensions/extension_keeplist_chromeos.h"
 #include "chrome/common/chrome_constants.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/base/storage_type.h"
@@ -154,7 +155,9 @@ bool ShouldRemoveExtensionByType(const base::StringPiece extension_id,
   switch (chrome_type) {
     case ChromeType::kAsh:
       return !base::Contains(kExtensionsAshOnly, extension_id) &&
-             !base::Contains(kExtensionsBothChromes, extension_id);
+             !base::Contains(
+                 extensions::GetExtensionsAndAppsRunInOSAndStandaloneBrowser(),
+                 extension_id);
 
     case ChromeType::kLacros:
       return base::Contains(kExtensionsAshOnly, extension_id);
@@ -650,7 +653,9 @@ bool MigrateLevelDB(const base::FilePath& original_path,
   // Copy all the key-value pairs that need to be kept in Ash.
   for (const auto& [extension_id, keys] : original_keys) {
     if (base::Contains(kExtensionsAshOnly, extension_id) ||
-        base::Contains(kExtensionsBothChromes, extension_id)) {
+        base::Contains(
+            extensions::GetExtensionsAndAppsRunInOSAndStandaloneBrowser(),
+            extension_id)) {
       for (const std::string& key : keys) {
         std::string value;
         status = original_db->Get(leveldb::ReadOptions(), key, &value);

chrome/browser/ash/crosapi/browser_data_migrator_util.h

@@ -210,13 +210,6 @@ constexpr const char* const kExtensionsAshOnly[] = {
     "cnbgggchhmkkdmeppjobngjoejnihlei",  // Arc Support (Play Store)
 };
 
-// List of extension ids to be kept in both Ash and Lacros.
-constexpr const char* const kExtensionsBothChromes[] = {
-    "cfmgaohenjcikllcgjpepfadgbflcjof",  // GCSE (Google Corp SSH Extension)
-    "lfboplenmmjcmpbkeemecobbadnmpfhi",  // gnubbyd-v3 (new Gnubby extension)
-    "beknehfpfkghjoafdifaflglpjkojoco",  // gnubbyd
-};
-
 // Extensions path.
 constexpr char kExtensionsFilePath[] = "Extensions";