Make PrefMember operate on Sequences instead of threads

It's an utility class that's currently only usable on full threads
and with a little effort can be made to only require a Sequence.

Bug: 978036
Change-Id: Iea6c57f81cb59000351534deae1dd445ad35dc03
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1675604
Auto-Submit: Tomasz Śniatowski <tsniatowski@vewd.com>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672499}

components/prefs/pref_member.cc

@@ -9,11 +9,11 @@
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
-#include "base/threading/thread_task_runner_handle.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/value_conversions.h"
 #include "components/prefs/pref_service.h"
 
-using base::SingleThreadTaskRunner;
+using base::SequencedTaskRunner;
 
 namespace subtle {
 
@@ -49,13 +49,13 @@ void PrefMemberBase::Destroy() {
   }
 }
 
-void PrefMemberBase::MoveToThread(
-    scoped_refptr<SingleThreadTaskRunner> task_runner) {
+void PrefMemberBase::MoveToSequence(
+    scoped_refptr<SequencedTaskRunner> task_runner) {
   VerifyValuePrefName();
   // Load the value from preferences if it hasn't been loaded so far.
   if (!internal())
     UpdateValueFromPref(base::Closure());
-  internal()->MoveToThread(std::move(task_runner));
+  internal()->MoveToSequence(std::move(task_runner));
 }
 
 void PrefMemberBase::OnPreferenceChanged(PrefService* service,
@@ -89,14 +89,13 @@ void PrefMemberBase::InvokeUnnamedCallback(const base::Closure& callback,
 }
 
 PrefMemberBase::Internal::Internal()
-    : thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+    : owning_task_runner_(base::SequencedTaskRunnerHandle::Get()),
       is_managed_(false),
-      is_user_modifiable_(false) {
-}
+      is_user_modifiable_(false) {}
 PrefMemberBase::Internal::~Internal() { }
 
-bool PrefMemberBase::Internal::IsOnCorrectThread() const {
-  return thread_task_runner_->BelongsToCurrentThread();
+bool PrefMemberBase::Internal::IsOnCorrectSequence() const {
+  return owning_task_runner_->RunsTasksInCurrentSequence();
 }
 
 void PrefMemberBase::Internal::UpdateValue(base::Value* v,
@@ -105,13 +104,13 @@ void PrefMemberBase::Internal::UpdateValue(base::Value* v,
                                            base::OnceClosure callback) const {
   std::unique_ptr<base::Value> value(v);
   base::ScopedClosureRunner closure_runner(std::move(callback));
-  if (IsOnCorrectThread()) {
+  if (IsOnCorrectSequence()) {
     bool rv = UpdateValueInternal(*value);
     DCHECK(rv);
     is_managed_ = is_managed;
     is_user_modifiable_ = is_user_modifiable;
   } else {
-    bool may_run = thread_task_runner_->PostTask(
+    bool may_run = owning_task_runner_->PostTask(
         FROM_HERE,
         base::BindOnce(&PrefMemberBase::Internal::UpdateValue, this,
                        value.release(), is_managed, is_user_modifiable,
@@ -120,10 +119,10 @@ void PrefMemberBase::Internal::UpdateValue(base::Value* v,
   }
 }
 
-void PrefMemberBase::Internal::MoveToThread(
-    scoped_refptr<SingleThreadTaskRunner> task_runner) {
-  CheckOnCorrectThread();
-  thread_task_runner_ = std::move(task_runner);
+void PrefMemberBase::Internal::MoveToSequence(
+    scoped_refptr<SequencedTaskRunner> task_runner) {
+  CheckOnCorrectSequence();
+  owning_task_runner_ = std::move(task_runner);
 }
 
 bool PrefMemberVectorStringUpdate(const base::Value& value,

components/prefs/pref_member.h

@@ -33,6 +33,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "base/values.h"
 #include "components/prefs/pref_observer.h"
@@ -58,14 +59,14 @@ class COMPONENTS_PREFS_EXPORT PrefMemberBase : public PrefObserver {
     Internal();
 
     // Update the value, either by calling |UpdateValueInternal| directly
-    // or by dispatching to the right thread.
+    // or by dispatching to the right sequence.
     // Takes ownership of |value|.
     void UpdateValue(base::Value* value,
                      bool is_managed,
                      bool is_user_modifiable,
                      base::OnceClosure callback) const;
 
-    void MoveToThread(scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+    void MoveToSequence(scoped_refptr<base::SequencedTaskRunner> task_runner);
 
     // See PrefMember<> for description.
     bool IsManaged() const {
@@ -80,18 +81,16 @@ class COMPONENTS_PREFS_EXPORT PrefMemberBase : public PrefObserver {
     friend class base::RefCountedThreadSafe<Internal>;
     virtual ~Internal();
 
-    void CheckOnCorrectThread() const {
-      DCHECK(IsOnCorrectThread());
-    }
+    void CheckOnCorrectSequence() const { DCHECK(IsOnCorrectSequence()); }
 
    private:
     // This method actually updates the value. It should only be called from
-    // the thread the PrefMember is on.
+    // the sequence the PrefMember is on.
     virtual bool UpdateValueInternal(const base::Value& value) const = 0;
 
-    bool IsOnCorrectThread() const;
+    bool IsOnCorrectSequence() const;
 
-    scoped_refptr<base::SingleThreadTaskRunner> thread_task_runner_;
+    scoped_refptr<base::SequencedTaskRunner> owning_task_runner_;
     mutable bool is_managed_;
     mutable bool is_user_modifiable_;
 
@@ -112,7 +111,7 @@ class COMPONENTS_PREFS_EXPORT PrefMemberBase : public PrefObserver {
   // See PrefMember<> for description.
   void Destroy();
 
-  void MoveToThread(scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+  void MoveToSequence(scoped_refptr<base::SequencedTaskRunner> task_runner);
 
   // PrefObserver
   void OnPreferenceChanged(PrefService* service,
@@ -186,8 +185,8 @@ class PrefMember : public subtle::PrefMemberBase {
 
   // Unsubscribes the PrefMember from the PrefService. After calling this
   // function, the PrefMember may not be used any more on the UI thread.
-  // Assuming |MoveToThread| was previously called, |GetValue|, |IsManaged|,
-  // and |IsUserModifiable| can still be called from the other thread but
+  // Assuming |MoveToSequence| was previously called, |GetValue|, |IsManaged|,
+  // and |IsUserModifiable| can still be called from the other sequence but
   // the results will no longer update from the PrefService.
   // This method should only be called on the UI thread.
   void Destroy() {
@@ -195,19 +194,26 @@ class PrefMember : public subtle::PrefMemberBase {
   }
 
   // Moves the PrefMember to another thread, allowing read accesses from there.
-  // Changes from the PrefService will be propagated asynchronously
+  // Forwarder to MoveToSequence.
+  // TODO(crbug.com/978036): Remove after all callers use MoveToSequence
+  void MoveToThread(scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+    MoveToSequence(task_runner);
+  }
+
+  // Moves the PrefMember to another sequence, allowing read accesses from
+  // there. Changes from the PrefService will be propagated asynchronously
   // via PostTask.
   // This method should only be used from the thread the PrefMember is currently
   // on, which is the UI thread by default.
-  void MoveToThread(scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
-    subtle::PrefMemberBase::MoveToThread(task_runner);
+  void MoveToSequence(scoped_refptr<base::SequencedTaskRunner> task_runner) {
+    subtle::PrefMemberBase::MoveToSequence(task_runner);
   }
 
   // Check whether the pref is managed, i.e. controlled externally through
   // enterprise configuration management (e.g. windows group policy). Returns
   // false for unknown prefs.
   // This method should only be used from the thread the PrefMember is currently
-  // on, which is the UI thread unless changed by |MoveToThread|.
+  // on, which is the UI thread unless changed by |MoveToSequence|.
   bool IsManaged() const {
     VerifyPref();
     return internal_->IsManaged();
@@ -217,7 +223,7 @@ class PrefMember : public subtle::PrefMemberBase {
   // when the pref is managed by a policy or an extension, and when a command
   // line flag overrides the pref.
   // This method should only be used from the thread the PrefMember is currently
-  // on, which is the UI thread unless changed by |MoveToThread|.
+  // on, which is the UI thread unless changed by |MoveToSequence|.
   bool IsUserModifiable() const {
     VerifyPref();
     return internal_->IsUserModifiable();
@@ -225,7 +231,7 @@ class PrefMember : public subtle::PrefMemberBase {
 
   // Retrieve the value of the member variable.
   // This method should only be used from the thread the PrefMember is currently
-  // on, which is the UI thread unless changed by |MoveToThread|.
+  // on, which is the UI thread unless changed by |MoveToSequence|.
   ValueType GetValue() const {
     VerifyPref();
     return internal_->value();
@@ -256,7 +262,7 @@ class PrefMember : public subtle::PrefMemberBase {
     Internal() : value_(ValueType()) {}
 
     ValueType value() {
-      CheckOnCorrectThread();
+      CheckOnCorrectSequence();
       return value_;
     }
 

components/prefs/pref_member_unittest.cc

@@ -8,10 +8,10 @@
 
 #include "base/bind.h"
 #include "base/location.h"
-#include "base/single_thread_task_runner.h"
+#include "base/sequenced_task_runner.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/task/post_task.h"
 #include "base/test/scoped_task_environment.h"
-#include "base/threading/thread.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -35,13 +35,12 @@ void RegisterTestPrefs(PrefRegistrySimple* registry) {
 class GetPrefValueHelper
     : public base::RefCountedThreadSafe<GetPrefValueHelper> {
  public:
-  GetPrefValueHelper() : value_(false), pref_thread_("pref thread") {
-    pref_thread_.Start();
-  }
+  GetPrefValueHelper()
+      : value_(false), task_runner_(base::CreateSequencedTaskRunner({})) {}
 
   void Init(const std::string& pref_name, PrefService* prefs) {
     pref_.Init(pref_name, prefs);
-    pref_.MoveToThread(pref_thread_.task_runner());
+    pref_.MoveToSequence(task_runner_);
   }
 
   void Destroy() {
@@ -51,18 +50,12 @@ class GetPrefValueHelper
   void FetchValue() {
     base::WaitableEvent event(base::WaitableEvent::ResetPolicy::MANUAL,
                               base::WaitableEvent::InitialState::NOT_SIGNALED);
-    ASSERT_TRUE(pref_thread_.task_runner()->PostTask(
+    ASSERT_TRUE(task_runner_->PostTask(
         FROM_HERE,
         base::BindOnce(&GetPrefValueHelper::GetPrefValue, this, &event)));
     event.Wait();
   }
 
-  // The thread must be stopped on the main thread. GetPrefValueHelper being
-  // ref-counted, the destructor can be called from any thread.
-  void StopThread() {
-    pref_thread_.Stop();
-  }
-
   bool value() { return value_; }
 
  private:
@@ -77,7 +70,8 @@ class GetPrefValueHelper
   BooleanPrefMember pref_;
   bool value_;
 
-  base::Thread pref_thread_;  // The thread |pref_| runs on.
+  // The sequence |pref_| runs on.
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
 };
 
 class PrefMemberTestClass {
@@ -306,7 +300,7 @@ TEST_F(PrefMemberTest, NoInit) {
   IntegerPrefMember pref;
 }
 
-TEST_F(PrefMemberTest, MoveToThread) {
+TEST_F(PrefMemberTest, MoveToSequence) {
   TestingPrefServiceSimple prefs;
   scoped_refptr<GetPrefValueHelper> helper(new GetPrefValueHelper());
   RegisterTestPrefs(prefs.registry());
@@ -324,6 +318,4 @@ TEST_F(PrefMemberTest, MoveToThread) {
 
   helper->FetchValue();
   EXPECT_TRUE(helper->value());
-
-  helper->StopThread();
 }