[Hashed logging] Delete all metric provider logic

The metrics provider class is going to be almost completely rewritten
for the framework. It will make future CLs cleaner to start at a blank
slate.

Bug: 1016655
Change-Id: Ib0e02d7632df11b01167a566db4eac4cbda6acab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903107Reviewed-by: Thanh Nguyen <thanhdng@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713627}

chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_metrics_provider.cc

@@ -4,172 +4,23 @@
 
 #include "chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_metrics_provider.h"
 
-#include "base/bind.h"
-#include "base/files/file_path.h"
-#include "base/files/file_util.h"
-#include "base/files/important_file_writer.h"
-#include "base/guid.h"
-#include "base/logging.h"
-#include "base/metrics/histogram_macros.h"
-#include "base/stl_util.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/task/post_task.h"
-#include "base/threading/scoped_blocking_call.h"
-#include "base/time/time.h"
-#include "base/unguessable_token.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_recorder.h"
-#include "chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_recorder_state.pb.h"
 #include "chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_recorder_util.h"
-#include "components/metrics/metrics_log.h"
-#include "crypto/sha2.h"
-#include "third_party/metrics_proto/chrome_os_app_list_launch_event.pb.h"
-#include "third_party/metrics_proto/chrome_user_metrics_extension.pb.h"
 
 namespace app_list {
 namespace {
 
-using ::metrics::ChromeOSAppListLaunchEventProto;
 using ::metrics::ChromeUserMetricsExtension;
-using ::metrics::MetricsLog;
-
-// Length of the user secret string, in bytes.
-constexpr size_t kSecretSize = 32;
-
-// Tries to serialize the given AppListLaunchRecorderStateProto to disk at the
-// given |filepath|.
-void SaveStateToDisk(const base::FilePath& filepath,
-                     const AppListLaunchRecorderStateProto& proto) {
-  std::string proto_str;
-  if (!proto.SerializeToString(&proto_str)) {
-    LOG(DFATAL) << "Error serializing AppListLaunchRecorderStateProto.";
-    LogMetricsProviderError(MetricsProviderError::kStateToProtoError);
-    return;
-  }
-
-  bool write_result = false;
-  {
-    base::ScopedBlockingCall scoped_blocking_call(
-        FROM_HERE, base::BlockingType::MAY_BLOCK);
-    write_result = base::ImportantFileWriter::WriteFileAtomically(
-        filepath, proto_str, "AppListLaunchMetricsProvider");
-  }
-
-  if (!write_result) {
-    LOG(DFATAL) << "Error writing AppListLaunchRecorderStateProto.";
-    LogMetricsProviderError(MetricsProviderError::kStateWriteError);
-  }
-}
-
-// Tries to load an |AppListLaunchRecorderStateProto| from the given filepath.
-// If it fails, returns nullopt.
-base::Optional<AppListLaunchRecorderStateProto> LoadStateFromDisk(
-    const base::FilePath& filepath) {
-  std::string proto_str;
-  {
-    base::ScopedBlockingCall scoped_blocking_call(
-        FROM_HERE, base::BlockingType::MAY_BLOCK);
-    // If the state file doesn't exist, it's not necessarily an error: this may
-    // be the first time the provider is being run.
-    if (!base::PathExists(filepath))
-      return base::nullopt;
-
-    if (!base::ReadFileToString(filepath, &proto_str)) {
-      LOG(DFATAL) << "Error reading AppListLaunchRecorderStateProto.";
-      LogMetricsProviderError(MetricsProviderError::kStateReadError);
-      return base::nullopt;
-    }
-  }
-
-  AppListLaunchRecorderStateProto proto;
-  if (!proto.ParseFromString(proto_str)) {
-    LOG(DFATAL) << "Error parsing AppListLaunchRecorderStateProto.";
-    LogMetricsProviderError(MetricsProviderError::kStateFromProtoError);
-    return base::nullopt;
-  }
-
-  return proto;
-}
-
-// Returns the file path of the primary user's profile, if it exists and is not
-// a guest session or system profile. Otherwise, returns base::nullopt.
-base::Optional<base::FilePath> GetProfileDir() {
-  // We do not handle multiprofile, events from all logged-in profiles are
-  // logged under the primary user.
-  Profile* profile = ProfileManager::GetPrimaryUserProfile();
-  // Only enable if the current profile is a regular profile, not a guest
-  // session or the login screen.
-  if (!profile || profile->IsGuestSession() || profile->IsSystemProfile())
-    return base::nullopt;
-  return profile->GetPath();
-}
-
-// Generates a cryptographically secure random secret of size |kSecretSize|.
-Secret GenerateSecret() {
-  const auto secret = base::UnguessableToken::Create().ToString();
-  DCHECK_EQ(secret.size(), kSecretSize);
-  return {secret};
-}
-
-// Generates a random user ID.
-uint64_t GenerateUserID() {
-  // This is analogous to how the UMA client ID is generated in
-  // metrics::MetricsStateManager.
-  return MetricsLog::Hash(base::GenerateGUID());
-}
-
-// Generates a proto containing a new secret and user ID.
-AppListLaunchRecorderStateProto GenerateStateProto() {
-  AppListLaunchRecorderStateProto proto;
-  proto.set_secret(GenerateSecret().value);
-  proto.set_recurrence_ranker_user_id(GenerateUserID());
-  return proto;
-}
-
-// Returns the user secret stored in |proto|.
-Secret GetSecretFromProto(const AppListLaunchRecorderStateProto& proto) {
-  DCHECK(proto.has_secret() && !proto.secret().empty());
-  return {proto.secret()};
-}
-
-// Returns the first 8 bytes of the SHA256 hash of |secret| concatenated with
-// |value|.
-uint64_t HashWithSecret(const std::string& value, const Secret& secret) {
-  DCHECK(!secret.value.empty());
-  uint64_t hash;
-  crypto::SHA256HashString(secret.value + value, &hash, sizeof(uint64_t));
-  return hash;
-}
 
 }  // namespace
 
 int AppListLaunchMetricsProvider::kMaxEventsPerUpload = 100;
-char AppListLaunchMetricsProvider::kStateProtoFilename[] =
-    "app_list_launch_recorder_state.pb";
-
-AppListLaunchMetricsProvider::AppListLaunchMetricsProvider(
-    base::RepeatingCallback<base::Optional<base::FilePath>()>
-        get_profile_dir_callback)
-    : get_profile_dir_callback_(get_profile_dir_callback),
-      init_state_(InitState::DISABLED),
-      secret_(base::nullopt),
-      user_id_(base::nullopt) {}
-
-AppListLaunchMetricsProvider::AppListLaunchMetricsProvider()
-    : AppListLaunchMetricsProvider(base::BindRepeating(GetProfileDir)) {}
 
+AppListLaunchMetricsProvider::AppListLaunchMetricsProvider() = default;
 AppListLaunchMetricsProvider::~AppListLaunchMetricsProvider() = default;
 
 void AppListLaunchMetricsProvider::OnRecordingEnabled() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // We do not perform actual initialization or set to InitState::ENABLED here.
-  // Initialization must occur after the browser thread has finished
-  // initializing, which may not be the case when OnRecordingEnabled is
-  // called. Instead, initialization happens on the first call to
-  // OnAppListLaunch.
-  init_state_ = InitState::UNINITIALIZED;
-
   subscription_ = AppListLaunchRecorder::GetInstance()->RegisterCallback(
       base::BindRepeating(&AppListLaunchMetricsProvider::OnAppListLaunch,
                           weak_factory_.GetWeakPtr()));
@@ -177,116 +28,20 @@ void AppListLaunchMetricsProvider::OnRecordingEnabled() {
 
 void AppListLaunchMetricsProvider::OnRecordingDisabled() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  init_state_ = InitState::DISABLED;
-  launch_info_cache_.clear();
-  secret_.reset();
-  user_id_.reset();
   subscription_.reset();
 }
 
 void AppListLaunchMetricsProvider::Initialize() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  const auto& profile_dir = get_profile_dir_callback_.Run();
-  if (!profile_dir) {
-    OnRecordingDisabled();
-    return;
-  }
-  const base::FilePath& proto_filepath = profile_dir.value().AppendASCII(
-      AppListLaunchMetricsProvider::kStateProtoFilename);
-
-  PostTaskAndReplyWithResult(
-      FROM_HERE, {base::ThreadPool(), base::MayBlock()},
-      base::BindOnce(&LoadStateFromDisk, proto_filepath),
-      base::BindOnce(&AppListLaunchMetricsProvider::OnStateLoaded,
-                     weak_factory_.GetWeakPtr(), proto_filepath));
-}
-
-void AppListLaunchMetricsProvider::OnStateLoaded(
-    const base::FilePath& proto_filepath,
-    const base::Optional<AppListLaunchRecorderStateProto>& proto) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (proto) {
-    secret_ = GetSecretFromProto(proto.value());
-    user_id_ = proto.value().recurrence_ranker_user_id();
-  } else {
-    // Either there is no state proto saved, or it was corrupt. Generate a new
-    // secret and ID regardless. We log an 'error' to UMA despite this not
-    // always being an error, because this happening too frequently would
-    // indicate an issue with the system.
-    LogMetricsProviderError(MetricsProviderError::kNoStateProto);
-
-    AppListLaunchRecorderStateProto new_proto = GenerateStateProto();
-    PostTask(FROM_HERE, {base::ThreadPool(), base::MayBlock()},
-             base::BindOnce(&SaveStateToDisk, proto_filepath, new_proto));
-
-    secret_ = GetSecretFromProto(new_proto);
-    user_id_ = new_proto.recurrence_ranker_user_id();
-  }
-
-  if (!user_id_) {
-    LogMetricsProviderError(MetricsProviderError::kInvalidUserId);
-    OnRecordingDisabled();
-  } else if (!secret_ || secret_.value().value.size() != kSecretSize) {
-    LogMetricsProviderError(MetricsProviderError::kInvalidSecret);
-    OnRecordingDisabled();
-  } else {
-    init_state_ = InitState::ENABLED;
-  }
 }
 
 void AppListLaunchMetricsProvider::ProvideCurrentSessionData(
     ChromeUserMetricsExtension* uma_proto) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (init_state_ == InitState::ENABLED) {
-    for (const auto& launch_info : launch_info_cache_) {
-      CreateLaunchEvent(launch_info,
-                        uma_proto->add_chrome_os_app_list_launch_event());
-    }
-    launch_info_cache_.clear();
-  }
 }
 
 void AppListLaunchMetricsProvider::OnAppListLaunch(
     const AppListLaunchRecorder::LaunchInfo& launch_info) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (init_state_ == InitState::UNINITIALIZED) {
-    init_state_ = InitState::INIT_STARTED;
-    Initialize();
-  }
-
-  if (init_state_ == InitState::DISABLED ||
-      launch_info_cache_.size() >= static_cast<size_t>(kMaxEventsPerUpload))
-    return;
-
-  if (launch_info.client == AppListLaunchRecorder::Client::kUnspecified) {
-    LogMetricsProviderError(MetricsProviderError::kLaunchTypeUnspecified);
-    return;
-  }
-
-  launch_info_cache_.push_back(launch_info);
-
-  // We want the metric to reflect the number of uploads affected, not the
-  // number of dropped events, so only log an error when max events is first
-  // exceeded.
-  if (launch_info_cache_.size() == static_cast<size_t>(kMaxEventsPerUpload))
-    LogMetricsProviderError(MetricsProviderError::kMaxEventsPerUploadExceeded);
-}
-
-void AppListLaunchMetricsProvider::CreateLaunchEvent(
-    const AppListLaunchRecorder::LaunchInfo& launch_info,
-    ChromeOSAppListLaunchEventProto* event) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(secret_ && user_id_);
-
-  base::Time::Exploded now;
-  base::Time::Now().LocalExplode(&now);
-
-  // TODO(crbug.com/1016655): set hashed and unhashed data after proto has been
-  // updated.
-
-  // Dummy call so that HashWithSecret compiles.
-  LOG(ERROR) << HashWithSecret("dummy hash", secret_.value());
 }
 
 }  // namespace app_list

chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_metrics_provider.h

@@ -25,14 +25,7 @@ class ChromeMetricsServiceClient;
 
 namespace app_list {
 
-// Stores a user's random secret. This struct exists to make clear at the type
-// system level what is a secret and what isn't.
-struct Secret {
-  std::string value;
-};
-
 class AppListLaunchMetricsProviderTest;
-class AppListLaunchRecorderStateProto;
 
 // AppListLaunchMetricsProvider is responsible for filling out the
 // |app_list_launch_event| section of the UMA proto. This class should not be
@@ -51,76 +44,28 @@ class AppListLaunchMetricsProvider : public metrics::MetricsProvider {
  private:
   friend class ::ChromeMetricsServiceClient;
   friend class app_list::AppListLaunchMetricsProviderTest;
-  FRIEND_TEST_ALL_PREFIXES(AppListLaunchMetricsProviderTest, EventsAreCapped);
-
-  // Filename for the the state proto within the user's home directory. This is
-  // just the basename, not the full path.
-  static char kStateProtoFilename[];
 
   // Beyond this number of OnAppListLaunch calls between successive calls to
   // ProvideCurrentSessionData, we stop logging.
   static int kMaxEventsPerUpload;
 
-  enum class InitState { UNINITIALIZED, INIT_STARTED, ENABLED, DISABLED };
-
   // The constructors are private so that this class can only be instantied by
-  // ChromeMetricsServiceClient and tests. The class has two constructors so
-  // that the tests can override the profile dir.
+  // ChromeMetricsServiceClient and tests.
   AppListLaunchMetricsProvider();
 
-  AppListLaunchMetricsProvider(
-      base::RepeatingCallback<base::Optional<base::FilePath>()>
-          get_profile_dir_callback);
-
   // Records the information in |launch_info|, to be converted into a hashed
   // form and logged to UMA. Actual logging occurs periodically, so it is not
   // guaranteed that any call to this method will be logged.
   //
-  // OnAppListLaunch should only be called from the browser thread, after it has
-  // finished initializing.
+  // OnAppListLaunch should only be called from the browser UI sequence.
   void OnAppListLaunch(const AppListLaunchRecorder::LaunchInfo& launch_info);
 
-  // Starts initialization if needed. This involves reading the secret from
-  // disk.
+  // Performs initialization once a user has logged in.
   void Initialize();
 
-  // Populates this object's internal state from the given state |proto|, and
-  // finishes initialisation. This is called by Initialize() and passed the
-  // state proto that has been loaded from disk, if it exists. OnStateLoaded may
-  // write a new proto to disk if, for example, the given |proto| is invalid.
-  void OnStateLoaded(
-      const base::FilePath& proto_filepath,
-      const base::Optional<AppListLaunchRecorderStateProto>& proto);
-
-  // Converts |launch_info| into a hashed |event| proto ready for logging.
-  void CreateLaunchEvent(const AppListLaunchRecorder::LaunchInfo& launch_info,
-                         metrics::ChromeOSAppListLaunchEventProto* event);
-
-  // A function that returns the directory of the current profile. This is a
-  // callback so that it can be faked out for testing.
-  const base::RepeatingCallback<base::Optional<base::FilePath>()>
-      get_profile_dir_callback_;
-
   // Subscription for receiving logging event callbacks from HashedLogger.
   std::unique_ptr<AppListLaunchRecorder::LaunchEventSubscription> subscription_;
 
-  // Cache of input launch data, to be hashed and returned when
-  // ProvideCurrentSessionData() is called.
-  std::vector<AppListLaunchRecorder::LaunchInfo> launch_info_cache_;
-
-  // The initialization state of the AppListLaunchMetricsProvider. Events are
-  // only provided on a call to ProvideCurrentSessionData() once this is
-  // enabled, and nothing is recorded if this is disabled.
-  InitState init_state_;
-
-  // Secret per-user salt concatenated with values before hashing. Serialized
-  // to disk.
-  base::Optional<Secret> secret_;
-
-  // Per-user per-client ID used only for AppListLaunchMetricsProvider.
-  // Serialized to disk.
-  base::Optional<uint64_t> user_id_;
-
   SEQUENCE_CHECKER(sequence_checker_);
 
   base::WeakPtrFactory<AppListLaunchMetricsProvider> weak_factory_{this};

chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_metrics_provider_unittest.cc

@@ -21,7 +21,6 @@
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_recorder.h"
-#include "chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_recorder_state.pb.h"
 #include "chrome/browser/ui/app_list/search/search_result_ranker/app_list_launch_recorder_util.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
@@ -31,7 +30,6 @@
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/metrics_proto/chrome_os_app_list_launch_event.pb.h"
 #include "third_party/metrics_proto/chrome_user_metrics_extension.pb.h"
 #include "third_party/protobuf/src/google/protobuf/repeated_field.h"
 
@@ -40,44 +38,6 @@ namespace app_list {
 namespace {
 
 using ::chromeos::ProfileHelper;
-using ::metrics::ChromeOSAppListLaunchEventProto;
-
-// 32 bytes long, matching the size of a real secret.
-constexpr char kSecret[] = "abcdefghijklmnopqrstuvwxyzabcdef";
-
-constexpr uint64_t kUserId = 1123u;
-
-constexpr char kValue1[] = "value one";
-constexpr char kValue2[] = "value two";
-constexpr char kValue3[] = "value three";
-constexpr char kValue4[] = "value four";
-
-// These are hex encoded values of the first 8 bytes of sha256(kSecret +
-// kValueN), generated with the sha256sum command. These are hex encoded to make
-// debugging incorrect values easier.
-constexpr char kValue1Hash[] = "E79C24CD2117A2BB";
-constexpr char kValue2Hash[] = "506ECDDC0BA3C341";
-constexpr char kValue3Hash[] = "1E0CDC361557A12F";
-constexpr char kValue4Hash[] = "4875030730DE902A";
-
-enum class TestEvent {
-  kValueA = 0,
-  kValueB = 1,
-  kValueC = 2,
-};
-
-std::string HashToHex(uint64_t hash) {
-  return base::HexEncode(&hash, sizeof(uint64_t));
-}
-
-void ExpectLoggingEventEquals(ChromeOSAppListLaunchEventProto proto,
-                              std::string value_a,
-                              std::string value_b,
-                              int value_c) {
-  // TODO(crbug.com/1016655): reimplement once proto has been refactored.
-  // Dummy call so HashToHex compiles.
-  LOG(ERROR) << HashToHex(0u);
-}
 
 }  // namespace
 
@@ -87,119 +47,6 @@ class AppListLaunchMetricsProviderTest : public testing::Test {
 
   base::Optional<base::FilePath> GetTempDir() { return temp_dir_.GetPath(); }
 
-  void MakeProvider() {
-    // Using WrapUnique to access a private constructor.
-    provider_ = base::WrapUnique(new AppListLaunchMetricsProvider(
-        base::BindRepeating(&AppListLaunchMetricsProviderTest::GetTempDir,
-                            base::Unretained(this))));
-    provider_->OnRecordingEnabled();
-  }
-
-  void InitProvider() {
-    AddLog("", "", 0);
-    Wait();
-  }
-
-  void ExpectUninitialized() {
-    EXPECT_EQ(AppListLaunchMetricsProvider::InitState::UNINITIALIZED,
-              provider_->init_state_);
-  }
-
-  void ExpectInitStarted() {
-    EXPECT_EQ(AppListLaunchMetricsProvider::InitState::INIT_STARTED,
-              provider_->init_state_);
-  }
-
-  void ExpectEnabled() {
-    EXPECT_EQ(AppListLaunchMetricsProvider::InitState::ENABLED,
-              provider_->init_state_);
-  }
-
-  void ExpectDisabled() {
-    EXPECT_EQ(AppListLaunchMetricsProvider::InitState::DISABLED,
-              provider_->init_state_);
-  }
-
-  base::Optional<Secret> GetSecret() { return provider_->secret_; }
-
-  std::string ReadSecret() {
-    std::string proto_str;
-    {
-      base::ScopedBlockingCall scoped_blocking_call(
-          FROM_HERE, base::BlockingType::MAY_BLOCK);
-
-      base::FilePath path = temp_dir_.GetPath().AppendASCII(
-          AppListLaunchMetricsProvider::kStateProtoFilename);
-      CHECK(base::ReadFileToString(path, &proto_str));
-    }
-
-    auto proto = std::make_unique<AppListLaunchRecorderStateProto>();
-    CHECK(proto->ParseFromString(proto_str));
-    return proto->secret();
-  }
-
-  uint64_t ReadUserId() {
-    std::string proto_str;
-    {
-      base::ScopedBlockingCall scoped_blocking_call(
-          FROM_HERE, base::BlockingType::MAY_BLOCK);
-
-      base::FilePath path = temp_dir_.GetPath().AppendASCII(
-          AppListLaunchMetricsProvider::kStateProtoFilename);
-      CHECK(base::ReadFileToString(path, &proto_str));
-    }
-
-    auto proto = std::make_unique<AppListLaunchRecorderStateProto>();
-    CHECK(proto->ParseFromString(proto_str));
-    return proto->recurrence_ranker_user_id();
-  }
-
-  void WriteStateProto(const std::string& secret) {
-    AppListLaunchRecorderStateProto proto;
-    proto.set_recurrence_ranker_user_id(kUserId);
-    proto.set_secret(secret);
-
-    std::string proto_str;
-    CHECK(proto.SerializeToString(&proto_str));
-    {
-      base::ScopedBlockingCall scoped_blocking_call(
-          FROM_HERE, base::BlockingType::MAY_BLOCK);
-      CHECK(base::ImportantFileWriter::WriteFileAtomically(
-          temp_dir_.GetPath().AppendASCII(
-              AppListLaunchMetricsProvider::kStateProtoFilename),
-          proto_str, "AppListLaunchMetricsProviderTest"));
-    }
-
-    Wait();
-  }
-
-  void DeleteStateProto() {
-    DeleteFile(temp_dir_.GetPath().AppendASCII(
-                   AppListLaunchMetricsProvider::kStateProtoFilename),
-               false);
-  }
-
-  void AddLog(std::string value_a, std::string value_b, int value_c) {
-    AppListLaunchRecorder::LaunchInfo event;
-    event.client = AppListLaunchRecorder::Client::kTesting;
-    event.hashed = {{static_cast<int>(TestEvent::kValueA), value_a},
-                    {static_cast<int>(TestEvent::kValueB), value_b}};
-    event.unhashed = {{static_cast<int>(TestEvent::kValueC), value_c}};
-
-    provider_->OnAppListLaunch(event);
-  }
-
-  google::protobuf::RepeatedPtrField<ChromeOSAppListLaunchEventProto>
-  GetLogs() {
-    metrics::ChromeUserMetricsExtension uma_log;
-    provider_->ProvideCurrentSessionData(&uma_log);
-    return uma_log.chrome_os_app_list_launch_event();
-  }
-
-  void ExpectNoErrors() {
-    histogram_tester_.ExpectTotalCount("Apps.AppListLaunchRecorderError", 0);
-  }
-
   void Wait() { task_environment_.RunUntilIdle(); }
 
   base::HistogramTester histogram_tester_;
@@ -209,166 +56,4 @@ class AppListLaunchMetricsProviderTest : public testing::Test {
   std::unique_ptr<AppListLaunchMetricsProvider> provider_;
 };
 
-TEST_F(AppListLaunchMetricsProviderTest,
-       DISABLED_ProvidesNothingWhenUninitialized) {
-  MakeProvider();
-
-  ExpectUninitialized();
-  EXPECT_TRUE(GetLogs().empty());
-  ExpectNoErrors();
-}
-
-TEST_F(AppListLaunchMetricsProviderTest, DISABLED_SucceedsGeneratingNewSecret) {
-  MakeProvider();
-  InitProvider();
-
-  ExpectEnabled();
-  // Because the secret is random, we settle for just checking its length.
-  // In the object:
-  EXPECT_EQ(32ul, GetSecret().value().value.size());
-  // On disk:
-  EXPECT_EQ(32ul, ReadSecret().size());
-
-  EXPECT_EQ(GetSecret().value().value, ReadSecret());
-
-  histogram_tester_.ExpectUniqueSample("Apps.AppListLaunchRecorderError",
-                                       MetricsProviderError::kNoStateProto, 1);
-}
-
-TEST_F(AppListLaunchMetricsProviderTest,
-       DISABLED_SucceedsLoadingExistingSecret) {
-  WriteStateProto(kSecret);
-
-  MakeProvider();
-  InitProvider();
-
-  ExpectEnabled();
-  EXPECT_EQ(kSecret, GetSecret().value().value);
-  ExpectNoErrors();
-}
-
-TEST_F(AppListLaunchMetricsProviderTest, DISABLED_DisableOnInvalidSecret) {
-  WriteStateProto("wrong length");
-
-  MakeProvider();
-  InitProvider();
-
-  ExpectDisabled();
-  histogram_tester_.ExpectUniqueSample("Apps.AppListLaunchRecorderError",
-                                       MetricsProviderError::kInvalidSecret, 1);
-}
-
-// Tests that a call to ProvideCurrentSessionData populates protos for each log,
-// and that those protos contain the right values.
-TEST_F(AppListLaunchMetricsProviderTest, DISABLED_CorrectHashedValues) {
-  WriteStateProto(kSecret);
-  MakeProvider();
-  InitProvider();
-
-  AddLog(kValue1, kValue2, 5);
-  AddLog(kValue2, kValue3, 7);
-  AddLog(kValue3, kValue4, 9);
-
-  const auto& events = GetLogs();
-  // events[0] is a dummy log created in |InitProvider|. We don't test for its
-  // contents below.
-  ASSERT_EQ(events.size(), 4);
-
-  ExpectLoggingEventEquals(events[1], kValue1Hash, kValue2Hash, 5);
-  ExpectLoggingEventEquals(events[2], kValue2Hash, kValue3Hash, 7);
-  ExpectLoggingEventEquals(events[3], kValue3Hash, kValue4Hash, 9);
-  ExpectNoErrors();
-}
-
-// Tests that the logs reported in one call to ProvideCurrentSessionData do no
-// appear in the next.
-TEST_F(AppListLaunchMetricsProviderTest, DISABLED_EventsNotDuplicated) {
-  WriteStateProto(kSecret);
-  MakeProvider();
-  InitProvider();
-
-  AddLog(kValue1, kValue2, 44);
-  auto events = GetLogs();
-  ASSERT_EQ(events.size(), 2);
-  ExpectLoggingEventEquals(events[1], kValue1Hash, kValue2Hash, 44);
-
-  AddLog(kValue1, kValue4, 22);
-  events = GetLogs();
-  ASSERT_EQ(events.size(), 1);
-  ExpectLoggingEventEquals(events[0], kValue3Hash, kValue4Hash, 22);
-
-  EXPECT_TRUE(GetLogs().empty());
-  ExpectNoErrors();
-}
-
-// Tests that logging events are dropped after an unreasonably large number of
-// them are made between uploads.
-TEST_F(AppListLaunchMetricsProviderTest, DISABLED_EventsAreCapped) {
-  MakeProvider();
-  InitProvider();
-
-  const int max_events = AppListLaunchMetricsProvider::kMaxEventsPerUpload;
-
-  // Not enough events to hit the cap.
-  for (int i = 0; i < max_events / 2; ++i)
-    AddLog(kValue1, kValue2, 1);
-  // One event from init, kMaxEventsPerUpload/2 from the loop.
-  EXPECT_EQ(1 + max_events / 2, GetLogs().size());
-
-  // Enough events to hit the cap.
-  for (int i = 0; i < 2 * max_events; ++i)
-    AddLog(kValue1, kValue2, 1);
-  EXPECT_EQ(max_events, GetLogs().size());
-
-  histogram_tester_.ExpectBucketCount(
-      "Apps.AppListLaunchRecorderError",
-      MetricsProviderError::kMaxEventsPerUploadExceeded, 1);
-}
-
-// Tests that logging events that occur before the provider is initialized are
-// still correctly logged after initialization.
-TEST_F(AppListLaunchMetricsProviderTest,
-       DISABLED_LaunchEventsBeforeInitializationAreRecorded) {
-  WriteStateProto(kSecret);
-  MakeProvider();
-
-  // To begin with, the provider is uninitialized and has no logs at all.
-  EXPECT_TRUE(GetLogs().empty());
-  // These logs are added before the provider has finished initialising.
-  AddLog(kValue1, kValue2, 1);
-  AddLog(kValue2, kValue3, 2);
-  AddLog(kValue3, kValue4, 3);
-  // Initialisation is finished here.
-  Wait();
-
-  const auto& events = GetLogs();
-  ASSERT_EQ(events.size(), 3);
-  ExpectLoggingEventEquals(events[0], kValue1Hash, kValue2Hash, 1);
-  ExpectLoggingEventEquals(events[1], kValue2Hash, kValue3Hash, 2);
-  ExpectLoggingEventEquals(events[2], kValue3Hash, kValue4Hash, 3);
-
-  EXPECT_TRUE(GetLogs().empty());
-  ExpectNoErrors();
-}
-
-// Without an existing saved state, instantiating a metrics provider should save
-// an almost certainly unique user ID and secret. Test this by creating a few
-// blank-slate metrics providers.
-TEST_F(AppListLaunchMetricsProviderTest, DISABLED_UserIDsAndSecretsAreUnique) {
-  const int num_runs = 10;
-
-  std::set<uint64_t> user_ids;
-  std::set<std::string> secrets;
-  for (int i = 0; i < num_runs; ++i) {
-    DeleteStateProto();
-    MakeProvider();
-    InitProvider();
-    secrets.insert(ReadSecret());
-    user_ids.insert(ReadUserId());
-  }
-
-  EXPECT_EQ(static_cast<size_t>(num_runs), secrets.size());
-  EXPECT_EQ(static_cast<size_t>(num_runs), user_ids.size());
-}
-
 }  // namespace app_list