Refactors DeviceOrientation to make it more extensible

content/browser/device_orientation/provider_impl.cc

Index: content/browser/device_orientation/provider_impl.cc
diff --git a/content/browser/device_orientation/provider_impl.cc b/content/browser/device_orientation/provider_impl.cc
index fab78a11a42733926495eaa1902bc52d879a21bb..acde10691cc0a4593064799aab0d9ef29d30e5be 100644
--- a/content/browser/device_orientation/provider_impl.cc
+++ b/content/browser/device_orientation/provider_impl.cc
@@ -2,7 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <cmath>
+#include "content/browser/device_orientation/provider_impl.h"
+
 #include <set>
 #include <vector>
 
@@ -12,7 +13,6 @@
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "content/browser/device_orientation/orientation.h"
-#include "content/browser/device_orientation/provider_impl.h"
 
 namespace device_orientation {
 
@@ -29,11 +29,21 @@ ProviderImpl::~ProviderImpl() {
 void ProviderImpl::AddObserver(Observer* observer) {
   DCHECK(MessageLoop::current() == creator_loop_);
 
+  DeviceData::Type type = observer->device_data_type();
+
   observers_.insert(observer);
   if (observers_.size() == 1)
-    Start();
-  else
-    observer->OnOrientationUpdate(last_notification_);
+    Start(type);
+  else {
+    // Notify observer of most recent notification if one exists.
+    typedef std::map<DeviceData::Type, scoped_refptr<const DeviceData> >::
+        const_iterator Iterator;
+    Iterator iter = last_notifications_map_.find(type);
+    if (iter != last_notifications_map_.end())
+      observer->OnDeviceDataUpdate((*iter).second, type);
+  }
+
+  ScheduleDoAddPollingDataType(type);
 }
 
 void ProviderImpl::RemoveObserver(Observer* observer) {
@@ -44,17 +54,22 @@ void ProviderImpl::RemoveObserver(Observer* observer) {
     Stop();
 }
 
-void ProviderImpl::Start() {
+void ProviderImpl::Start(DeviceData::Type type) {
   DCHECK(MessageLoop::current() == creator_loop_);
   DCHECK(!polling_thread_.get());
 
-  polling_thread_.reset(new base::Thread("Device orientation polling thread"));
+  // Reset members used on polling thread.
+  last_device_data_map_.reset(new std::map<DeviceData::Type,
+                              scoped_refptr<const DeviceData> >);
+  polling_data_types_.clear();
+
+  polling_thread_.reset(new base::Thread("Device data polling thread"));
   if (!polling_thread_->Start()) {
-    LOG(ERROR) << "Failed to start device orientation polling thread";
+    LOG(ERROR) << "Failed to start device data polling thread";
     polling_thread_.reset();
     return;
   }
-  ScheduleInitializePollingThread();
+  ScheduleInitializePollingThread(type);
 }
 
 void ProviderImpl::Stop() {
@@ -67,24 +82,45 @@ void ProviderImpl::Stop() {
   data_fetcher_.reset();
 }
 
+void ProviderImpl::DoAddPollingDataType(DeviceData::Type type) {
+  DCHECK(MessageLoop::current() == polling_thread_->message_loop());
+
+  polling_data_types_.insert(type);
+}
+
+void ProviderImpl::ScheduleDoAddPollingDataType(DeviceData::Type type) {
+  DCHECK(MessageLoop::current() == creator_loop_);
+
+  MessageLoop* polling_loop = polling_thread_->message_loop();
+  polling_loop->PostTask(FROM_HERE,
+                         base::Bind(&ProviderImpl::DoAddPollingDataType,
+                                    this,
+                                    type));
+}
+
 void ProviderImpl::DoInitializePollingThread(
-    const std::vector<DataFetcherFactory>& factories) {
+    const std::vector<DataFetcherFactory>& factories,
+    DeviceData::Type device_data_type) {
   DCHECK(MessageLoop::current() == polling_thread_->message_loop());
 
   typedef std::vector<DataFetcherFactory>::const_iterator Iterator;
-  for (Iterator i = factories_.begin(), e = factories_.end(); i != e; ++i) {
+  for (Iterator i = factories_.begin(); i != factories_.end(); ++i) {
     DataFetcherFactory factory = *i;
     scoped_ptr<DataFetcher> fetcher(factory());
-    Orientation orientation;
 
-    if (fetcher.get() && fetcher->GetOrientation(&orientation)) {
+    if (!fetcher.get())
+      continue;
+
+    scoped_refptr<const DeviceData> device_data(fetcher->GetDeviceData(
+        device_data_type));
+    if (device_data != NULL) {
       // Pass ownership of fetcher to provider_.
       data_fetcher_.swap(fetcher);
-      last_orientation_ = orientation;
+      last_device_data_map_->insert(std::pair<DeviceData::Type,
+          scoped_refptr<const DeviceData> >(device_data_type, device_data));
 
       // Notify observers.
-      if (!orientation.is_empty())
-        ScheduleDoNotify(orientation);
+      ScheduleDoNotify(device_data, device_data_type);
 
       // Start polling.
       ScheduleDoPoll();
@@ -92,58 +128,94 @@ void ProviderImpl::DoInitializePollingThread(
     }
   }
 
-  // When no orientation data can be provided.
-  ScheduleDoNotify(Orientation::Empty());
+  // When no device data can be provided.
+  ScheduleDoNotify(NULL, device_data_type);
 }
 
-void ProviderImpl::ScheduleInitializePollingThread() {
+void ProviderImpl::ScheduleInitializePollingThread(
+    DeviceData::Type device_data_type) {
   DCHECK(MessageLoop::current() == creator_loop_);
 
   MessageLoop* polling_loop = polling_thread_->message_loop();
   polling_loop->PostTask(FROM_HERE,
                          base::Bind(&ProviderImpl::DoInitializePollingThread,
                                     this,
-                                    factories_));
+                                    factories_,
+                                    device_data_type));
 }
 
-void ProviderImpl::DoNotify(const Orientation& orientation) {
+void ProviderImpl::DoNotify(const DeviceData* device_data,
+    DeviceData::Type device_data_type) {
   DCHECK(MessageLoop::current() == creator_loop_);
 
-  last_notification_ = orientation;
+  scoped_refptr<const DeviceData> data(device_data);
 
-  typedef std::set<Observer*>::const_iterator Iterator;
-  for (Iterator i = observers_.begin(), e = observers_.end(); i != e; ++i)
-    (*i)->OnOrientationUpdate(orientation);
+  // Update last notification of this type.
+  if (data != NULL)

[hans, 2012/07/30 16:12:52]

do we need this if statement?

[aousterh, 2012/08/03 11:08:14]

Done.

+    last_notifications_map_[device_data_type] = data;
 
-  if (orientation.is_empty()) {
-    // Notify observers about failure to provide data exactly once.
-    observers_.clear();
-    Stop();
+  // Notify observers of this type of the new data.
+  typedef std::set<Observer*>::const_iterator ConstIterator;
+  for (ConstIterator i = observers_.begin(); i != observers_.end(); ++i) {
+    if ((*i)->device_data_type() == device_data_type)
+      (*i)->OnDeviceDataUpdate(data.get(), device_data_type);
+  }
+
+  if (data == NULL) {
+    // Notify observers exactly once about failure to provide data.
+    typedef std::set<Observer*>::iterator Iterator;
+    Iterator i = observers_.begin();
+    while (i != observers_.end()) {
+      Iterator current = i++;
+      if ((*current)->device_data_type() == device_data_type)
+        observers_.erase(current);
+    }
+
+    if (observers_.empty())
+      Stop();
   }
 }
 
-void ProviderImpl::ScheduleDoNotify(const Orientation& orientation) {
+void ProviderImpl::ScheduleDoNotify(const DeviceData* device_data,
+    DeviceData::Type device_data_type) {
   DCHECK(MessageLoop::current() == polling_thread_->message_loop());
 
-  creator_loop_->PostTask(
-      FROM_HERE, base::Bind(&ProviderImpl::DoNotify, this, orientation));
+  creator_loop_->PostTask(FROM_HERE,
+                          base::Bind(&ProviderImpl::DoNotify, this,
+                                     device_data, device_data_type));
 }
 
 void ProviderImpl::DoPoll() {
   DCHECK(MessageLoop::current() == polling_thread_->message_loop());
 
-  Orientation orientation;
-  if (!data_fetcher_->GetOrientation(&orientation)) {
-    LOG(ERROR) << "Failed to poll device orientation data fetcher.";
+  // Poll the fetcher for each type of data.
+  typedef std::set<DeviceData::Type>::const_iterator SetIterator;
+  for (SetIterator i = polling_data_types_.begin();
+       i != polling_data_types_.end(); ++i) {
+    DeviceData::Type device_data_type = *i;
+    scoped_refptr<const DeviceData> device_data(data_fetcher_->GetDeviceData(
+        device_data_type));
+
+    if (device_data == NULL) {
+      LOG(ERROR) << "Failed to poll device data fetcher.";
+      ScheduleDoNotify(NULL, device_data_type);
+      continue;
+    }
+
+    typedef std::map<DeviceData::Type, scoped_refptr<const DeviceData> >::
+        iterator Iterator;
+    Iterator iter = last_device_data_map_->find(device_data_type);
 
-    ScheduleDoNotify(Orientation::Empty());
-    return;
-  }
+    if (iter != last_device_data_map_->end() &&

[hans, 2012/07/30 16:12:52]

maybe a comment here would help?

or maybe we don't actually need to worry about inserting NULL device data
pointers in that map, maybe we could just do

scoped_refptr<const DeviceData> last_data = last_device_data_map[type];
if (!device_data->ShouldFireEvent(last_data))
  continue;

it would reduce the complexity a bit if instead of doing the .find() and !=
end() thing everywhere, we could just check for last_device_data_map_[type]
being null.

[aousterh, 2012/08/03 11:08:14]

I did it that way because last_device_data_map_ was a scoped_ptr to a map, so
that the map could be reset whenever the polling thread was restarted. Now that
PollingThread is a separate class that isn't a problem anymore, and
last_device_data_map_ can simply be a member of PollingThread instead of a
scoped_ptr in ProviderImpl, and we can easily use the [] operator. Done.

+        !device_data->ShouldFireEvent((*iter).second))
+      continue;
 
-  if (!orientation.is_empty() &&
-      SignificantlyDifferent(orientation, last_orientation_)) {
-    last_orientation_ = orientation;
-    ScheduleDoNotify(orientation);
+    // Update the last device data of this type and notify observers

[hans, 2012/07/30 16:12:52]

nit: period.

[aousterh, 2012/08/03 11:08:14]

Done.

+    if (device_data->ShouldFireEvent((*iter).second))

[hans, 2012/07/30 16:12:52]

i'm confused by this and the two lines below..
don't we just want something like 

last_device_data_map[data_type] = device_data;

[aousterh, 2012/08/03 11:08:14]

Done.

+      last_device_data_map_->erase(iter);
+    last_device_data_map_->insert(std::pair<DeviceData::Type,
+        scoped_refptr<const DeviceData> >(device_data_type, device_data));
+    ScheduleDoNotify(device_data, device_data_type);
   }
 
   ScheduleDoPoll();
@@ -159,43 +231,6 @@ void ProviderImpl::ScheduleDoPoll() {
       SamplingInterval());
 }
 
-namespace {
-
-bool IsElementSignificantlyDifferent(bool can_provide_element1,
-                                     bool can_provide_element2,
-                                     double element1,
-                                     double element2) {
-  const double kThreshold = 0.1;
-
-  if (can_provide_element1 != can_provide_element2)
-    return true;
-  if (can_provide_element1 &&
-      std::fabs(element1 - element2) >= kThreshold)
-    return true;
-  return false;
-}
-}  // namespace
-
-// Returns true if two orientations are considered different enough that
-// observers should be notified of the new orientation.
-bool ProviderImpl::SignificantlyDifferent(const Orientation& o1,
-                                          const Orientation& o2) {
-  return IsElementSignificantlyDifferent(o1.can_provide_alpha(),
-                                         o2.can_provide_alpha(),
-                                         o1.alpha(),
-                                         o2.alpha()) ||
-         IsElementSignificantlyDifferent(o1.can_provide_beta(),
-                                         o2.can_provide_beta(),
-                                         o1.beta(),
-                                         o2.beta()) ||
-         IsElementSignificantlyDifferent(o1.can_provide_gamma(),
-                                         o2.can_provide_gamma(),
-                                         o1.gamma(),
-                                         o2.gamma()) ||
-         (o1.can_provide_absolute() != o2.can_provide_absolute() ||
-          o1.absolute() != o2.absolute());
-}
-
 base::TimeDelta ProviderImpl::SamplingInterval() const {
   DCHECK(MessageLoop::current() == polling_thread_->message_loop());
   DCHECK(data_fetcher_.get());