[CloudReady][OOBE] Add controller to write setting before ownership
kRevenEnableDeviceHWDataUsage cros setting can be set during user
onboarding. HWDataUsageController class allows writing device settings
before ownership of the device is taken.
Bug: b:190727230
Change-Id: I71260bcaa4896cc82bb2d5284e99739574e0ce3c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3301468
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Danila Kuzmin <dkuzmin@google.com>
Cr-Commit-Position: refs/heads/main@{#949507}
diff --git a/chrome/browser/ash/login/screens/hardware_data_collection_screen.cc b/chrome/browser/ash/login/screens/hardware_data_collection_screen.cc
index 09eb9d2..bf900d1 100644
--- a/chrome/browser/ash/login/screens/hardware_data_collection_screen.cc
+++ b/chrome/browser/ash/login/screens/hardware_data_collection_screen.cc
@@ -8,6 +8,7 @@
#include "ash/constants/ash_switches.h"
#include "chrome/browser/ash/policy/core/browser_policy_connector_ash.h"
+#include "chrome/browser/ash/settings/hardware_data_usage_controller.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/browser_process_platform_part.h"
#include "chrome/browser/profiles/profile_manager.h"
@@ -94,7 +95,8 @@
void HWDataCollectionScreen::OnUserAction(const std::string& action_id) {
if (action_id == kUserActionAcceptButtonClicked) {
- // TODO(dkuzmin): set a device setting here, once it will be available
+ HWDataUsageController::Get()->Set(ProfileManager::GetActiveUserProfile(),
+ base::Value(hw_data_usage_enabled_));
exit_callback_.Run(hw_data_usage_enabled_
? Result::ACCEPTED_WITH_HW_DATA_USAGE
: Result::ACCEPTED_WITHOUT_HW_DATA_USAGE);
diff --git a/chrome/browser/ash/settings/device_settings_provider.cc b/chrome/browser/ash/settings/device_settings_provider.cc
index b7b5167..d17694e5 100644
--- a/chrome/browser/ash/settings/device_settings_provider.cc
+++ b/chrome/browser/ash/settings/device_settings_provider.cc
@@ -29,6 +29,7 @@
#include "chrome/browser/ash/policy/off_hours/off_hours_proto_parser.h"
#include "chrome/browser/ash/settings/cros_settings.h"
#include "chrome/browser/ash/settings/device_settings_cache.h"
+#include "chrome/browser/ash/settings/hardware_data_usage_controller.h"
#include "chrome/browser/ash/settings/stats_reporting_controller.h"
#include "chrome/browser/ash/tpm_firmware_update.h"
#include "chromeos/dbus/dbus_thread_manager.h"
@@ -1409,18 +1410,21 @@
// TODO(https://crbug.com/433840): Some of the above code can be
// simplified or removed, once the DoSet function is removed - then there
- // will be no pending writes. This is because the only value that needs to
- // be written as a pending write is kStatsReportingPref, and this is now
- // handled by the StatsReportingController - see below. Once DoSet is
- // removed and there are no pending writes that are being maintained by
- // DeviceSettingsProvider, this code for updating the signed settings for
- // the new owner should probably be moved outside of
- // DeviceSettingsProvider.
+ // will be no pending writes. This is because the only values that need to
+ // be written as a pending write is kStatsReportingPref and
+ // kEnableDeviceHWDataUsage, and those are now handled by the Controllers
+ // - see below. Once DoSet is removed and there are no pending writes that
+ // are being maintained by DeviceSettingsProvider, this code for updating
+ // the signed settings for the new owner should probably be moved outside
+ // of DeviceSettingsProvider.
StatsReportingController::Get()->OnOwnershipTaken(
device_settings_service_->GetOwnerSettingsService());
+ HWDataUsageController::Get()->OnOwnershipTaken(
+ device_settings_service_->GetOwnerSettingsService());
} else if (chromeos::InstallAttributes::Get()->IsEnterpriseManaged()) {
StatsReportingController::Get()->ClearPendingValue();
+ HWDataUsageController::Get()->ClearPendingValue();
}
}
diff --git a/chrome/browser/ash/settings/hardware_data_usage_controller.cc b/chrome/browser/ash/settings/hardware_data_usage_controller.cc
new file mode 100644
index 0000000..74b09a3
--- /dev/null
+++ b/chrome/browser/ash/settings/hardware_data_usage_controller.cc
@@ -0,0 +1,69 @@
+// Copyright 2021 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/ash/settings/hardware_data_usage_controller.h"
+
+#include "ash/components/settings/cros_settings_names.h"
+#include "base/bind.h"
+#include "chrome/browser/ash/settings/cros_settings.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+
+namespace {
+
+constexpr char kPendingPref[] = "pending.cros.reven.enable_hw_data_usage";
+
+} // namespace
+
+namespace ash {
+
+static HWDataUsageController* g_hw_data_usage_controller = nullptr;
+
+// static
+void HWDataUsageController::Initialize(PrefService* local_state) {
+ CHECK(!g_hw_data_usage_controller);
+ g_hw_data_usage_controller = new HWDataUsageController(local_state);
+}
+
+// static
+bool HWDataUsageController::IsInitialized() {
+ return g_hw_data_usage_controller;
+}
+
+// static
+void HWDataUsageController::Shutdown() {
+ DCHECK(g_hw_data_usage_controller);
+ delete g_hw_data_usage_controller;
+ g_hw_data_usage_controller = nullptr;
+}
+
+// static
+HWDataUsageController* HWDataUsageController::Get() {
+ CHECK(g_hw_data_usage_controller);
+ return g_hw_data_usage_controller;
+}
+
+// static
+void HWDataUsageController::RegisterLocalStatePrefs(
+ PrefRegistrySimple* registry) {
+ registry->RegisterBooleanPref(kPendingPref, false,
+ PrefRegistry::NO_REGISTRATION_FLAGS);
+}
+
+HWDataUsageController::HWDataUsageController(PrefService* local_state)
+ : OwnerPendingSettingController(kRevenEnableDeviceHWDataUsage,
+ kPendingPref,
+ local_state) {
+ setting_subscription_ = CrosSettings::Get()->AddSettingsObserver(
+ kRevenEnableDeviceHWDataUsage,
+ base::BindRepeating(&HWDataUsageController::NotifyObservers,
+ this->as_weak_ptr()));
+}
+
+HWDataUsageController::~HWDataUsageController() {
+ owner_settings_service_observation_.Reset();
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+
+} // namespace ash
diff --git a/chrome/browser/ash/settings/hardware_data_usage_controller.h b/chrome/browser/ash/settings/hardware_data_usage_controller.h
new file mode 100644
index 0000000..ea48de46
--- /dev/null
+++ b/chrome/browser/ash/settings/hardware_data_usage_controller.h
@@ -0,0 +1,43 @@
+// Copyright 2021 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_ASH_SETTINGS_HARDWARE_DATA_USAGE_CONTROLLER_H_
+#define CHROME_BROWSER_ASH_SETTINGS_HARDWARE_DATA_USAGE_CONTROLLER_H_
+
+#include "chrome/browser/ash/settings/owner_pending_setting_controller.h"
+
+class PrefRegistrySimple;
+class PrefService;
+
+namespace ash {
+
+// Class to control setting of cros.reven.enable_hw_data_usage device preference
+// before ownership is taken.
+class HWDataUsageController : public OwnerPendingSettingController {
+ public:
+ // Manage singleton instance.
+ static void Initialize(PrefService* local_state);
+ static bool IsInitialized();
+ static void Shutdown();
+ static HWDataUsageController* Get();
+
+ HWDataUsageController(const HWDataUsageController&) = delete;
+ HWDataUsageController& operator=(const HWDataUsageController&) = delete;
+
+ static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
+
+ private:
+ explicit HWDataUsageController(PrefService* local_state);
+ ~HWDataUsageController() override;
+};
+
+} // namespace ash
+
+// TODO(https://crbug.com/1164001): remove when Chrome OS code migration is
+// done.
+namespace chromeos {
+using ::ash::HWDataUsageController;
+} // namespace chromeos
+
+#endif // CHROME_BROWSER_ASH_SETTINGS_HARDWARE_DATA_USAGE_CONTROLLER_H_
diff --git a/chrome/browser/ash/settings/owner_pending_setting_controller.cc b/chrome/browser/ash/settings/owner_pending_setting_controller.cc
new file mode 100644
index 0000000..36ed811
--- /dev/null
+++ b/chrome/browser/ash/settings/owner_pending_setting_controller.cc
@@ -0,0 +1,215 @@
+// Copyright 2021 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/ash/settings/owner_pending_setting_controller.h"
+
+#include <string>
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "chrome/browser/ash/ownership/owner_settings_service_ash.h"
+#include "chrome/browser/ash/ownership/owner_settings_service_ash_factory.h"
+#include "chrome/browser/ash/settings/cros_settings.h"
+#include "chrome/browser/ash/settings/device_settings_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/ownership/owner_settings_service.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+
+namespace ash {
+
+OwnerPendingSettingController::OwnerPendingSettingController(
+ const std::string& pref_name,
+ const std::string& pending_pref_name,
+ PrefService* local_state)
+ : local_state_(local_state),
+ pref_name_(pref_name),
+ pending_pref_name_(pending_pref_name) {
+ value_notified_to_observers_ = GetValue();
+}
+
+void OwnerPendingSettingController::Set(Profile* profile,
+ const base::Value& value) {
+ DCHECK(profile);
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+ if (GetOwnershipStatus() == DeviceSettingsService::OWNERSHIP_TAKEN) {
+ // The device has an owner. If the current profile is that owner, we will
+ // write the value on their behalf, otherwise no action is taken.
+ VLOG(1) << "Already has owner";
+ SetWithServiceAsync(GetOwnerSettingsService(profile), value);
+ } else {
+ // The device has no owner, or we do not know yet whether the device has an
+ // owner. We write a pending value that will be persisted when ownership is
+ // taken (if that has not already happened).
+ // We store the new value in the local state, so that even if Chrome is
+ // restarted before ownership is taken, we will still persist it eventually.
+ // See OnOwnershipTaken.
+ VLOG(1) << "Pending owner; setting pending pref name: "
+ << pending_pref_name_;
+ local_state_->Set(pending_pref_name_, value);
+ NotifyObservers();
+ }
+}
+
+absl::optional<base::Value> OwnerPendingSettingController::GetValue() const {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ absl::optional<base::Value> value = GetPendingValue();
+ if (ShouldReadFromPendingValue() && value.has_value()) {
+ // Return the pending value if it exists.
+ return value;
+ }
+ // Otherwise, always return the value from the signed store.
+ return GetSignedStoredValue();
+}
+
+base::CallbackListSubscription OwnerPendingSettingController::AddObserver(
+ const base::RepeatingClosure& callback) {
+ DCHECK(!callback.is_null());
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ return callback_list_.Add(callback);
+}
+
+void OwnerPendingSettingController::OnOwnershipTaken(
+ ownership::OwnerSettingsService* service) {
+ DCHECK_EQ(GetOwnershipStatus(), DeviceSettingsService::OWNERSHIP_TAKEN);
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ VLOG(1) << "OnOwnershipTaken";
+
+ absl::optional<base::Value> pending_value = GetPendingValue();
+ if (pending_value.has_value()) {
+ // At the time ownership is taken, there is a value waiting to be written.
+ // Use the OwnerSettingsService of the new owner to write the setting.
+ SetWithServiceAsync(service, pending_value.value());
+ }
+}
+
+OwnerPendingSettingController::~OwnerPendingSettingController() {
+ owner_settings_service_observation_.Reset();
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+
+void OwnerPendingSettingController::OnSignedPolicyStored(bool success) {
+ if (!success)
+ return;
+ absl::optional<base::Value> pending_value = GetPendingValue();
+ absl::optional<base::Value> signed_value = GetSignedStoredValue();
+ if (pending_value.has_value() && signed_value.has_value() &&
+ pending_value == signed_value) {
+ is_value_being_set_with_service_ = false;
+ owner_settings_service_observation_.Reset();
+ ClearPendingValue();
+ NotifyObservers();
+ if (on_device_settings_stored_callback_)
+ std::move(on_device_settings_stored_callback_).Run();
+ }
+}
+
+void OwnerPendingSettingController::SetOnDeviceSettingsStoredCallBack(
+ base::OnceClosure callback) {
+ CHECK(!on_device_settings_stored_callback_);
+ on_device_settings_stored_callback_ = std::move(callback);
+}
+
+void OwnerPendingSettingController::SetWithServiceAsync(
+ ownership::OwnerSettingsService* service, // Can be null for non-owners.
+ const base::Value& value) {
+ bool not_yet_ready = service && !service->IsReady();
+ if (not_yet_ready) {
+ VLOG(1) << "Service not yet ready. Adding listener.";
+ // Service is not yet ready. Listen for changes in its readiness so we can
+ // write the value once it is ready. Uses weak pointers, so if everything
+ // is shutdown and deleted in the meantime, this callback isn't run.
+ service->IsOwnerAsync(base::BindOnce(
+ &OwnerPendingSettingController::SetWithServiceCallback,
+ this->as_weak_ptr(), service->as_weak_ptr(), value.Clone()));
+ } else {
+ // Service is either null, or ready - use it right now.
+ SetWithService(service, value);
+ }
+}
+
+void OwnerPendingSettingController::SetWithServiceCallback(
+ const base::WeakPtr<ownership::OwnerSettingsService>& service,
+ const base::Value value,
+ bool is_owner) {
+ if (service) // Make sure service wasn't deleted in the meantime.
+ SetWithService(service.get(), value);
+}
+
+void OwnerPendingSettingController::SetWithService(
+ ownership::OwnerSettingsService* service, // Can be null for non-owners.
+ const base::Value& value) {
+ if (service && service->IsOwner()) {
+ if (!owner_settings_service_observation_.IsObserving())
+ owner_settings_service_observation_.Observe(service);
+ is_value_being_set_with_service_ = true;
+ service->Set(pref_name_, value);
+ local_state_->Set(pending_pref_name_, value);
+ } else {
+ // Do nothing since we are not the owner.
+ LOG(WARNING) << "Changing settings from non-owner, setting=" << pref_name_;
+ }
+}
+
+void OwnerPendingSettingController::NotifyObservers() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ absl::optional<base::Value> current_value = GetValue();
+ if (current_value != value_notified_to_observers_) {
+ VLOG(1) << "Notifying observers";
+ value_notified_to_observers_ = std::move(current_value);
+ callback_list_.Notify();
+ } else {
+ VLOG(1) << "Not notifying (already notified)";
+ }
+}
+
+DeviceSettingsService::OwnershipStatus
+OwnerPendingSettingController::GetOwnershipStatus() const {
+ return DeviceSettingsService::Get()->GetOwnershipStatus();
+}
+
+ownership::OwnerSettingsService*
+OwnerPendingSettingController::GetOwnerSettingsService(Profile* profile) {
+ return OwnerSettingsServiceAshFactory::GetForBrowserContext(profile);
+}
+
+absl::optional<base::Value> OwnerPendingSettingController::GetPendingValue()
+ const {
+ if (local_state_->HasPrefPath(pending_pref_name_)) {
+ return local_state_->Get(pending_pref_name_)->Clone();
+ }
+ return absl::nullopt;
+}
+
+void OwnerPendingSettingController::ClearPendingValue() {
+ VLOG(1) << "ClearPendingValue";
+ local_state_->ClearPref(pending_pref_name_);
+}
+
+absl::optional<base::Value>
+OwnerPendingSettingController::GetSignedStoredValue() const {
+ const base::Value* value = CrosSettings::Get()->GetPref(pref_name_);
+ if (value) {
+ return value->Clone();
+ }
+ return absl::nullopt;
+}
+
+bool OwnerPendingSettingController::ShouldReadFromPendingValue() const {
+ // Read from pending value before ownership is taken or ownership is
+ // unknown. There's a brief moment when ownership is unknown for every
+ // Chrome starts. In that case, we will read from pending value if it exists
+ // (which means ownership is not taken), and read from service when pending
+ // value pending is cleared (which means ownership is taken).
+ if (GetOwnershipStatus() == DeviceSettingsService::OWNERSHIP_NONE ||
+ GetOwnershipStatus() == DeviceSettingsService::OWNERSHIP_UNKNOWN) {
+ return true;
+ }
+ // Read from pending value if ownership is taken but pending value has not
+ // been set successfully with service.
+ return is_value_being_set_with_service_;
+}
+
+} // namespace ash
diff --git a/chrome/browser/ash/settings/owner_pending_setting_controller.h b/chrome/browser/ash/settings/owner_pending_setting_controller.h
new file mode 100644
index 0000000..7e06c67
--- /dev/null
+++ b/chrome/browser/ash/settings/owner_pending_setting_controller.h
@@ -0,0 +1,165 @@
+// Copyright 2021 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_ASH_SETTINGS_OWNER_PENDING_SETTING_CONTROLLER_H_
+#define CHROME_BROWSER_ASH_SETTINGS_OWNER_PENDING_SETTING_CONTROLLER_H_
+
+#include "base/callback_list.h"
+#include "base/memory/weak_ptr.h"
+#include "base/scoped_observation.h"
+#include "base/sequence_checker.h"
+#include "base/values.h"
+#include "chrome/browser/ash/settings/cros_settings.h"
+#include "chrome/browser/ash/settings/device_settings_service.h"
+#include "components/ownership/owner_settings_service.h"
+
+class PrefService;
+class Profile;
+
+namespace ownership {
+class OwnerSettingsService;
+}
+
+namespace ash {
+
+// An extra layer on top of CrosSettings / OwnerSettingsService that allows for
+// writing a setting before ownership is taken.
+//
+// Ordinarily, the OwnerSettingsService interface is used for writing settings,
+// and the CrosSettings interface is used for reading them - but as the OSS
+// cannot be used until the device has an owner, this class can be used instead,
+// since writing the new value with SetEnabled works even before ownership is
+// taken.
+//
+// If OSS is ready then the new value is written straight away, and if not, then
+// a pending write is queued that is completed as soon as the OSS is ready.
+// This write will complete even if Chrome is restarted in the meantime.
+// The caller need not care whether the write was immediate or pending, as long
+// as they also use this class to read the value of kStatsReportingPref.
+// IsEnabled will return the pending value until ownership is taken and the
+// pending value is written - from then on it will return the signed, stored
+// value from CrosSettings.
+class OwnerPendingSettingController
+ : public ownership::OwnerSettingsService::Observer {
+ public:
+ OwnerPendingSettingController() = delete;
+ OwnerPendingSettingController(const OwnerPendingSettingController&) = delete;
+ OwnerPendingSettingController& operator=(
+ const OwnerPendingSettingController&) = delete;
+
+ // Store the new value. This will happen straight away if |profile| is the
+ // owner, and it will cause a pending write to be buffered and written later
+ // if the device has no owner yet. It will write a warning and skip if the
+ // device already has an owner, and |profile| is not that owner.
+ void Set(Profile* profile, const base::Value& value);
+
+ // Returns the latest value - regardless of whether this has been successfully
+ // signed and persisted, or if it is still stored as a pending write. Can
+ // return absl::nullopt if there is no pending write and no signed value.
+ absl::optional<base::Value> GetValue() const;
+
+ // Add an observer |callback| for changes to the setting.
+ base::CallbackListSubscription AddObserver(
+ const base::RepeatingClosure& callback) WARN_UNUSED_RESULT;
+
+ // Called once ownership is taken, |service| is the service of the user taking
+ // ownership.
+ void OnOwnershipTaken(ownership::OwnerSettingsService* service);
+
+ // Sets the callback which is called once when the pending |value| is
+ // propagated to the device settings. Support only one callback at a time.
+ // CHECKs if the second callback is being set.
+ // It's different from the |AddObserver| API. Observers are called
+ // immediately after |Set| is called with the different |value| setting.
+ void SetOnDeviceSettingsStoredCallBack(base::OnceClosure callback);
+
+ // ownership::OwnerSettingsService::Observer implementation:
+ void OnSignedPolicyStored(bool success) override;
+
+ // Clears any value waiting to be written (from storage in local state).
+ void ClearPendingValue();
+
+ protected:
+ OwnerPendingSettingController(const std::string& pref_name,
+ const std::string& pending_pref_name,
+ PrefService* local_state);
+ ~OwnerPendingSettingController() override;
+
+ // Delegates immediately to SetWithService if |service| is ready, otherwise
+ // runs SetWithService asynchronously once |service| is ready.
+ void SetWithServiceAsync(ownership::OwnerSettingsService* service,
+ const base::Value& value);
+
+ // Callback used by SetWithServiceAsync.
+ void SetWithServiceCallback(
+ const base::WeakPtr<ownership::OwnerSettingsService>& service,
+ const base::Value value,
+ bool is_owner);
+
+ // Uses |service| to write the latest value, as long as |service| belongs
+ // to the owner - otherwise just prints a warning.
+ void SetWithService(ownership::OwnerSettingsService* service,
+ const base::Value& value);
+
+ // Notifies observers if the value has changed.
+ void NotifyObservers();
+
+ base::WeakPtr<OwnerPendingSettingController> as_weak_ptr() {
+ return weak_factory_.GetWeakPtr();
+ }
+
+ SEQUENCE_CHECKER(sequence_checker_);
+
+ PrefService* local_state_;
+ absl::optional<base::Value> value_notified_to_observers_;
+ base::RepeatingClosureList callback_list_;
+ base::CallbackListSubscription setting_subscription_;
+
+ base::ScopedObservation<ownership::OwnerSettingsService,
+ ownership::OwnerSettingsService::Observer>
+ owner_settings_service_observation_{this};
+
+ // Indicates if the setting value is in the process of being set with the
+ // service. There is a small period of time needed between start saving the
+ // value and before the value is stored correctly in the service. We should
+ // not use the setting value from the service if it is still in the process
+ // of being saved.
+ bool is_value_being_set_with_service_ = false;
+
+ private:
+ friend class StatsReportingControllerTest;
+
+ // Gets the current ownership status - owned, unowned, or unknown.
+ DeviceSettingsService::OwnershipStatus GetOwnershipStatus() const;
+
+ // Get the owner-settings service for a particular profile. A variety of
+ // different results can be returned, depending on the profile.
+ // a) A ready-to-use service that we know belongs to the owner.
+ // b) A ready-to-use service that we know does NOT belong to the owner.
+ // c) A service that is NOT ready-to-use, which MIGHT belong to the owner.
+ // d) nullptr (for instance, if |profile| is a guest).
+ ownership::OwnerSettingsService* GetOwnerSettingsService(Profile* profile);
+
+ // Return the value waiting to be written (stored in local_state), if one
+ // exists.
+ absl::optional<base::Value> GetPendingValue() const;
+
+ // Return the value signed and stored in CrosSettings, if one exists.
+ absl::optional<base::Value> GetSignedStoredValue() const;
+
+ // Returns whether pending value should be used when determining the value
+ // of `GetValue`.
+ bool ShouldReadFromPendingValue() const;
+
+ base::OnceClosure on_device_settings_stored_callback_;
+
+ const std::string pref_name_;
+ const std::string pending_pref_name_;
+
+ base::WeakPtrFactory<OwnerPendingSettingController> weak_factory_{this};
+};
+
+} // namespace ash
+
+#endif // CHROME_BROWSER_ASH_SETTINGS_OWNER_PENDING_SETTING_CONTROLLER_H_
diff --git a/chrome/browser/ash/settings/stats_reporting_controller.cc b/chrome/browser/ash/settings/stats_reporting_controller.cc
index 11ea28d05..652327565 100644
--- a/chrome/browser/ash/settings/stats_reporting_controller.cc
+++ b/chrome/browser/ash/settings/stats_reporting_controller.cc
@@ -4,19 +4,12 @@
#include "chrome/browser/ash/settings/stats_reporting_controller.h"
-#include <string>
-
#include "ash/components/settings/cros_settings_names.h"
#include "base/bind.h"
#include "base/logging.h"
-#include "chrome/browser/ash/ownership/owner_settings_service_ash.h"
-#include "chrome/browser/ash/ownership/owner_settings_service_ash_factory.h"
#include "chrome/browser/ash/settings/cros_settings.h"
-#include "chrome/browser/ash/settings/device_settings_service.h"
-#include "chrome/browser/profiles/profile.h"
#include "components/metrics/structured/neutrino_logging.h"
#include "components/metrics/structured/neutrino_logging_util.h"
-#include "components/ownership/owner_settings_service.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
@@ -62,92 +55,31 @@
}
void StatsReportingController::SetEnabled(Profile* profile, bool enabled) {
- DCHECK(profile);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- VLOG(1) << "SetEnabled(" << std::boolalpha << enabled << ")";
-
- if (GetOwnershipStatus() == DeviceSettingsService::OWNERSHIP_TAKEN) {
- // The device has an owner. If the current profile is that owner, we will
- // write the value on their behalf, otherwise no action is taken.
- VLOG(1) << "Already has owner";
- SetWithServiceAsync(GetOwnerSettingsService(profile), enabled);
- } else {
- // The device has no owner, or we do not know yet whether the device has an
- // owner. We write a pending value that will be persisted when ownership is
- // taken (if that has not already happened).
- // We store the new value in the local state, so that even if Chrome is
- // restarted before ownership is taken, we will still persist it eventually.
- // See OnOwnershipTaken.
- VLOG(1) << "Pending owner; setting kPendingPref";
- local_state_->SetBoolean(kPendingPref, enabled);
- NotifyObservers();
- }
+ Set(profile, base::Value(enabled));
}
-bool StatsReportingController::IsEnabled() {
+bool StatsReportingController::IsEnabled() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- bool value = false;
metrics::structured::NeutrinoDevicesLogWithLocalState(
local_state_, metrics::structured::NeutrinoDevicesLocation::kIsEnabled);
- if (ShouldReadFromPendingValue() && GetPendingValue(&value)) {
- // Return the pending value if it exists.
- return value;
- }
- // Otherwise, always return the value from the signed store.
- GetSignedStoredValue(&value);
- return value;
-}
-
-base::CallbackListSubscription StatsReportingController::AddObserver(
- const base::RepeatingClosure& callback) {
- DCHECK(!callback.is_null());
- DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- return callback_list_.Add(callback);
-}
-
-void StatsReportingController::OnOwnershipTaken(
- ownership::OwnerSettingsService* service) {
- DCHECK_EQ(GetOwnershipStatus(), DeviceSettingsService::OWNERSHIP_TAKEN);
- DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- VLOG(1) << "OnOwnershipTaken";
-
- bool pending_value;
- if (GetPendingValue(&pending_value)) {
- // At the time ownership is taken, there is a value waiting to be written.
- // Use the OwnerSettingsService of the new owner to write the setting.
- SetWithServiceAsync(service, pending_value);
- }
-}
-
-void StatsReportingController::OnSignedPolicyStored(bool success) {
- if (!success)
- return;
- bool pending_value;
- bool signed_value;
- if (GetPendingValue(&pending_value) && GetSignedStoredValue(&signed_value) &&
- pending_value == signed_value) {
- is_value_being_set_with_service_ = false;
- owner_settings_service_observation_.Reset();
- ClearPendingValue();
- NotifyObservers();
- if (on_device_settings_stored_callback_)
- std::move(on_device_settings_stored_callback_).Run();
- }
-}
-
-void StatsReportingController::SetOnDeviceSettingsStoredCallBack(
- base::OnceClosure callback) {
- CHECK(!on_device_settings_stored_callback_);
- on_device_settings_stored_callback_ = std::move(callback);
+ absl::optional<base::Value> value = GetValue();
+ return value.has_value() && value->is_bool() && value->GetBool();
}
StatsReportingController::StatsReportingController(PrefService* local_state)
- : local_state_(local_state) {
+ : OwnerPendingSettingController(kStatsReportingPref,
+ kPendingPref,
+ local_state) {
setting_subscription_ = CrosSettings::Get()->AddSettingsObserver(
kStatsReportingPref,
base::BindRepeating(&StatsReportingController::NotifyObservers,
this->as_weak_ptr()));
- value_notified_to_observers_ = IsEnabled();
+ // AddObserver to log NeutrinoDevicesLogWithLocalState every time when
+ // observers are notified.
+ neutrino_logging_subscription_ = AddObserver(base::BindRepeating(
+ &metrics::structured::NeutrinoDevicesLogWithLocalState, local_state_,
+ metrics::structured::NeutrinoDevicesLocation::kNotifyObservers));
}
StatsReportingController::~StatsReportingController() {
@@ -155,107 +87,4 @@
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
}
-void StatsReportingController::SetWithServiceAsync(
- ownership::OwnerSettingsService* service, // Can be null for non-owners.
- bool enabled) {
- VLOG(1) << "SetWithServiceAsync(" << std::boolalpha << enabled << ")";
- bool not_yet_ready = service && !service->IsReady();
- if (not_yet_ready) {
- VLOG(1) << "Service not yet ready. Adding listener.";
- // Service is not yet ready. Listen for changes in its readiness so we can
- // write the value once it is ready. Uses weak pointers, so if everything
- // is shutdown and deleted in the meantime, this callback isn't run.
- service->IsOwnerAsync(
- base::BindOnce(&StatsReportingController::SetWithServiceCallback,
- this->as_weak_ptr(), service->as_weak_ptr(), enabled));
- } else {
- // Service is either null, or ready - use it right now.
- SetWithService(service, enabled);
- }
-}
-
-void StatsReportingController::SetWithServiceCallback(
- const base::WeakPtr<ownership::OwnerSettingsService>& service,
- bool enabled,
- bool is_owner) {
- VLOG(1) << "SetWithServiceCallback(" << std::boolalpha << enabled << ")";
- if (service) // Make sure service wasn't deleted in the meantime.
- SetWithService(service.get(), enabled);
-}
-
-void StatsReportingController::SetWithService(
- ownership::OwnerSettingsService* service, // Can be null for non-owners.
- bool enabled) {
- VLOG(1) << "SetWithService(" << std::boolalpha << enabled << ")";
- if (service && service->IsOwner()) {
- if (!owner_settings_service_observation_.IsObserving())
- owner_settings_service_observation_.Observe(service);
- is_value_being_set_with_service_ = true;
- service->SetBoolean(kStatsReportingPref, enabled);
- local_state_->SetBoolean(kPendingPref, enabled);
- } else {
- // Do nothing since we are not the owner.
- LOG(WARNING) << "Changing settings from non-owner, setting="
- << kStatsReportingPref;
- }
-}
-
-void StatsReportingController::NotifyObservers() {
- DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- bool current_value = IsEnabled();
- VLOG(1) << "Maybe notifying observers of " << std::boolalpha << current_value;
- if (current_value != value_notified_to_observers_) {
- VLOG(1) << "Notifying observers";
- value_notified_to_observers_ = current_value;
- metrics::structured::NeutrinoDevicesLogWithLocalState(
- local_state_,
- metrics::structured::NeutrinoDevicesLocation::kNotifyObservers);
- callback_list_.Notify();
- } else {
- VLOG(1) << "Not notifying (already notified)";
- }
-}
-
-DeviceSettingsService::OwnershipStatus
-StatsReportingController::GetOwnershipStatus() const {
- return DeviceSettingsService::Get()->GetOwnershipStatus();
-}
-
-ownership::OwnerSettingsService*
-StatsReportingController::GetOwnerSettingsService(Profile* profile) {
- return OwnerSettingsServiceAshFactory::GetForBrowserContext(profile);
-}
-
-bool StatsReportingController::GetPendingValue(bool* result) {
- if (local_state_->HasPrefPath(kPendingPref)) {
- *result = local_state_->GetBoolean(kPendingPref);
- return true;
- }
- return false;
-}
-
-void StatsReportingController::ClearPendingValue() {
- VLOG(1) << "ClearPendingValue";
- local_state_->ClearPref(kPendingPref);
-}
-
-bool StatsReportingController::GetSignedStoredValue(bool* result) {
- return CrosSettings::Get()->GetBoolean(kStatsReportingPref, result);
-}
-
-bool StatsReportingController::ShouldReadFromPendingValue() const {
- // Read from pending value before ownership is taken or ownership is
- // unknown. There's a brief moment when ownership is unknown for every
- // Chrome starts. In that case, we will read from pending value if it exists
- // (which means ownership is not taken), and read from service when pending
- // value pending is cleared (which means ownership is taken).
- if (GetOwnershipStatus() == DeviceSettingsService::OWNERSHIP_NONE ||
- GetOwnershipStatus() == DeviceSettingsService::OWNERSHIP_UNKNOWN) {
- return true;
- }
- // Read from pending value if ownership is taken but pending value has not
- // been set successfully with service.
- return is_value_being_set_with_service_;
-}
-
} // namespace ash
diff --git a/chrome/browser/ash/settings/stats_reporting_controller.h b/chrome/browser/ash/settings/stats_reporting_controller.h
index 9a29d47..e2f8f22 100644
--- a/chrome/browser/ash/settings/stats_reporting_controller.h
+++ b/chrome/browser/ash/settings/stats_reporting_controller.h
@@ -5,44 +5,18 @@
#ifndef CHROME_BROWSER_ASH_SETTINGS_STATS_REPORTING_CONTROLLER_H_
#define CHROME_BROWSER_ASH_SETTINGS_STATS_REPORTING_CONTROLLER_H_
-#include "base/callback_list.h"
+#include "chrome/browser/ash/settings/owner_pending_setting_controller.h"
+
#include "base/memory/weak_ptr.h"
-#include "base/scoped_observation.h"
-#include "base/sequence_checker.h"
-#include "chrome/browser/ash/settings/cros_settings.h"
-#include "chrome/browser/ash/settings/device_settings_service.h"
-#include "components/ownership/owner_settings_service.h"
class PrefRegistrySimple;
class PrefService;
-class Profile;
-
-namespace ownership {
-class OwnerSettingsService;
-}
namespace ash {
-// An extra layer on top of CrosSettings / OwnerSettingsService that allows for
-// writing a setting before ownership is taken, for one setting only:
-// kStatsReportingPref, which has the key: "cros.metrics.reportingEnabled".
-//
-// Ordinarily, the OwnerSettingsService interface is used for writing settings,
-// and the CrosSettings interface is used for reading them - but as the OSS
-// cannot be used until the device has an owner, this class can be used instead,
-// since writing the new value with SetEnabled works even before ownership is
-// taken.
-//
-// If OSS is ready then the new value is written straight away, and if not, then
-// a pending write is queued that is completed as soon as the OSS is ready.
-// This write will complete even if Chrome is restarted in the meantime.
-// The caller need not care whether the write was immediate or pending, as long
-// as they also use this class to read the value of kStatsReportingPref.
-// IsEnabled will return the pending value until ownership is taken and the
-// pending value is written - from then on it will return the signed, stored
-// value from CrosSettings.
-class StatsReportingController
- : public ownership::OwnerSettingsService::Observer {
+// Class to control setting of cros.metrics.reportingEnabled device preference
+// before ownership is taken.
+class StatsReportingController : public OwnerPendingSettingController {
public:
// Manage singleton instance.
static void Initialize(PrefService* local_state);
@@ -55,109 +29,18 @@
static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
- // Store the new value of |enabled|. This will happen straight away if
- // |profile| is the owner, and it will cause a pending write to be buffered
- // and written later if the device has no owner yet. It will fail if the
- // device already has an owner, and |profile| is not that owner.
void SetEnabled(Profile* profile, bool enabled);
-
- // Returns the latest value of enabled - regardless of whether this has been
- // successfully signed and persisted, or if it is still stored as a pending
- // write.
- bool IsEnabled();
-
- // Add an observer |callback| for changes to the setting.
- base::CallbackListSubscription AddObserver(
- const base::RepeatingClosure& callback) WARN_UNUSED_RESULT;
-
- // Called once ownership is taken, |service| is the service of the user taking
- // ownership.
- void OnOwnershipTaken(ownership::OwnerSettingsService* service);
-
- // ownership::OwnerSettingsService::Observer implementation:
- void OnSignedPolicyStored(bool success) override;
-
- // Sets the callback which is called once when the |enabled| value is
- // propagated to the device settings. Support only one callback at a time.
- // CHECKs if the second callback is being set.
- // It's different from the |AddObserver| API. Observers are called
- // immediately after |SetEnabled| is called with the different |enabled|
- // setting.
- void SetOnDeviceSettingsStoredCallBack(base::OnceClosure callback);
-
- // Clears any value waiting to be written (from storage in local state).
- void ClearPendingValue();
+ bool IsEnabled() const;
private:
- friend class StatsReportingControllerTest;
-
explicit StatsReportingController(PrefService* local_state);
~StatsReportingController() override;
- // Delegates immediately to SetWithService if |service| is ready, otherwise
- // runs SetWithService asynchronously once |service| is ready.
- void SetWithServiceAsync(ownership::OwnerSettingsService* service,
- bool enabled);
-
- // Callback used by SetWithServiceAsync.
- void SetWithServiceCallback(
- const base::WeakPtr<ownership::OwnerSettingsService>& service,
- bool enabled,
- bool is_owner);
-
- // Uses |service| to write the latest value, as long as |service| belongs
- // to the owner - otherwise just prints a warning.
- void SetWithService(ownership::OwnerSettingsService* service, bool enabled);
-
- // Notifies observers if the value has changed.
- void NotifyObservers();
-
- // Gets the current ownership status - owned, unowned, or unknown.
- DeviceSettingsService::OwnershipStatus GetOwnershipStatus() const;
-
- // Get the owner-settings service for a particular profile. A variety of
- // different results can be returned, depending on the profile.
- // a) A ready-to-use service that we know belongs to the owner.
- // b) A ready-to-use service that we know does NOT belong to the owner.
- // c) A service that is NOT ready-to-use, which MIGHT belong to the owner.
- // d) nullptr (for instance, if |profile| is a guest).
- ownership::OwnerSettingsService* GetOwnerSettingsService(Profile* profile);
-
- // Sets |*value| to the value waiting to be written (stored in local_state),
- // if one exists. Returns false if there is no such value.
- bool GetPendingValue(bool* value);
-
- // Sets |*value| to the value signed and stored in CrosSettings, if one
- // exists. Returns false if there is no such value.
- bool GetSignedStoredValue(bool* value);
-
- // Returns whether pending value should be used when determining the value
- // of `IsEnabled`.
- bool ShouldReadFromPendingValue() const;
-
base::WeakPtr<StatsReportingController> as_weak_ptr() {
return weak_factory_.GetWeakPtr();
}
- SEQUENCE_CHECKER(sequence_checker_);
-
- PrefService* local_state_;
- bool value_notified_to_observers_;
- base::RepeatingClosureList callback_list_;
- base::CallbackListSubscription setting_subscription_;
-
- // Indicates if the setting value is in the process of being set with the
- // service. There is a small period of time needed between start saving the
- // value and before the value is stored correctly in the service. We should
- // not use the setting value from the service if it is still in the process
- // of being saved.
- bool is_value_being_set_with_service_ = false;
-
- base::ScopedObservation<ownership::OwnerSettingsService,
- ownership::OwnerSettingsService::Observer>
- owner_settings_service_observation_{this};
-
- base::OnceClosure on_device_settings_stored_callback_;
+ base::CallbackListSubscription neutrino_logging_subscription_;
base::WeakPtrFactory<StatsReportingController> weak_factory_{this};
};
diff --git a/chrome/browser/ash/settings/stats_reporting_controller_unittest.cc b/chrome/browser/ash/settings/stats_reporting_controller_unittest.cc
index c9283c1..0e17a28 100644
--- a/chrome/browser/ash/settings/stats_reporting_controller_unittest.cc
+++ b/chrome/browser/ash/settings/stats_reporting_controller_unittest.cc
@@ -67,20 +67,25 @@
}
void ExpectThatPendingValueIs(bool expected) {
- bool pending = false;
- EXPECT_TRUE(StatsReportingController::Get()->GetPendingValue(&pending));
- EXPECT_EQ(expected, pending);
+ absl::optional<base::Value> pending =
+ StatsReportingController::Get()->GetPendingValue();
+ EXPECT_TRUE(pending.has_value());
+ EXPECT_TRUE(pending->is_bool());
+ EXPECT_EQ(expected, pending->GetBool());
}
void ExpectThatPendingValueIsNotSet() {
- bool pending = false;
- EXPECT_FALSE(StatsReportingController::Get()->GetPendingValue(&pending));
+ absl::optional<base::Value> pending =
+ StatsReportingController::Get()->GetPendingValue();
+ EXPECT_FALSE(pending.has_value());
}
void ExpectThatSignedStoredValueIs(bool expected) {
- bool stored = false;
- EXPECT_TRUE(StatsReportingController::Get()->GetSignedStoredValue(&stored));
- EXPECT_EQ(expected, stored);
+ absl::optional<base::Value> stored =
+ StatsReportingController::Get()->GetSignedStoredValue();
+ EXPECT_TRUE(stored.has_value());
+ EXPECT_TRUE(stored->is_bool());
+ EXPECT_EQ(expected, stored->GetBool());
}
void OnNotifiedOfChange() {
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index 9a2def9..98556e45 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -215,6 +215,7 @@
#include "ash/components/settings/cros_settings_names.h"
#include "ash/constants/ash_switches.h"
#include "chrome/browser/ash/settings/cros_settings.h"
+#include "chrome/browser/ash/settings/hardware_data_usage_controller.h"
#include "chrome/browser/ash/settings/stats_reporting_controller.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
@@ -904,6 +905,7 @@
#if BUILDFLAG(IS_CHROMEOS_ASH)
ash::CrosSettings::Initialize(local_state);
+ ash::HWDataUsageController::Initialize(local_state);
ash::StatsReportingController::Initialize(local_state);
arc::StabilityMetricsManager::Initialize(local_state);
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
@@ -1886,6 +1888,7 @@
device_event_log::Shutdown();
#if BUILDFLAG(IS_CHROMEOS_ASH)
+ ash::HWDataUsageController::Shutdown();
arc::StabilityMetricsManager::Shutdown();
ash::StatsReportingController::Shutdown();
ash::CrosSettings::Shutdown();
diff --git a/chrome/browser/chromeos/BUILD.gn b/chrome/browser/chromeos/BUILD.gn
index 469bbfd..560b0bc 100644
--- a/chrome/browser/chromeos/BUILD.gn
+++ b/chrome/browser/chromeos/BUILD.gn
@@ -2966,6 +2966,10 @@
"../ash/settings/device_settings_provider.h",
"../ash/settings/device_settings_service.cc",
"../ash/settings/device_settings_service.h",
+ "../ash/settings/hardware_data_usage_controller.cc",
+ "../ash/settings/hardware_data_usage_controller.h",
+ "../ash/settings/owner_pending_setting_controller.cc",
+ "../ash/settings/owner_pending_setting_controller.h",
"../ash/settings/session_manager_operation.cc",
"../ash/settings/session_manager_operation.h",
"../ash/settings/shutdown_policy_forwarder.cc",
diff --git a/chrome/browser/extensions/api/settings_private/prefs_util.cc b/chrome/browser/extensions/api/settings_private/prefs_util.cc
index fdf5198..8ffb21e 100644
--- a/chrome/browser/extensions/api/settings_private/prefs_util.cc
+++ b/chrome/browser/extensions/api/settings_private/prefs_util.cc
@@ -660,6 +660,8 @@
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_allowlist)[::ash::prefs::kLocalStateDevicePeripheralDataAccessEnabled] =
settings_api::PrefType::PREF_TYPE_BOOLEAN;
+ (*s_allowlist)[ash::kRevenEnableDeviceHWDataUsage] =
+ settings_api::PrefType::PREF_TYPE_BOOLEAN;
// Bluetooth & Internet settings.
(*s_allowlist)[ash::kAllowBluetooth] =
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 8834b6ac..079d3be96 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -186,6 +186,7 @@
#include "chrome/browser/ash/net/system_proxy_manager.h"
#include "chrome/browser/ash/platform_keys/key_permissions/key_permissions_manager_impl.h"
#include "chrome/browser/ash/policy/networking/euicc_status_uploader.h"
+#include "chrome/browser/ash/settings/hardware_data_usage_controller.h"
#include "chrome/browser/ash/settings/stats_reporting_controller.h"
#include "chrome/browser/chromeos/extensions/extensions_permissions_tracker.h"
#include "chrome/browser/component_updater/metadata_table_chromeos.h"
@@ -1006,6 +1007,7 @@
ash::device_activity::DeviceActivityController::RegisterPrefs(registry);
chromeos::EnableDebuggingScreenHandler::RegisterPrefs(registry);
chromeos::FastTransitionObserver::RegisterPrefs(registry);
+ ash::HWDataUsageController::RegisterLocalStatePrefs(registry);
ash::KerberosCredentialsManager::RegisterLocalStatePrefs(registry);
ash::KioskAppManager::RegisterPrefs(registry);
ash::KioskCryptohomeRemover::RegisterPrefs(registry);
