[Metrics] Revert FamilyUserMetricsProvider

FamilyUserMetricsProvider causes an OOBE crash from dereferencing a
null sign-in profile. Revert this code temporarily while working on a
fix.

Bug: 1128281
Change-Id: If13d6fc4a0c222053e8beae3d306e9aa9fae2340
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427426
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810235}

chrome/browser/BUILD.gn

@@ -4102,8 +4102,6 @@ static_library("browser") {
       "metrics/chromeos_metrics_provider.h",
       "metrics/cros_healthd_metrics_provider.cc",
       "metrics/cros_healthd_metrics_provider.h",
-      "metrics/family_user_metrics_provider.cc",
-      "metrics/family_user_metrics_provider.h",
       "metrics/perf/collection_params.cc",
       "metrics/perf/collection_params.h",
       "metrics/perf/cpu_identity.cc",

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -150,7 +150,6 @@
 #include "chrome/browser/metrics/assistant_service_metrics_provider.h"
 #include "chrome/browser/metrics/chromeos_metrics_provider.h"
 #include "chrome/browser/metrics/cros_healthd_metrics_provider.h"
-#include "chrome/browser/metrics/family_user_metrics_provider.h"
 #include "chrome/browser/signin/signin_status_metrics_provider_chromeos.h"
 #include "components/metrics/structured/structured_metrics_provider.h"
 #endif  // defined(OS_CHROMEOS)
@@ -734,9 +733,6 @@ void ChromeMetricsServiceClient::RegisterMetricsServiceProviders() {
 
   metrics_service_->RegisterMetricsProvider(
       std::make_unique<AmbientModeMetricsProvider>());
-
-  metrics_service_->RegisterMetricsProvider(
-      std::make_unique<FamilyUserMetricsProvider>());
 #endif  // defined(OS_CHROMEOS)
 
 #if !defined(OS_CHROMEOS)

chrome/browser/metrics/chrome_metrics_service_client_unittest.cc

@@ -182,9 +182,9 @@ TEST_F(ChromeMetricsServiceClientTest, TestRegisterMetricsServiceProviders) {
 #if defined(OS_CHROMEOS)
   // AmbientModeMetricsProvider, AssistantServiceMetricsProvider,
   // CrosHealthdMetricsProvider, ChromeOSMetricsProvider,
-  // SigninStatusMetricsProviderChromeOS, PrinterMetricsProvider,
-  // HashedLoggingMetricsProvider, and FamilyUserMetricsProvider.
-  expected_providers += 8;
+  // SigninStatusMetricsProviderChromeOS, PrinterMetricsProvider, and
+  // HashedLoggingMetricsProvider.
+  expected_providers += 7;
 #endif  // defined(OS_CHROMEOS)
 
 #if !defined(OS_CHROMEOS)

chrome/browser/metrics/family_user_metrics_provider.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/metrics/family_user_metrics_provider.h"
-
-#include "base/bind.h"
-#include "base/logging.h"
-#include "base/metrics/histogram_functions.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/browser_process_platform_part.h"
-#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
-#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h"
-#include "chrome/browser/policy/profile_policy_connector.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chromeos/components/account_manager/account_manager_factory.h"
-
-// static
-const char FamilyUserMetricsProvider::kFamilyUserLogSegmentHistogramName[] =
-    "ChromeOS.FamilyUser.LogSegment";
-
-FamilyUserMetricsProvider::FamilyUserMetricsProvider() = default;
-
-FamilyUserMetricsProvider::~FamilyUserMetricsProvider() = default;
-
-void FamilyUserMetricsProvider::ProvideCurrentSessionData(
-    metrics::ChromeUserMetricsExtension* uma_proto_unused) {
-  Profile* profile = cached_profile_.GetMetricsProfile();
-  DCHECK(profile);
-  chromeos::AccountManager* account_manager =
-      g_browser_process->platform_part()
-          ->GetAccountManagerFactory()
-          ->GetAccountManager(profile->GetPath().value());
-  DCHECK(account_manager);
-  if (!account_manager->IsInitialized()) {
-    base::UmaHistogramEnumeration(kFamilyUserLogSegmentHistogramName,
-                                  LogSegment::kOther);
-    return;
-  }
-  // Calls the callback immediately and not asynchronously.
-  account_manager->GetAccounts(base::BindOnce(
-      &FamilyUserMetricsProvider::CheckSecondaryAccountsAndLogSegment,
-      weak_factory_.GetWeakPtr()));
-}
-
-void FamilyUserMetricsProvider::CheckSecondaryAccountsAndLogSegment(
-    const std::vector<chromeos::AccountManager::Account>& accounts) {
-  DCHECK(!accounts.empty());
-  LogSegment log_segment = LogSegment::kOther;
-
-  policy::BrowserPolicyConnectorChromeOS* connector =
-      g_browser_process->platform_part()->browser_policy_connector_chromeos();
-  DCHECK(connector);
-  Profile* profile = cached_profile_.GetMetricsProfile();
-  DCHECK(profile);
-
-  if (profile->IsChild() && accounts.size() == 1) {
-    log_segment = LogSegment::kSupervisedUser;
-  } else if (profile->IsChild() && accounts.size() > 1) {
-    // If a supervised user has a secondary account, then the secondary account
-    // must be K-12 EDU.
-    log_segment = LogSegment::kSupervisedStudent;
-  } else if (!connector->IsEnterpriseManaged() &&
-             GetMetricsLogSegment() == enterprise_management::PolicyData::K12) {
-    DCHECK(profile->GetProfilePolicyConnector()->IsManaged());
-    // This is a K-12 EDU user on an unmanaged ChromeOS device.
-    log_segment = LogSegment::kStudentAtHome;
-  }
-
-  base::UmaHistogramEnumeration(kFamilyUserLogSegmentHistogramName,
-                                log_segment);
-}
-
-enterprise_management::PolicyData::MetricsLogSegment
-FamilyUserMetricsProvider::GetMetricsLogSegment() {
-  Profile* profile = cached_profile_.GetMetricsProfile();
-  DCHECK(profile);
-  const policy::UserCloudPolicyManagerChromeOS* user_cloud_policy_manager =
-      profile->GetUserCloudPolicyManagerChromeOS();
-  if (!user_cloud_policy_manager)
-    return enterprise_management::PolicyData::UNSPECIFIED;
-  const enterprise_management::PolicyData* policy =
-      user_cloud_policy_manager->core()->store()->policy();
-  if (!policy || !policy->has_metrics_log_segment())
-    return enterprise_management::PolicyData::UNSPECIFIED;
-  return policy->metrics_log_segment();
-}

chrome/browser/metrics/family_user_metrics_provider.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_METRICS_FAMILY_USER_METRICS_PROVIDER_H_
-#define CHROME_BROWSER_METRICS_FAMILY_USER_METRICS_PROVIDER_H_
-
-#include <vector>
-
-#include "base/memory/weak_ptr.h"
-#include "chrome/browser/metrics/cached_metrics_profile.h"
-#include "chromeos/components/account_manager/account_manager.h"
-#include "components/metrics/metrics_provider.h"
-#include "components/policy/proto/device_management_backend.pb.h"
-
-namespace metrics {
-class ChromeUserMetricsExtension;
-}  // namespace metrics
-
-// Provides metrics for user types of interest to the Family Experiences team.
-// These categories include supervised users, supervised students, students at
-// home, and other. This metrics provider is ChromeOS specific.
-class FamilyUserMetricsProvider : public metrics::MetricsProvider {
- public:
-  // These enum values represent the current user's log segment for the Family
-  // Experiences team's metrics.
-  // These values are logged to UMA. Entries should not be renumbered and
-  // numeric values should never be reused. Please keep in sync with
-  // "FamilyUserLogSegment" in src/tools/metrics/histograms/enums.xml.
-  enum class LogSegment {
-    // Supervised primary account with no secondary accounts.
-    kSupervisedUser = 0,
-    // Supervised primary account with K-12 EDU secondary account. If the
-    // primary account is supervised, then the secondary account must be K-12
-    // EDU if one exists.
-    kSupervisedStudent = 1,
-    // K-12 EDU primary account on an unmanaged device, regardless of the
-    // secondary account.
-    kStudentAtHome = 2,
-    // User does not fall into any of the above categories.
-    kOther = 3,
-    // Add future entries above this comment, in sync with
-    // "FamilyUserLogSegment" in src/tools/metrics/histograms/enums.xml.
-    // Update kMaxValue to the last value.
-    kMaxValue = kOther
-  };
-
-  // Family user metrics log segment histogram name.
-  static const char kFamilyUserLogSegmentHistogramName[];
-
-  FamilyUserMetricsProvider();
-  ~FamilyUserMetricsProvider() override;
-  FamilyUserMetricsProvider(const FamilyUserMetricsProvider&) = delete;
-  FamilyUserMetricsProvider& operator=(const FamilyUserMetricsProvider&) =
-      delete;
-
-  // MetricsProvider:
-  void ProvideCurrentSessionData(
-      metrics::ChromeUserMetricsExtension* uma_proto_unused) override;
-
- private:
-  // Callback handler for |AccountManager::GetAccounts|.
-  void CheckSecondaryAccountsAndLogSegment(
-      const std::vector<chromeos::AccountManager::Account>& accounts);
-
-  // Returns user's segment for metrics logging.
-  enterprise_management::PolicyData::MetricsLogSegment GetMetricsLogSegment();
-
-  // Use the first signed-in profile for profile-dependent metrics.
-  metrics::CachedMetricsProfile cached_profile_;
-
-  base::WeakPtrFactory<FamilyUserMetricsProvider> weak_factory_{this};
-};
-
-#endif  // CHROME_BROWSER_METRICS_FAMILY_USER_METRICS_PROVIDER_H_

chrome/browser/metrics/family_user_metrics_provider_browsertest.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/metrics/family_user_metrics_provider.h"
-
-#include "base/optional.h"
-#include "base/test/metrics/histogram_tester.h"
-#include "chrome/browser/chromeos/login/test/fake_gaia_mixin.h"
-#include "chrome/browser/chromeos/login/test/scoped_policy_update.h"
-#include "chrome/browser/chromeos/login/test/user_policy_mixin.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/signin/identity_manager_factory.h"
-#include "chrome/browser/supervised_user/logged_in_user_mixin.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/test/base/mixin_based_in_process_browser_test.h"
-#include "components/account_id/account_id.h"
-#include "components/signin/public/identity_manager/account_info.h"
-#include "components/signin/public/identity_manager/identity_manager.h"
-#include "components/signin/public/identity_manager/identity_test_utils.h"
-#include "content/public/test/browser_test.h"
-
-namespace {
-constexpr char kSecondaryK12EDUEmail[] = "testuser1@managedchrome.com";
-
-// Returns the user type for the primary test account for logging in.
-chromeos::LoggedInUserMixin::LogInType GetPrimaryLogInType(
-    FamilyUserMetricsProvider::LogSegment log_segment) {
-  switch (log_segment) {
-    case FamilyUserMetricsProvider::LogSegment::kSupervisedUser:
-      return chromeos::LoggedInUserMixin::LogInType::kChild;
-    case FamilyUserMetricsProvider::LogSegment::kSupervisedStudent:
-      return chromeos::LoggedInUserMixin::LogInType::kChild;
-    case FamilyUserMetricsProvider::LogSegment::kStudentAtHome:
-      return chromeos::LoggedInUserMixin::LogInType::kRegular;
-    case FamilyUserMetricsProvider::LogSegment::kOther:
-      return chromeos::LoggedInUserMixin::LogInType::kRegular;
-  }
-}
-
-// Returns the account id for the primary test account for logging in.
-base::Optional<AccountId> GetPrimaryAccountId(
-    FamilyUserMetricsProvider::LogSegment log_segment) {
-  if (log_segment == FamilyUserMetricsProvider::LogSegment::kStudentAtHome) {
-    // To distinguish K-12 EDU users from Enterprise users in ChromeOS, we use a
-    // PolicyData field. Fetching policy is skipped for obviously consumer
-    // users, who have an @gmail.com e-mail, for example (see comments in
-    // fake_gaia_mixin.h). Since we need policies for this test, we must use an
-    // e-mail address that has an enterprise domain. Of all the user categories,
-    // kStudentAtHome is the only one with an enterprise managed primary
-    // account.
-    return AccountId::FromUserEmailGaiaId(
-        chromeos::FakeGaiaMixin::kEnterpriseUser1,
-        chromeos::FakeGaiaMixin::kEnterpriseUser1GaiaId);
-  }
-  // Use the default FakeGaiaMixin::kFakeUserEmail consumer test account id.
-  return base::nullopt;
-}
-}  // namespace
-
-class FamilyUserMetricsProviderTest
-    : public MixinBasedInProcessBrowserTest,
-      public testing::WithParamInterface<
-          FamilyUserMetricsProvider::LogSegment> {
- public:
-  void SetUpInProcessBrowserTestFixture() override {
-    MixinBasedInProcessBrowserTest::SetUpInProcessBrowserTestFixture();
-
-    const FamilyUserMetricsProvider::LogSegment log_segment = GetParam();
-    if (log_segment == FamilyUserMetricsProvider::LogSegment::kStudentAtHome) {
-      logged_in_user_mixin_.GetUserPolicyMixin()
-          ->RequestPolicyUpdate()
-          ->policy_data()
-          ->set_metrics_log_segment(enterprise_management::PolicyData::K12);
-    }
-  }
-
-  void SetUpOnMainThread() override {
-    MixinBasedInProcessBrowserTest::SetUpOnMainThread();
-    logged_in_user_mixin_.LogInUser();
-  }
-
- protected:
-  chromeos::LoggedInUserMixin logged_in_user_mixin_{
-      &mixin_host_, GetPrimaryLogInType(GetParam()), embedded_test_server(),
-      this,
-      /*should_launch_browser=*/true, GetPrimaryAccountId(GetParam()),
-      /*include_initial_user=*/true,
-      // Don't use LocalPolicyTestServer because it does not support customizing
-      // PolicyData.
-      // TODO(crbug/1112885): Use LocalPolicyTestServer when this is fixed.
-      /*use_local_policy_server=*/false};
-};
-
-IN_PROC_BROWSER_TEST_P(FamilyUserMetricsProviderTest, UserCategory) {
-  base::HistogramTester histogram_tester;
-  FamilyUserMetricsProvider provider;
-
-  const FamilyUserMetricsProvider::LogSegment log_segment = GetParam();
-  if (log_segment ==
-      FamilyUserMetricsProvider::LogSegment::kSupervisedStudent) {
-    // Add a secondary K-12 EDU account.
-    Profile* profile = browser()->profile();
-    ASSERT_TRUE(profile);
-    signin::IdentityManager* identity_manager =
-        IdentityManagerFactory::GetForProfile(profile);
-    AccountInfo account_info =
-        signin::MakeAccountAvailable(identity_manager, kSecondaryK12EDUEmail);
-    EXPECT_TRUE(
-        identity_manager->HasAccountWithRefreshToken(account_info.account_id));
-  }
-
-  provider.ProvideCurrentSessionData(/*uma_proto_unused=*/nullptr);
-
-  histogram_tester.ExpectUniqueSample(
-      FamilyUserMetricsProvider::kFamilyUserLogSegmentHistogramName,
-      log_segment, 1);
-}
-
-INSTANTIATE_TEST_SUITE_P(
-    ,
-    FamilyUserMetricsProviderTest,
-    testing::Values(FamilyUserMetricsProvider::LogSegment::kSupervisedUser,
-                    FamilyUserMetricsProvider::LogSegment::kSupervisedStudent,
-                    FamilyUserMetricsProvider::LogSegment::kStudentAtHome,
-                    FamilyUserMetricsProvider::LogSegment::kOther));

chrome/test/BUILD.gn

@@ -2624,7 +2624,6 @@ if (!is_android) {
         "../browser/extensions/api/settings_private/settings_private_browsertest_chromeos.cc",
         "../browser/extensions/api/vpn_provider/vpn_provider_apitest.cc",
         "../browser/extensions/chromeos_component_extensions_browsertest.cc",
-        "../browser/metrics/family_user_metrics_provider_browsertest.cc",
         "../browser/notifications/notification_platform_bridge_chromeos_browsertest.cc",
         "../browser/resources/chromeos/zip_archiver/test/zip_archiver_jstest.cc",
         "../browser/signin/chromeos_mirror_account_consistency_browsertest.cc",