[CrOS Bluetooth] Persistent Bluetooth Verbose Log

Makes the verbose bluetooth option persist across device reboot by
adding a new preference flag.

Bug: 1004572
Change-Id: Ibdb278c9b4a326368a591463223e0d1241f2b115
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810451Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Archie Pusaka <apusaka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709806}

chrome/browser/chromeos/BUILD.gn

@@ -2541,6 +2541,7 @@ source_set("unit_tests") {
     "authpolicy/auth_policy_credentials_manager_unittest.cc",
     "authpolicy/authpolicy_helper.unittest.cc",
     "base/file_flusher_unittest.cc",
+    "bluetooth/debug_logs_manager_unittest.cc",
     "certificate_provider/certificate_provider_service_unittest.cc",
     "child_accounts/event_based_status_reporting_service_unittest.cc",
     "child_accounts/parent_access_code/authenticator_unittest.cc",

chrome/browser/chromeos/bluetooth/debug_logs_manager.cc

@@ -7,7 +7,8 @@
 #include "base/feature_list.h"
 #include "base/strings/string_util.h"
 #include "chromeos/constants/chromeos_features.h"
-#include "components/user_manager/user.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
 
 namespace chromeos {
 
@@ -15,19 +16,28 @@ namespace bluetooth {
 
 namespace {
 const char kSupportedEmailSuffix[] = "@google.com";
+const char kVerboseLoggingEnablePrefName[] = "bluetooth.verboseLogging.enable";
 }  // namespace
 
-DebugLogsManager::DebugLogsManager(const user_manager::User* primary_user)
-    : primary_user_(primary_user) {}
+DebugLogsManager::DebugLogsManager(const std::string& primary_user_email,
+                                   PrefService* pref_service)
+    : primary_user_email_(primary_user_email), pref_service_(pref_service) {}
 
 DebugLogsManager::~DebugLogsManager() = default;
 
+// static
+void DebugLogsManager::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterBooleanPref(kVerboseLoggingEnablePrefName,
+                                false /* default_value */);
+}
+
 DebugLogsManager::DebugLogsState DebugLogsManager::GetDebugLogsState() const {
   if (!AreDebugLogsSupported())
     return DebugLogsState::kNotSupported;
 
-  return are_debug_logs_enabled_ ? DebugLogsState::kSupportedAndEnabled
-                                 : DebugLogsState::kSupportedButDisabled;
+  return pref_service_->GetBoolean(kVerboseLoggingEnablePrefName)
+             ? DebugLogsState::kSupportedAndEnabled
+             : DebugLogsState::kSupportedButDisabled;
 }
 
 mojom::DebugLogsChangeHandlerPtr DebugLogsManager::GenerateInterfacePtr() {
@@ -39,8 +49,9 @@ mojom::DebugLogsChangeHandlerPtr DebugLogsManager::GenerateInterfacePtr() {
 void DebugLogsManager::ChangeDebugLogsState(bool should_debug_logs_be_enabled) {
   DCHECK_NE(GetDebugLogsState(), DebugLogsState::kNotSupported);
 
-  // TODO(yshavit): Handle the user enabling/disabling logs.
-  are_debug_logs_enabled_ = should_debug_logs_be_enabled;
+  pref_service_->SetBoolean(kVerboseLoggingEnablePrefName,
+                            should_debug_logs_be_enabled);
+  // TODO(crbug.com/734152): On login, enable logs based on this value
 }
 
 bool DebugLogsManager::AreDebugLogsSupported() const {
@@ -49,10 +60,7 @@ bool DebugLogsManager::AreDebugLogsSupported() const {
     return false;
   }
 
-  if (!primary_user_)
-    return false;
-
-  return base::EndsWith(primary_user_->GetDisplayEmail(), kSupportedEmailSuffix,
+  return base::EndsWith(primary_user_email_, kSupportedEmailSuffix,
                         base::CompareCase::INSENSITIVE_ASCII);
 }
 

chrome/browser/chromeos/bluetooth/debug_logs_manager.h

@@ -9,9 +9,8 @@
 #include "chrome/browser/ui/webui/bluetooth_internals/bluetooth_internals.mojom.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 
-namespace user_manager {
-class User;
-}  // namespace user_manager
+class PrefService;
+class PrefRegistrySimple;
 
 namespace chromeos {
 
@@ -24,7 +23,8 @@ namespace bluetooth {
 // state of debug logs and handles the user enabling/disabling them.
 class DebugLogsManager : public mojom::DebugLogsChangeHandler {
  public:
-  explicit DebugLogsManager(const user_manager::User* primary_user);
+  DebugLogsManager(const std::string& primary_user_email,
+                   PrefService* pref_service);
   ~DebugLogsManager() override;
 
   // State for capturing debug Bluetooth logs; logs are only captured when
@@ -37,19 +37,21 @@ class DebugLogsManager : public mojom::DebugLogsChangeHandler {
     kSupportedAndEnabled
   };
 
+  static void RegisterPrefs(PrefRegistrySimple* registry);
+
   DebugLogsState GetDebugLogsState() const;
 
+  // mojom::DebugLogsManager:
+  void ChangeDebugLogsState(bool should_debug_logs_be_enabled) override;
+
   // Generates an InterfacePtr bound to this object.
   mojom::DebugLogsChangeHandlerPtr GenerateInterfacePtr();
 
  private:
-  // mojom::DebugLogsManager:
-  void ChangeDebugLogsState(bool should_debug_logs_be_enabled) override;
-
   bool AreDebugLogsSupported() const;
 
-  const user_manager::User* primary_user_ = nullptr;
-  bool are_debug_logs_enabled_ = false;
+  const std::string primary_user_email_;
+  PrefService* pref_service_ = nullptr;
   mojo::BindingSet<mojom::DebugLogsChangeHandler> bindings_;
 
   DISALLOW_COPY_AND_ASSIGN(DebugLogsManager);

chrome/browser/chromeos/bluetooth/debug_logs_manager_factory.cc

@@ -23,8 +23,10 @@ namespace {
 class DebugLogsManagerService : public KeyedService {
  public:
   explicit DebugLogsManagerService(Profile* profile)
-      : debug_logs_manager_(
-            chromeos::ProfileHelper::Get()->GetUserByProfile(profile)) {}
+      : debug_logs_manager_(chromeos::ProfileHelper::Get()
+                                ->GetUserByProfile(profile)
+                                ->GetDisplayEmail(),
+                            profile->GetPrefs()) {}
 
   ~DebugLogsManagerService() override = default;
 

chrome/browser/chromeos/bluetooth/debug_logs_manager_unittest.cc

+// Copyright 2019 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/chromeos/bluetooth/debug_logs_manager.h"
+
+#include <memory>
+
+#include "base/test/scoped_feature_list.h"
+#include "chromeos/constants/chromeos_features.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+namespace bluetooth {
+
+namespace {
+
+constexpr char kTestGooglerEmail[] = "user@google.com";
+constexpr char kTestNonGooglerEmail[] = "user@gmail.com";
+
+}  // namespace
+
+class DebugLogsManagerTest : public testing::Test {
+ public:
+  DebugLogsManagerTest() = default;
+
+  void SetUp() override { DebugLogsManager::RegisterPrefs(prefs_.registry()); }
+
+  void TearDown() override { debug_logs_manager_.reset(); }
+
+  void InitDebugManager(const char* email, bool debug_flag_enabled) {
+    feature_list_.InitWithFeatureState(
+        chromeos::features::kShowBluetoothDebugLogToggle, debug_flag_enabled);
+
+    debug_logs_manager_ = std::make_unique<DebugLogsManager>(email, &prefs_);
+  }
+
+  void DeleteAndRecreateDebugManager(const char* email) {
+    debug_logs_manager_.reset();
+    debug_logs_manager_ = std::make_unique<DebugLogsManager>(email, &prefs_);
+  }
+
+  DebugLogsManager* debug_manager() const { return debug_logs_manager_.get(); }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+  std::unique_ptr<DebugLogsManager> debug_logs_manager_;
+  TestingPrefServiceSimple prefs_;
+
+  DISALLOW_COPY_AND_ASSIGN(DebugLogsManagerTest);
+};
+
+TEST_F(DebugLogsManagerTest, FlagNotEnabled) {
+  InitDebugManager(kTestGooglerEmail, false /* debug_flag_enabled */);
+  EXPECT_EQ(debug_manager()->GetDebugLogsState(),
+            DebugLogsManager::DebugLogsState::kNotSupported);
+}
+
+TEST_F(DebugLogsManagerTest, NonGoogler) {
+  InitDebugManager(kTestNonGooglerEmail, true /* debug_flag_enabled */);
+  EXPECT_EQ(debug_manager()->GetDebugLogsState(),
+            DebugLogsManager::DebugLogsState::kNotSupported);
+}
+
+TEST_F(DebugLogsManagerTest, ChangeDebugLogsState) {
+  InitDebugManager(kTestGooglerEmail, true /* debug_flag_enabled */);
+  EXPECT_EQ(debug_manager()->GetDebugLogsState(),
+            DebugLogsManager::DebugLogsState::kSupportedButDisabled);
+
+  debug_manager()->ChangeDebugLogsState(true);
+  EXPECT_EQ(debug_manager()->GetDebugLogsState(),
+            DebugLogsManager::DebugLogsState::kSupportedAndEnabled);
+
+  // debug logs state should be saved despite DebugLogsManager is destroyed.
+  DeleteAndRecreateDebugManager(kTestGooglerEmail);
+  EXPECT_EQ(debug_manager()->GetDebugLogsState(),
+            DebugLogsManager::DebugLogsState::kSupportedAndEnabled);
+
+  debug_manager()->ChangeDebugLogsState(false);
+  EXPECT_EQ(debug_manager()->GetDebugLogsState(),
+            DebugLogsManager::DebugLogsState::kSupportedButDisabled);
+
+  DeleteAndRecreateDebugManager(kTestGooglerEmail);
+  EXPECT_EQ(debug_manager()->GetDebugLogsState(),
+            DebugLogsManager::DebugLogsState::kSupportedButDisabled);
+}
+
+}  // namespace bluetooth
+
+}  // namespace chromeos

chrome/browser/prefs/browser_prefs.cc

@@ -243,6 +243,7 @@
 #include "chrome/browser/chromeos/app_mode/web_app/web_kiosk_app_manager.h"
 #include "chrome/browser/chromeos/arc/policy/arc_policy_bridge.h"
 #include "chrome/browser/chromeos/arc/session/arc_session_manager.h"
+#include "chrome/browser/chromeos/bluetooth/debug_logs_manager.h"
 #include "chrome/browser/chromeos/child_accounts/parent_access_code/parent_access_service.h"
 #include "chrome/browser/chromeos/child_accounts/screen_time_controller.h"
 #include "chrome/browser/chromeos/crostini/crostini_pref_names.h"
@@ -935,6 +936,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry,
   certificate_manager::CertificatesHandler::RegisterProfilePrefs(registry);
   chromeos::AccountManager::RegisterPrefs(registry);
   chromeos::assistant::prefs::RegisterProfilePrefsForBrowser(registry);
+  chromeos::bluetooth::DebugLogsManager::RegisterPrefs(registry);
   chromeos::CupsPrintersManager::RegisterProfilePrefs(registry);
   chromeos::device_sync::DeviceSyncImpl::RegisterProfilePrefs(registry);
   chromeos::first_run::RegisterProfilePrefs(registry);