Initialize AccelerometerReader using a SequencedTaskRunner

Change the initialization of the AccelerometerReader to occur using a SequencedTaskRunner. Updated the subsequent reads of the accelerometer file to also use the same SequencedTaskRunner.
This way reading and processing accelerometer values doesn't affect UI.

It already notifies observers on their own threads. So items which need to
update UI receive the updates on the UI thread.

TEST=manual testing on device
BUG=461433

Review URL: https://codereview.chromium.org/1174193002

Cr-Commit-Position: refs/heads/master@{#335711}

3 files changed, 189 insertions, 154 deletions

diff --git a/chrome/browser/ui/ash/ash_init.cc b/chrome/browser/ui/ash/ash_init.cc
index 5e4229e..319457b 100644
--- a/chrome/browser/ui/ash/ash_init.cc
+++ b/chrome/browser/ui/ash/ash_init.cc
@@ -67,7 +67,8 @@ void OpenAsh(gfx::AcceleratedWidget remote_window) {
   // TODO(flackr): Investigate exposing a blocking pool task runner to chromeos.
   chromeos::AccelerometerReader::GetInstance()->Initialize(
       content::BrowserThread::GetBlockingPool()
-          ->GetTaskRunnerWithShutdownBehavior(
+          ->GetSequencedTaskRunnerWithShutdownBehavior(
+              content::BrowserThread::GetBlockingPool()->GetSequenceToken(),
               base::SequencedWorkerPool::SKIP_ON_SHUTDOWN));
   shell->accelerator_controller()->SetImeControlDelegate(
       scoped_ptr<ash::ImeControlDelegate>(new ImeController).Pass());
diff --git a/chromeos/accelerometer/accelerometer_reader.cc b/chromeos/accelerometer/accelerometer_reader.cc
index bca962f..b238e75 100644
--- a/chromeos/accelerometer/accelerometer_reader.cc
+++ b/chromeos/accelerometer/accelerometer_reader.cc
@@ -17,6 +17,7 @@
 #include "base/task_runner.h"
 #include "base/task_runner_util.h"
 #include "base/thread_task_runner_handle.h"
+#include "base/threading/platform_thread.h"
 #include "base/threading/sequenced_worker_pool.h"
 
 namespace chromeos {
@@ -56,6 +57,15 @@ const size_t kDataSize = 2;
 // The mean acceleration due to gravity on Earth in m/s^2.
 const float kMeanGravity = 9.80665f;
 
+// The number of accelerometers.
+const int kNumberOfAccelerometers = 2;
+
+// The number of axes for which there are acceleration readings.
+const int kNumberOfAxes = 3;
+
+// The size of data in one reading of the accelerometers.
+const int kSizeOfReading = kDataSize * kNumberOfAccelerometers * kNumberOfAxes;
+
 // Reads |path| to the unsigned int pointed to by |value|. Returns true on
 // success or false on failure.
 bool ReadFileToInt(const base::FilePath& path, int* value) {
@@ -72,20 +82,106 @@ bool ReadFileToInt(const base::FilePath& path, int* value) {
   return true;
 }
 
-bool DetectAndReadAccelerometerConfiguration(
-    scoped_refptr<AccelerometerReader::Configuration> configuration) {
+}  // namespace
+
+const int AccelerometerReader::kDelayBetweenReadsMs = 100;
+
+// Work that runs on a base::TaskRunner. It determines the accelerometer
+// configuartion, and reads the data. Upon a successful read it will notify
+// all observers.
+class AccelerometerFileReader
+    : public base::RefCountedThreadSafe<AccelerometerFileReader> {
+ public:
+  AccelerometerFileReader();
+
+  // Detects the accelerometer configuration, if an accelerometer is available
+  // triggers reads.
+  void Initialize(
+      scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner);
+
+  // Attempts to read the accelerometer data. Upon a success, converts the raw
+  // reading to an AccelerometerUpdate and notifies observers. Triggers another
+  // read at the current sampling rate.
+  void Read();
+
+  // Add/Remove observers.
+  void AddObserver(AccelerometerReader::Observer* observer);
+  void RemoveObserver(AccelerometerReader::Observer* observer);
+
+ private:
+  friend class base::RefCountedThreadSafe<AccelerometerFileReader>;
+
+  // Configuration structure for accelerometer device.
+  struct ConfigurationData {
+    ConfigurationData();
+    ~ConfigurationData();
+
+    // Number of accelerometers on device.
+    size_t count;
+
+    // Length of accelerometer updates.
+    size_t length;
+
+    // Which accelerometers are present on device.
+    bool has[ACCELEROMETER_SOURCE_COUNT];
+
+    // Scale of accelerometers (i.e. raw value * scale = m/s^2).
+    float scale[ACCELEROMETER_SOURCE_COUNT][3];
+
+    // Index of each accelerometer axis in data stream.
+    int index[ACCELEROMETER_SOURCE_COUNT][3];
+  };
+
+  ~AccelerometerFileReader() {}
+
+  // Attempts to read the accelerometer data. Upon a success, converts the raw
+  // reading to an AccelerometerUpdate and notifies observers.
+  void ReadFileAndNotify();
+
+  // True if Initialize completed successfully, and there is an accelerometer
+  // file to read.
+  bool initialization_successful_;
+
+  // The accelerometer configuration.
+  ConfigurationData configuration_;
+
+  // The observers to notify of accelerometer updates.
+  scoped_refptr<base::ObserverListThreadSafe<AccelerometerReader::Observer>>
+      observers_;
+
+  // The task runner to use for blocking tasks.
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  // The last seen accelerometer data.
+  scoped_refptr<AccelerometerUpdate> update_;
+
+  DISALLOW_COPY_AND_ASSIGN(AccelerometerFileReader);
+};
+
+AccelerometerFileReader::AccelerometerFileReader()
+    : initialization_successful_(false),
+      observers_(
+          new base::ObserverListThreadSafe<AccelerometerReader::Observer>()) {
+}
+
+void AccelerometerFileReader::Initialize(
+    scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner) {
+  DCHECK(
+      base::SequencedWorkerPool::GetSequenceTokenForCurrentThread().IsValid());
+  task_runner_ = sequenced_task_runner;
+
   // Check for accelerometer symlink which will be created by the udev rules
   // file on detecting the device.
   base::FilePath device;
   if (!base::ReadSymbolicLink(base::FilePath(kAccelerometerDevicePath),
                               &device)) {
-    return false;
+    return;
   }
 
   if (!base::PathExists(base::FilePath(kAccelerometerTriggerPath))) {
     LOG(ERROR) << "Accelerometer trigger does not exist at"
                << kAccelerometerTriggerPath;
-    return false;
+    return;
   }
 
   base::FilePath iio_path(base::FilePath(kAccelerometerIioBasePath).Append(
@@ -99,78 +195,116 @@ bool DetectAndReadAccelerometerConfiguration(
     int scale_divisor;
     if (!ReadFileToInt(iio_path.Append(accelerometer_scale_path.c_str()),
         &scale_divisor)) {
-      configuration->data.has[i] = false;
+      configuration_.has[i] = false;
       continue;
     }
 
-    configuration->data.has[i] = true;
-    configuration->data.count++;
+    configuration_.has[i] = true;
+    configuration_.count++;
     for (size_t j = 0; j < arraysize(kAccelerometerAxes); ++j) {
-      configuration->data.scale[i][j] = kMeanGravity / scale_divisor;
+      configuration_.scale[i][j] = kMeanGravity / scale_divisor;
       std::string accelerometer_index_path = base::StringPrintf(
           kAccelerometerScanIndexPath, kAccelerometerAxes[j],
           kAccelerometerNames[i]);
       if (!ReadFileToInt(iio_path.Append(accelerometer_index_path.c_str()),
-                         &(configuration->data.index[i][j]))) {
-        return false;
+                         &(configuration_.index[i][j]))) {
+        return;
       }
     }
   }
 
   // Adjust the directions of accelerometers to match the AccelerometerUpdate
   // type specified in chromeos/accelerometer/accelerometer_types.h.
-  configuration->data.scale[ACCELEROMETER_SOURCE_SCREEN][0] *= -1.0f;
+  configuration_.scale[ACCELEROMETER_SOURCE_SCREEN][0] *= -1.0f;
   for (int i = 0; i < 3; ++i) {
-    configuration->data.scale[ACCELEROMETER_SOURCE_ATTACHED_KEYBOARD][i] *=
-        -1.0f;
+    configuration_.scale[ACCELEROMETER_SOURCE_ATTACHED_KEYBOARD][i] *= -1.0f;
   }
 
   // Verify indices are within bounds.
   for (int i = 0; i < ACCELEROMETER_SOURCE_COUNT; ++i) {
-    if (!configuration->data.has[i])
+    if (!configuration_.has[i])
       continue;
     for (int j = 0; j < 3; ++j) {
-      if (configuration->data.index[i][j] < 0 ||
-          configuration->data.index[i][j] >=
-              3 * static_cast<int>(configuration->data.count)) {
+      if (configuration_.index[i][j] < 0 ||
+          configuration_.index[i][j] >=
+              3 * static_cast<int>(configuration_.count)) {
         LOG(ERROR) << "Field index for " << kAccelerometerNames[i] << " "
                    << kAccelerometerAxes[j] << " axis out of bounds.";
-        return false;
+        return;
       }
     }
   }
-  configuration->data.length = kDataSize * 3 * configuration->data.count;
-  return true;
+  configuration_.length = kDataSize * 3 * configuration_.count;
+  initialization_successful_ = true;
+  Read();
+}
+
+void AccelerometerFileReader::Read() {
+  DCHECK(
+      base::SequencedWorkerPool::GetSequenceTokenForCurrentThread().IsValid());
+  ReadFileAndNotify();
+  task_runner_->PostNonNestableDelayedTask(
+      FROM_HERE, base::Bind(&AccelerometerFileReader::Read, this),
+      base::TimeDelta::FromMilliseconds(
+          AccelerometerReader::kDelayBetweenReadsMs));
 }
 
-bool ReadAccelerometer(
-    scoped_refptr<AccelerometerReader::Reading> reading,
-    size_t length) {
+void AccelerometerFileReader::AddObserver(
+    AccelerometerReader::Observer* observer) {
+  observers_->AddObserver(observer);
+  if (initialization_successful_) {
+    task_runner_->PostNonNestableTask(
+        FROM_HERE,
+        base::Bind(&AccelerometerFileReader::ReadFileAndNotify, this));
+  }
+}
+
+void AccelerometerFileReader::RemoveObserver(
+    AccelerometerReader::Observer* observer) {
+  observers_->RemoveObserver(observer);
+}
+
+void AccelerometerFileReader::ReadFileAndNotify() {
+  DCHECK(initialization_successful_);
+  char reading[kSizeOfReading];
+
   // Initiate the trigger to read accelerometers simultaneously
   int bytes_written = base::WriteFile(
       base::FilePath(kAccelerometerTriggerPath), "1\n", 2);
   if (bytes_written < 2) {
     PLOG(ERROR) << "Accelerometer trigger failure: " << bytes_written;
-    return false;
+    return;
   }
 
   // Read resulting sample from /dev/cros-ec-accel.
   int bytes_read = base::ReadFile(base::FilePath(kAccelerometerDevicePath),
-                                  reading->data, length);
-  if (bytes_read < static_cast<int>(length)) {
+                                  reading, configuration_.length);
+  if (bytes_read < static_cast<int>(configuration_.length)) {
     LOG(ERROR) << "Read " << bytes_read << " byte(s), expected "
-               << length << " bytes from accelerometer";
-    return false;
+               << configuration_.length << " bytes from accelerometer";
+    return;
   }
-  return true;
-}
 
-}  // namespace
+  update_ = new AccelerometerUpdate();
 
-const int AccelerometerReader::kDelayBetweenReadsMs = 100;
+  for (int i = 0; i < ACCELEROMETER_SOURCE_COUNT; ++i) {
+    if (!configuration_.has[i])
+      continue;
 
-AccelerometerReader::ConfigurationData::ConfigurationData()
-    : count(0) {
+    int16* values = reinterpret_cast<int16*>(reading);
+    update_->Set(
+        static_cast<AccelerometerSource>(i),
+        values[configuration_.index[i][0]] * configuration_.scale[i][0],
+        values[configuration_.index[i][1]] * configuration_.scale[i][1],
+        values[configuration_.index[i][2]] * configuration_.scale[i][2]);
+  }
+
+  observers_->Notify(FROM_HERE,
+                     &AccelerometerReader::Observer::OnAccelerometerUpdated,
+                     update_);
+}
+
+AccelerometerFileReader::ConfigurationData::ConfigurationData() : count(0) {
   for (int i = 0; i < ACCELEROMETER_SOURCE_COUNT; ++i) {
     has[i] = false;
     for (int j = 0; j < 3; ++j) {
@@ -180,7 +314,7 @@ AccelerometerReader::ConfigurationData::ConfigurationData()
   }
 }
 
-AccelerometerReader::ConfigurationData::~ConfigurationData() {
+AccelerometerFileReader::ConfigurationData::~ConfigurationData() {
 }
 
 // static
@@ -189,87 +323,29 @@ AccelerometerReader* AccelerometerReader::GetInstance() {
 }
 
 void AccelerometerReader::Initialize(
-    scoped_refptr<base::TaskRunner> blocking_task_runner) {
-  DCHECK(blocking_task_runner.get());
-  task_runner_ = blocking_task_runner;
+    scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner) {
+  DCHECK(sequenced_task_runner.get());
   // Asynchronously detect and initialize the accelerometer to avoid delaying
   // startup.
-  base::PostTaskAndReplyWithResult(
-      task_runner_.get(), FROM_HERE,
-      base::Bind(&DetectAndReadAccelerometerConfiguration, configuration_),
-      base::Bind(&AccelerometerReader::OnInitialized,
-                 weak_factory_.GetWeakPtr(), configuration_));
+  sequenced_task_runner->PostNonNestableTask(
+      FROM_HERE,
+      base::Bind(&AccelerometerFileReader::Initialize,
+                 accelerometer_file_reader_.get(), sequenced_task_runner));
 }
 
 void AccelerometerReader::AddObserver(Observer* observer) {
-  observers_->AddObserver(observer);
-  if (update_)
-    observer->OnAccelerometerUpdated(update_.get());
+  accelerometer_file_reader_->AddObserver(observer);
 }
 
 void AccelerometerReader::RemoveObserver(Observer* observer) {
-  observers_->RemoveObserver(observer);
+  accelerometer_file_reader_->RemoveObserver(observer);
 }
 
 AccelerometerReader::AccelerometerReader()
-    : configuration_(new AccelerometerReader::Configuration()),
-      observers_(new base::ObserverListThreadSafe<Observer>()),
-      weak_factory_(this) {
+    : accelerometer_file_reader_(new AccelerometerFileReader()) {
 }
 
 AccelerometerReader::~AccelerometerReader() {
 }
 
-void AccelerometerReader::OnInitialized(
-    scoped_refptr<AccelerometerReader::Configuration> configuration,
-    bool success) {
-  if (success)
-    TriggerRead();
-}
-
-void AccelerometerReader::TriggerRead() {
-  DCHECK(!task_runner_->RunsTasksOnCurrentThread());
-
-  scoped_refptr<AccelerometerReader::Reading> reading(
-      new AccelerometerReader::Reading());
-  base::PostTaskAndReplyWithResult(task_runner_.get(),
-                                   FROM_HERE,
-                                   base::Bind(&ReadAccelerometer, reading,
-                                              configuration_->data.length),
-                                   base::Bind(&AccelerometerReader::OnDataRead,
-                                              weak_factory_.GetWeakPtr(),
-                                              reading));
-}
-
-void AccelerometerReader::OnDataRead(
-    scoped_refptr<AccelerometerReader::Reading> reading,
-    bool success) {
-  DCHECK(!task_runner_->RunsTasksOnCurrentThread());
-
-  if (success) {
-    update_ = new AccelerometerUpdate();
-    for (int i = 0; i < ACCELEROMETER_SOURCE_COUNT; ++i) {
-      if (!configuration_->data.has[i])
-        continue;
-
-      int16* values = reinterpret_cast<int16*>(reading->data);
-      update_->Set(static_cast<AccelerometerSource>(i),
-                   values[configuration_->data.index[i][0]] *
-                       configuration_->data.scale[i][0],
-                   values[configuration_->data.index[i][1]] *
-                       configuration_->data.scale[i][1],
-                   values[configuration_->data.index[i][2]] *
-                       configuration_->data.scale[i][2]);
-    }
-    // TODO(jonross): move this to the blocking thread (crbug.com/461433)
-    observers_->Notify(FROM_HERE, &Observer::OnAccelerometerUpdated, update_);
-  }
-
-  // Trigger another read after the current sampling delay.
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&AccelerometerReader::TriggerRead, weak_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(kDelayBetweenReadsMs));
-}
-
 }  // namespace chromeos
diff --git a/chromeos/accelerometer/accelerometer_reader.h b/chromeos/accelerometer/accelerometer_reader.h
index f0e385b..8ec4ec8 100644
--- a/chromeos/accelerometer/accelerometer_reader.h
+++ b/chromeos/accelerometer/accelerometer_reader.h
@@ -6,7 +6,6 @@
 #define CHROMEOS_ACCELEROMETER_ACCELEROMETER_READER_H_
 
 #include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
 #include "base/observer_list_threadsafe.h"
 #include "chromeos/accelerometer/accelerometer_types.h"
 #include "chromeos/chromeos_export.h"
@@ -15,11 +14,13 @@ template <typename T>
 struct DefaultSingletonTraits;
 
 namespace base {
-class TaskRunner;
+class SequencedTaskRunner;
 }
 
 namespace chromeos {
 
+class AccelerometerFileReader;
+
 // Reads an accelerometer device and reports data back to an
 // AccelerometerDelegate.
 class CHROMEOS_EXPORT AccelerometerReader {
@@ -27,29 +28,6 @@ class CHROMEOS_EXPORT AccelerometerReader {
   // The time to wait between reading the accelerometer.
   static const int kDelayBetweenReadsMs;
 
-  // Configuration structure for accelerometer device.
-  struct ConfigurationData {
-    ConfigurationData();
-    ~ConfigurationData();
-
-    // Number of accelerometers on device.
-    size_t count;
-
-    // Length of accelerometer updates.
-    size_t length;
-
-    // Which accelerometers are present on device.
-    bool has[ACCELEROMETER_SOURCE_COUNT];
-
-    // Scale of accelerometers (i.e. raw value * scale = m/s^2).
-    float scale[ACCELEROMETER_SOURCE_COUNT][3];
-
-    // Index of each accelerometer axis in data stream.
-    int index[ACCELEROMETER_SOURCE_COUNT][3];
-  };
-  typedef base::RefCountedData<ConfigurationData> Configuration;
-  typedef base::RefCountedData<char[12]> Reading;
-
   // An interface to receive data from the AccelerometerReader.
   class Observer {
    public:
@@ -62,7 +40,8 @@ class CHROMEOS_EXPORT AccelerometerReader {
 
   static AccelerometerReader* GetInstance();
 
-  void Initialize(scoped_refptr<base::TaskRunner> blocking_task_runner);
+  void Initialize(
+      scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner);
 
   // Add/Remove observers.
   void AddObserver(Observer* observer);
@@ -75,31 +54,10 @@ class CHROMEOS_EXPORT AccelerometerReader {
  private:
   friend struct DefaultSingletonTraits<AccelerometerReader>;
 
-  // Dispatched when initialization is complete. If |success|, |configuration|
-  // provides the details of the detected accelerometer.
-  void OnInitialized(scoped_refptr<Configuration> configuration, bool success);
-
-  // Triggers an asynchronous read from the accelerometer, signalling
-  // OnDataRead with the result.
-  void TriggerRead();
-
-  // If |success|, converts the raw reading to an AccelerometerUpdate
-  // message and notifies the |delegate_| with the new readings.
-  // Triggers another read from the accelerometer at the current sampling rate.
-  void OnDataRead(scoped_refptr<Reading> reading, bool success);
-
-  // The task runner to use for blocking tasks.
-  scoped_refptr<base::TaskRunner> task_runner_;
-
-  // The last seen accelerometer data.
-  scoped_refptr<AccelerometerUpdate> update_;
-
-  // The accelerometer configuration.
-  scoped_refptr<Configuration> configuration_;
-
-  scoped_refptr<base::ObserverListThreadSafe<Observer>> observers_;
-
-  base::WeakPtrFactory<AccelerometerReader> weak_factory_;
+  // Worker that will run on the base::SequencedTaskRunner provided to
+  // Initialize. It will determine accelerometer configuration, read the data,
+  // and notify observers.
+  scoped_refptr<AccelerometerFileReader> accelerometer_file_reader_;
 
   DISALLOW_COPY_AND_ASSIGN(AccelerometerReader);
 };