Add seq checks into CloudPolicyStore

Change-Id: I0495931f6b8826f2668b08a95ca06449d0df8cab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2215860
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772216}

chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc

@@ -10,6 +10,7 @@
 #include "base/logging.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/chromeos/policy/device_policy_decoder_chromeos.h"
@@ -66,6 +67,7 @@ DeviceCloudPolicyStoreChromeOS::~DeviceCloudPolicyStoreChromeOS() {
 
 void DeviceCloudPolicyStoreChromeOS::Store(
     const em::PolicyFetchResponse& policy) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // The policy and the public key must have already been loaded by the device
   // settings service.
   DCHECK(is_initialized());
@@ -106,6 +108,8 @@ void DeviceCloudPolicyStoreChromeOS::Store(
 }
 
 void DeviceCloudPolicyStoreChromeOS::Load() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // Cancel all pending requests.
   weak_factory_.InvalidateWeakPtrs();
 
@@ -114,6 +118,8 @@ void DeviceCloudPolicyStoreChromeOS::Load() {
 
 void DeviceCloudPolicyStoreChromeOS::InstallInitialPolicy(
     const em::PolicyFetchResponse& policy) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // Cancel all pending requests.
   weak_factory_.InvalidateWeakPtrs();
 
@@ -134,11 +140,15 @@ void DeviceCloudPolicyStoreChromeOS::InstallInitialPolicy(
 }
 
 void DeviceCloudPolicyStoreChromeOS::DeviceSettingsUpdated() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   if (!weak_factory_.HasWeakPtrs())
     UpdateFromService();
 }
 
 void DeviceCloudPolicyStoreChromeOS::OnDeviceSettingsServiceShutdown() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   device_settings_service_->RemoveObserver(this);
   device_settings_service_ = nullptr;
 }

chrome/browser/chromeos/policy/device_local_account_policy_store.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "chrome/browser/chromeos/policy/value_validation/onc_user_policy_value_validator.h"
 #include "components/ownership/owner_key_util.h"
@@ -40,6 +41,8 @@ DeviceLocalAccountPolicyStore::DeviceLocalAccountPolicyStore(
 DeviceLocalAccountPolicyStore::~DeviceLocalAccountPolicyStore() {}
 
 void DeviceLocalAccountPolicyStore::Load() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // Cancel all pending requests.
   weak_factory_.InvalidateWeakPtrs();
 
@@ -61,6 +64,8 @@ DeviceLocalAccountPolicyStore::CreateValidator(
 }
 
 void DeviceLocalAccountPolicyStore::LoadImmediately() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // This blocking D-Bus call is in the startup path and will block the UI
   // thread. This only happens when the Profile is created synchronously, which
   // on Chrome OS happens whenever the browser is restarted into the same
@@ -83,6 +88,8 @@ void DeviceLocalAccountPolicyStore::LoadImmediately() {
 
 void DeviceLocalAccountPolicyStore::Store(
     const em::PolicyFetchResponse& policy) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // Cancel all pending requests.
   weak_factory_.InvalidateWeakPtrs();
 

chrome/browser/chromeos/policy/device_local_account_policy_store.h

@@ -39,7 +39,10 @@ class DeviceLocalAccountPolicyStore : public UserCloudPolicyStoreBase {
       scoped_refptr<base::SequencedTaskRunner> background_task_runner);
   ~DeviceLocalAccountPolicyStore() override;
 
-  const std::string& account_id() const { return account_id_; }
+  const std::string& account_id() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return account_id_;
+  }
 
   // CloudPolicyStore:
   void Store(const enterprise_management::PolicyFetchResponse& policy) override;

chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos.cc

@@ -11,6 +11,7 @@
 #include "base/debug/dump_without_crashing.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "chrome/browser/chromeos/policy/cached_policy_key_loader_chromeos.h"
 #include "chrome/browser/chromeos/policy/value_validation/onc_user_policy_value_validator.h"
@@ -58,6 +59,7 @@ UserCloudPolicyStoreChromeOS::~UserCloudPolicyStoreChromeOS() {}
 
 void UserCloudPolicyStoreChromeOS::Store(
     const em::PolicyFetchResponse& policy) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!is_active_directory_);
 
   // Cancel all pending requests.
@@ -71,6 +73,8 @@ void UserCloudPolicyStoreChromeOS::Store(
 }
 
 void UserCloudPolicyStoreChromeOS::Load() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // Cancel all pending requests.
   weak_factory_.InvalidateWeakPtrs();
 
@@ -91,6 +95,8 @@ UserCloudPolicyStoreChromeOS::CreateValidator(
 }
 
 void UserCloudPolicyStoreChromeOS::LoadImmediately() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // This blocking D-Bus call is in the startup path and will block the UI
   // thread. This only happens when the Profile is created synchronously, which
   // on Chrome OS happens whenever the browser is restarted into the same

components/policy/core/common/cloud/cloud_policy_store.cc

@@ -18,10 +18,13 @@ CloudPolicyStore::CloudPolicyStore()
       is_initialized_(false) {}
 
 CloudPolicyStore::~CloudPolicyStore() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!external_data_manager_);
 }
 
 bool CloudPolicyStore::is_managed() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   return policy_.get() &&
          policy_->state() == enterprise_management::PolicyData::ACTIVE;
 }
@@ -29,15 +32,21 @@ bool CloudPolicyStore::is_managed() const {
 void CloudPolicyStore::Store(
     const enterprise_management::PolicyFetchResponse& policy,
     int64_t invalidation_version) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   invalidation_version_ = invalidation_version;
   Store(policy);
 }
 
 void CloudPolicyStore::AddObserver(CloudPolicyStore::Observer* observer) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   observers_.AddObserver(observer);
 }
 
 void CloudPolicyStore::RemoveObserver(CloudPolicyStore::Observer* observer) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   observers_.RemoveObserver(observer);
 }
 
@@ -61,13 +70,17 @@ void CloudPolicyStore::NotifyStoreError() {
 
 void CloudPolicyStore::SetExternalDataManager(
     base::WeakPtr<CloudExternalDataManager> external_data_manager) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!external_data_manager_);
+
   external_data_manager_ = external_data_manager;
   if (is_initialized_)
     external_data_manager_->OnPolicyStoreLoaded();
 }
 
 void CloudPolicyStore::SetPolicyMapForTesting(const PolicyMap& policy_map) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   policy_map_.CopyFrom(policy_map);
   NotifyStoreLoaded();
 }

components/policy/core/common/cloud/cloud_policy_store.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/sequence_checker.h"
 #include "components/policy/core/common/cloud/cloud_policy_validator.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/policy_export.h"
@@ -66,29 +67,44 @@ class POLICY_EXPORT CloudPolicyStore {
 
   // Indicates whether the store has been fully initialized. This is
   // accomplished by calling Load() after startup.
-  bool is_initialized() const { return is_initialized_; }
+  bool is_initialized() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return is_initialized_;
+  }
 
   base::WeakPtr<CloudExternalDataManager> external_data_manager() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return external_data_manager_;
   }
 
-  const PolicyMap& policy_map() const { return policy_map_; }
+  const PolicyMap& policy_map() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return policy_map_;
+  }
   bool has_policy() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return policy_.get() != NULL;
   }
   const enterprise_management::PolicyData* policy() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return policy_.get();
   }
   bool is_managed() const;
-  Status status() const { return status_; }
+  Status status() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return status_;
+  }
   CloudPolicyValidatorBase::Status validation_status() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return validation_result_.get() ? validation_result_->status
                                     : CloudPolicyValidatorBase::VALIDATION_OK;
   }
   const CloudPolicyValidatorBase::ValidationResult* validation_result() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return validation_result_.get();
   }
   const std::string& policy_signature_public_key() const {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     return policy_signature_public_key_;
   }
 
@@ -119,7 +135,10 @@ class POLICY_EXPORT CloudPolicyStore {
 
   // The invalidation version of the last policy stored. This value can be read
   // by observers to determine which version of the policy is now available.
-  int64_t invalidation_version() { return invalidation_version_; }
+  int64_t invalidation_version() {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return invalidation_version_;
+  }
 
   // Indicate that external data referenced by policies in this store is managed
   // by |external_data_manager|. The |external_data_manager| will be notified
@@ -140,6 +159,9 @@ class POLICY_EXPORT CloudPolicyStore {
   void NotifyStoreLoaded();
   void NotifyStoreError();
 
+  // Assert non-concurrent usage in debug builds.
+  SEQUENCE_CHECKER(sequence_checker_);
+
   // Manages external data referenced by policies.
   base::WeakPtr<CloudExternalDataManager> external_data_manager_;
 

components/policy/core/common/cloud/user_cloud_policy_store.cc

@@ -13,6 +13,7 @@
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "base/task_runner_util.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
@@ -111,6 +112,8 @@ DesktopCloudPolicyStore::DesktopCloudPolicyStore(
 DesktopCloudPolicyStore::~DesktopCloudPolicyStore() {}
 
 void DesktopCloudPolicyStore::LoadImmediately() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   DVLOG(1) << "Initiating immediate policy load from disk";
   // Cancel any pending Load/Store/Validate operations.
   weak_factory_.InvalidateWeakPtrs();
@@ -122,6 +125,8 @@ void DesktopCloudPolicyStore::LoadImmediately() {
 }
 
 void DesktopCloudPolicyStore::Clear() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   background_task_runner()->PostTask(
       FROM_HERE, base::BindOnce(base::IgnoreResult(&base::DeleteFile),
                                 policy_path_, false));
@@ -136,6 +141,8 @@ void DesktopCloudPolicyStore::Clear() {
 }
 
 void DesktopCloudPolicyStore::Load() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   DVLOG(1) << "Initiating policy load from disk";
   // Cancel any pending Load/Store/Validate operations.
   weak_factory_.InvalidateWeakPtrs();
@@ -345,6 +352,8 @@ void DesktopCloudPolicyStore::InstallLoadedPolicyAfterValidation(
 }
 
 void DesktopCloudPolicyStore::Store(const em::PolicyFetchResponse& policy) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // Cancel all pending requests.
   weak_factory_.InvalidateWeakPtrs();
 

components/policy/core/common/cloud/user_cloud_policy_store_base.h

@@ -32,7 +32,10 @@ class POLICY_EXPORT UserCloudPolicyStoreBase : public CloudPolicyStore {
       PolicySource policy_source);
   ~UserCloudPolicyStoreBase() override;
 
-  PolicySource source() { return policy_source_; }
+  PolicySource source() {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return policy_source_;
+  }
 
  protected:
   // Creates a validator configured to validate a user policy. The caller owns