ChromeOs Power Emulation Impl

chromeos/dbus/fake_power_manager_client.h

 // 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_DBUS_FAKE_POWER_MANAGER_CLIENT_H_
 #define CHROMEOS_DBUS_FAKE_POWER_MANAGER_CLIENT_H_
 
 #include <string>
 
-#include "base/basictypes.h"
-#include "base/macros.h"
 #include "base/observer_list.h"
 #include "chromeos/dbus/power_manager/policy.pb.h"
+#include "chromeos/dbus/power_manager/power_supply_properties.pb.h"
 #include "chromeos/dbus/power_manager/suspend.pb.h"
 #include "chromeos/dbus/power_manager_client.h"
 
 namespace chromeos {
 
 // A fake implementation of PowerManagerClient. This remembers the policy passed
 // to SetPolicy() and the user of this class can inspect the last set policy by
 // get_policy().
 class FakePowerManagerClient : public PowerManagerClient {
  public:
@@ skipping 36 matching lines @@
 
   // Emulates the power manager announcing that the system is starting or
   // completing a suspend attempt.
   void SendSuspendImminent();
   void SendSuspendDone();
   void SendDarkSuspendImminent();
 
   // Notifies observers that the power button has been pressed or released.
   void SendPowerButtonEvent(bool down, const base::TimeTicks& timestamp);
 
+  // Emulates the changes in various power settings.
+  void UpdateBatteryPercentage(int percentage);
+  void UpdateBatteryFullTimeToEmpty(int time_in_hours);

[Daniel Erat, 2015/06/27 13:09:12]

this should take a base::TimeDelta instead

"battery full time to empty" doesn't make sense to me either. what does the
"full" mean? should it just be UpdateBatteryTimeToEmpty()?

after looking at .cc file: oh, i see; this is either the time-to-full or the
time-to-empty depending on UpdateIsBatteryCharging(). how about renaming to
something like UpdateBatteryTimeToFullOrEmpty()?

[mozartalouis, 2015/06/29 18:16:11]

Done.

+  void UpdateExternalPower(int external_power);

[Daniel Erat, 2015/06/27 13:09:12]

this should be the proper enum type instead of an int; a caller wouldn't have
any idea what the int is supposed to represent. (even after looking at the
implementation, i'm not sure.)

+  void UpdateIsBatteryCharging(bool is_charging);
+  void UpdateIsCalculatingBatteryTime(bool is_calculating);
+
+  // Called once any power states are changed to notify the observer of the
+  // change.
+  void UpdatePowerStatus();

[Daniel Erat, 2015/06/27 13:09:12]

this should probably be private rather than public, no?

[mozartalouis, 2015/06/29 18:16:11]

Done.

+
  private:
   // Callback that will be run by asynchronous suspend delays to report
   // readiness.
   void HandleSuspendReadiness();
 
   base::ObserverList<Observer> observers_;
 
   // Last policy passed to SetPolicy().
   power_manager::PowerManagementPolicy policy_;
 
+  // The power properties set for for the UI.
+  power_manager::PowerSupplyProperties props_;
+
+  // Variables that handle states of the battery and power.

[Daniel Erat, 2015/06/27 13:09:12]

high-level question: why have all of these different variables and a bunch of
different methods to set them instead of just storing a PowerSupplyProperties
protocol buffer and having a single method to set a new protobuf?

[mozartalouis, 2015/06/29 18:16:11]

Done.

+  bool is_charging_;
+  bool is_calculating_;
+  int battery_percentage_;
+  int power_source_;
+  int seconds_to_empty_full_battery_;
+
   // Number of times that various methods have been called.
   int num_request_restart_calls_;
   int num_request_shutdown_calls_;
   int num_set_policy_calls_;
   int num_set_is_projecting_calls_;
 
   // Number of pending suspend readiness callbacks.
   int num_pending_suspend_readiness_callbacks_;
 
   // Last projecting state set in SetIsProjecting().
   bool is_projecting_;
 
   // Delegate for managing power consumption of Chrome's renderer processes.
   base::WeakPtr<RenderProcessManagerDelegate> render_process_manager_delegate_;
 
+  // Note: This should remain the last member so it'll be destroyed and
+  // invalidate its weak pointers before any other members are destroyed.
+  base::WeakPtrFactory<FakePowerManagerClient> weak_ptr_factory_;

[Daniel Erat, 2015/06/27 13:09:12]

doesn't look like you actually use this...?

[mozartalouis, 2015/06/29 18:16:11]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(FakePowerManagerClient);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_DBUS_FAKE_POWER_MANAGER_CLIENT_H_