Changes ProviderImpl to use a single DataFetcherFactory

content/browser/device_orientation/provider_impl.cc

 // Copyright (c) 2012 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 <cmath>
 #include <set>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 29 matching lines @@
 namespace device_orientation {
 
 class ProviderImpl::PollingThread : public base::Thread {
  public:
   PollingThread(const char* name,
                 base::WeakPtr<ProviderImpl> provider,
                 MessageLoop* creator_loop);
   virtual ~PollingThread();
 
   // Method for finding a suitable DataFetcher and starting the polling.
-  void Initialize(const std::vector<DataFetcherFactory>& factories);
+  void Initialize(const DataFetcherFactory* factory);
 
  private:
   // Method for polling a DataFetcher.
   void DoPoll();
   void ScheduleDoPoll();
 
   // Schedule a notification to the |provider_| which lives on a different
   // thread (|creator_loop_| is its message loop).
   void ScheduleDoNotify(const Orientation& orientation);
 
@@ skipping 19 matching lines @@
     MessageLoop* creator_loop)
     : base::Thread(name),
       creator_loop_(creator_loop),
       provider_(provider) {
 }
 
 ProviderImpl::PollingThread::~PollingThread() {
 }
 
 void ProviderImpl::PollingThread::Initialize(
-    const std::vector<DataFetcherFactory>& factories) {
+    const DataFetcherFactory* factory) {
   DCHECK(MessageLoop::current() == message_loop());
 
-  typedef std::vector<DataFetcherFactory>::const_iterator Iterator;
-  for (Iterator i = factories.begin(); i != factories.end(); ++i) {
-    DataFetcherFactory factory = *i;
-    scoped_ptr<DataFetcher> fetcher(factory());
+  if (factory != NULL) {

[hans, 2012/08/01 19:08:47]

i wonder if it would make the code simpler to check if factory == NULL first,
and bail out early if so

[aousterh, 2012/08/02 10:10:38]

I don't think it would really be simpler, because we need to call
ScheduleDoNotify(Orientation::Empty()) in two cases - when factory == NULL and
when factory != NULL but the fetcher can't provide Orientation data. So if we
checked for factory == NULL first and then bailed, we would have to duplicate
the call ScheduleDoNotify(Orientation::Empty()).

+    // Try to use factory to create a fetcher that can provide orientation data.
+    DataFetcherFactory data_fetcher_factory = *factory;
+    scoped_ptr<DataFetcher> fetcher(data_fetcher_factory());
     Orientation orientation;
 
     if (fetcher.get() && fetcher->GetOrientation(&orientation)) {
       // Pass ownership of fetcher to provider_.
       data_fetcher_.swap(fetcher);
       last_orientation_ = orientation;
 
       // Notify observers.
       if (!orientation.is_empty())
         ScheduleDoNotify(orientation);
@@ skipping 69 matching lines @@
 base::TimeDelta ProviderImpl::PollingThread::SamplingInterval() const {
   DCHECK(MessageLoop::current() == message_loop());
   DCHECK(data_fetcher_.get());
 
   // TODO(erg): There used to be unused code here, that called a default
   // implementation on the DataFetcherInterface that was never defined. I'm
   // removing unused methods from headers.
   return base::TimeDelta::FromMilliseconds(kDesiredSamplingIntervalMs);
 }
 
-ProviderImpl::ProviderImpl(const DataFetcherFactory factories[])
+ProviderImpl::ProviderImpl(const DataFetcherFactory* factory)
     : creator_loop_(MessageLoop::current()),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
       polling_thread_(NULL) {
-  for (const DataFetcherFactory* fp = factories; *fp; ++fp)
-    factories_.push_back(*fp);
+  if (factory == NULL)
+    factory_ = NULL;
+  else
+    factory_ = new DataFetcherFactory(*factory);

[hans, 2012/08/01 19:08:47]

this looks overly complex; i think it should just boil down to factory_ =
factory

[aousterh, 2012/08/02 10:10:38]

Done.

 }
 
 ProviderImpl::~ProviderImpl() {
   Stop();
 }
 
 void ProviderImpl::AddObserver(Observer* observer) {
   DCHECK(MessageLoop::current() == creator_loop_);
 
   observers_.insert(observer);
@@ skipping 42 matching lines @@
   }
 }
 
 void ProviderImpl::ScheduleInitializePollingThread() {
   DCHECK(MessageLoop::current() == creator_loop_);
 
   MessageLoop* polling_loop = polling_thread_->message_loop();
   polling_loop->PostTask(FROM_HERE,
                          base::Bind(&PollingThread::Initialize,
                                     base::Unretained(polling_thread_),
-                                    factories_));
+                                    factory_));
 }
 
 void ProviderImpl::DoNotify(const Orientation& orientation) {
   DCHECK(MessageLoop::current() == creator_loop_);
 
   last_notification_ = orientation;
 
   typedef std::set<Observer*>::const_iterator Iterator;
   for (Iterator i = observers_.begin(); i != observers_.end(); ++i)
     (*i)->OnOrientationUpdate(orientation);
 
   if (orientation.is_empty()) {
     // Notify observers about failure to provide data exactly once.
     observers_.clear();
     Stop();
   }
 }
 
 }  // namespace device_orientation