Skip the Default profile when reading sync settings.

The Default profile on Chrome OS is used before the user signs in.

The sync settings are always disabled on the Default profile.

Bug: b:175067939
Change-Id: I67c8f341f9ea5ea0b0d08261dc0d5b6fdf4814aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587658
Commit-Queue: Bin Du <dubin@google.com>
Reviewed-by: Gabriel Marin <gmx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837227}

chrome/browser/metrics/perf/metric_provider.cc

@@ -9,6 +9,7 @@
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
@@ -219,14 +220,24 @@ MetricProvider::RecordAttemptStatus MetricProvider::GetAppSyncState() {
     return RecordAttemptStatus::kProfileManagerUnset;
 
   std::vector<Profile*> profiles = profile_manager_->GetLoadedProfiles();
-  if (profiles.size() == 0)
-    return RecordAttemptStatus::kNoLoadedProfile;
+  // Tracks the number of user profiles initialized on Chrome OS other than the
+  // Default profile.
+  int user_profile_count = 0;
 
   for (Profile* profile : profiles) {
+    // The Default profile on Chrome OS is used when the user has not logged in
+    // and this profile always disables sync. We skip this profile and look at
+    // sync settings of user profiles.
+    if (chromeos::ProfileHelper::IsSigninProfile(profile))
+      continue;
     if (!IsAppSyncEnabledForUserProfile(profile))
       return RecordAttemptStatus::kAppSyncDisabled;
+    user_profile_count++;
   }
 
+  if (user_profile_count == 0)
+    return RecordAttemptStatus::kNoLoadedProfile;
+
   return RecordAttemptStatus::kAppSyncEnabled;
 }
 

chrome/browser/metrics/perf/metric_provider_unittest.cc

@@ -17,6 +17,7 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
@@ -306,6 +307,11 @@ class MetricProviderSyncSettingsTest : public testing::Test {
     testing_profile_manager_ = std::make_unique<TestingProfileManager>(
         TestingBrowserProcess::GetGlobal());
     ASSERT_TRUE(testing_profile_manager_->SetUp());
+    // A Default profile is always loaded before all other user profiles are
+    // initialized on Chrome OS. So creating the Default profile here to reflect
+    // this. The Default profile is skipped when getting the sync settings from
+    // user profile(s).
+    testing_profile_manager_->CreateTestingProfile(chrome::kInitialProfile);
     metric_provider_ = std::make_unique<TestMetricProvider>(
         std::make_unique<TestMetricCollector>(test_params),
         testing_profile_manager_->profile_manager());
@@ -372,6 +378,9 @@ class MetricProviderSyncSettingsTest : public testing::Test {
 };
 
 TEST_F(MetricProviderSyncSettingsTest, NoLoadedUserProfile) {
+  // The Default profile is skipped and there is no other user profile
+  // initialized. So we would expect the perf data to be redacted and a
+  // histogram count of kNoLoadedProfile.
   base::HistogramTester histogram_tester;
   std::vector<SampledProfile> stored_profiles;
   metric_provider_->OnUserLoggedIn();
@@ -396,7 +405,8 @@ TEST_F(MetricProviderSyncSettingsTest, SplitSettingsAppSyncEnabled) {
   metric_provider_->OnUserLoggedIn();
   feature_list_.InitAndEnableFeature(chromeos::features::kSplitSettingsSync);
 
-  // Set up two testing profiles, both with OS App Sync enabled.
+  // Set up two testing profiles, both with OS App Sync enabled. The Default
+  // profile has OS App Sync disabled but is skipped.
   TestSyncService* sync_service1 =
       GetSyncService(testing_profile_manager_->CreateTestingProfile("user1"));
   TestSyncService* sync_service2 =
@@ -453,7 +463,8 @@ TEST_F(MetricProviderSyncSettingsTest, UnifiedSettingsAppSyncEnabled) {
   metric_provider_->OnUserLoggedIn();
   feature_list_.InitAndDisableFeature(chromeos::features::kSplitSettingsSync);
 
-  // Set up two testing profiles, both with App Sync enabled.
+  // Set up two testing profiles, both with App Sync enabled. The Default
+  // profile has App Sync disabled but is skipped.
   TestSyncService* sync_service1 =
       GetSyncService(testing_profile_manager_->CreateTestingProfile("user1"));
   TestSyncService* sync_service2 =

tools/metrics/histograms/enums.xml

@@ -9989,7 +9989,8 @@ histogram as enum -->
     profile(s) is unset.
   </int>
   <int value="2" label="No loaded user profile is found">
-    Profile data was redacted since there is no user profile initialized.
+    Profile data was redacted since there is no user profile initialized. When
+    looking at user profile(s), the Default profile in Chrome OS is skipped.
   </int>
   <int value="3" label="App sync is disabled">
     Profile data was redacted since App Sync is disabled.