[chromeos] New PowerManagerClient observer to collect power data.

chromeos/power/power_data_collector.h

Index: chromeos/power/power_data_collector.h
diff --git a/chromeos/power/power_data_collector.h b/chromeos/power/power_data_collector.h
new file mode 100644
index 0000000000000000000000000000000000000000..eee9f98348b1a8603d0b358ef656b0e924c42217
--- /dev/null
+++ b/chromeos/power/power_data_collector.h
@@ -0,0 +1,88 @@
+// Copyright (c) 2013 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.
+
+#ifndef CHROMEOS_POWER_POWER_DATA_COLLECTOR_H_
+#define CHROMEOS_POWER_POWER_DATA_COLLECTOR_H_
+
+#include <vector>
+
+#include "base/gtest_prod_util.h"
+#include "base/synchronization/lock.h"

[Daniel Erat, 2013/12/11 22:26:47]

nit: remove this

[Siva Chandra, 2013/12/12 00:35:59]

Done.

I realize now that I did not remove a lot of unnecessary pieces.

+#include "base/time/time.h"
+#include "chromeos/dbus/power_manager_client.h"
+
+namespace power_manager {
+class PowerSupplyProperties;
+}
+
+namespace chromeos {
+
+// A class which starts collecting power metrics, like the battery charge, as
+// soon as it is initialized via Initialize().
+//
+// This class is implemented as a global singleton, initialized after
+// DBusThreadManager which it depends on.
+class PowerDataCollector
+    : public PowerManagerClient::Observer,
+      public base::RefCountedThreadSafe<PowerDataCollector> {

[Daniel Erat, 2013/12/11 22:26:47]

you don't need this, i don't think (as this will only be created, accessed, and
destroyed from the UI thread, and there won't be any references to it floating
around, just a global pointer)

[Siva Chandra, 2013/12/12 00:35:59]

Done.

+ public:
+  struct PowerSupplySnapshot {

[Daniel Erat, 2013/12/11 22:26:47]

this creates a default c'tor that leaves |external_power| and |battery_charge|
uninitialized. please explicitly add a no-args c'tor that initializes those
fields to sane values

[Siva Chandra, 2013/12/12 00:35:59]

Done.

+    // Time the snapshot was captured.
+    base::TimeTicks time;
+
+    // True if connected to external power at the time of the snapshot.
+    bool external_power;
+
+    // The battery charge as a percentage of full charge in range [0.0, 100.00].
+    double battery_charge;
+  };
+
+ public:
+  // Can be called only after DBusThreadManager is initialized.
+  static void Initialize();
+
+  // Can be called only if initialized via Initialize, and before
+  // DBusThreadManager is destroyed.
+  static void Shutdown();
+
+  // Returns the global instance of PowerDataCollector.
+  static PowerDataCollector *Get();

[Daniel Erat, 2013/12/11 22:26:47]

nit: move '*' to left side of space to match the rest of chrome (i.e.
PowerDataCollector* Get())

[Siva Chandra, 2013/12/12 00:35:59]

Done.

+
+  // Returns a reference to a vector of power supply data.
+  std::vector<PowerSupplySnapshot> &GetPowerSupplyData(void);

[Daniel Erat, 2013/12/11 22:26:47]

nit: make this be a const reference and move '&' to the left of the space

note that you can just make it be an inlined accessor, though (i.e. rename the
member variable to |power_supply_snapshots_| and add an inlined const
std::vector<PowerSupplySnapshot>& power_supply_snapshots() const method)

[Siva Chandra, 2013/12/12 00:35:59]

Done.

+
+ private:
+  friend class base::RefCountedThreadSafe<PowerDataCollector>;
+  friend class PowerDataCollectorTest;
+  FRIEND_TEST_ALL_PREFIXES(PowerDataCollectorTest, AppendPowerSupplySnapshot);
+
+  // When the 'testing' argument is 'true', the instance is created without
+  // checking for an initialized DBusThreadManager and hence it does not listen
+  // to any PowerManagerClient notifications.
+  explicit PowerDataCollector(const bool testing);
+
+  virtual ~PowerDataCollector() {}

[Daniel Erat, 2013/12/11 22:26:47]

move the destructor implementation to the .cc file instead of leaving it in the
header

[Siva Chandra, 2013/12/12 00:35:59]

Done.

+
+  // Used in tests. Initializes the PowerDataCollector so that unit tests
+  // can be performed without depending on the DBusThreadManager.
+  static void InitializeTesting();
+
+  void ShutdownInstance(void);

[Daniel Erat, 2013/12/11 22:26:47]

get rid of this method and just do cleanup in the d'tor

[Siva Chandra, 2013/12/12 00:35:59]

Done.

+
+  // PowerManagerClient::Observer implementation:
+  virtual void PowerChanged(
+      const power_manager::PowerSupplyProperties& prop) OVERRIDE;
+
+ private:
+  // Is true for testing instance.
+  bool testing_;
+
+  std::vector<PowerSupplySnapshot> power_supply_;
+
+  DISALLOW_COPY_AND_ASSIGN(PowerDataCollector);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_POWER_POWER_DATA_COLLECTOR_H_