Revise the SystemState hierarchy.

* Removed all #includes from SystemState; added includes in .cc files
  that use the various objects (MetricsLibrary, DevicePolicy, etc).

* MockSystemState:

  - Regulated the set of getters/setters: foo() returns the current Foo
    object interface; this object can be overridden by set_foo();
    mock_foo() or fake_foo() returns the default (internal) mock/fake
    equivalent, and fails if it is different from foo() (safety).

  - Make member declaration order consistent with that of API.

  - Removed MOCK_METHOD declarations for two methods and replaced them
    with fake getter/setter. This means that MockSystemState is now
    reduced to a fake, and can be renamed (separate CL). This also means
    that a few tests have a slightly different semantics now.

* All virtual overrides are qualified as such. However, removed the
  'const' method qualified from all getters: it made little sense,
  especially when considering that getters are handing addresses of
  internal mock members.

* Made the UpdateAttempter a contained member of both
  {Real,Mock}SystemState, resolving initialization dependencies. In
  general, the invariant is that all members of the SystemState that
  rely on it being fully populated by the time of their initialization,
  need to export a separate Init() method, that will be called (by the
  SystemState implementation constructor or Init() method) only after
  all members are set.

* Made the mock GPIO handler and connection manager contained members of
  MockSystemState; the destructor could safely be moved.

* Cleanup in UpdateAttempter (part of resolving dependencies):

  - Ordinary member initialization done via default initializers
    (constants) or initializer list in the constructor (parameters).

  - Init() method only does work that cannot be done during
    construction, with appropriate comment documenting the need for it.

  - Better reuse via constructor delegation.

BUG=chromium:358278
TEST=Unit tests.

Change-Id: I96ff6fc7e7400b0a9feb6cc8d4ffe97a51000f91
Reviewed-on: https://chromium-review.googlesource.com/193587
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
diff --git a/mock_system_state.h b/mock_system_state.h
index 73987d5..7d79b77 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -5,12 +5,12 @@
 #ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_SYSTEM_STATE_H_
 #define CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_SYSTEM_STATE_H_
 
+#include <base/logging.h>
 #include <gmock/gmock.h>
-
-#include <metrics/metrics_library_mock.h>
 #include <policy/mock_device_policy.h>
 
-#include "update_engine/clock.h"
+#include "metrics/metrics_library_mock.h"
+#include "update_engine/fake_clock.h"
 #include "update_engine/fake_hardware.h"
 #include "update_engine/mock_connection_manager.h"
 #include "update_engine/mock_dbus_wrapper.h"
@@ -24,154 +24,240 @@
 
 namespace chromeos_update_engine {
 
-class UpdateAttempterMock;
-
 // Mock the SystemStateInterface so that we could lie that
 // OOBE is completed even when there's no such marker file, etc.
 class MockSystemState : public SystemState {
  public:
   MockSystemState();
 
-  virtual ~MockSystemState();
+  // Base class overrides. All getters return the current implementation of
+  // various members, either the default (fake/mock) or the one set to override
+  // it by client code.
 
-  MOCK_METHOD1(set_device_policy, void(const policy::DevicePolicy*));
-  MOCK_CONST_METHOD0(device_policy, const policy::DevicePolicy*());
-  MOCK_METHOD0(system_rebooted, bool());
-
-  inline virtual ClockInterface* clock() {
+  virtual inline ClockInterface* clock() override {
     return clock_;
   }
 
-  inline virtual ConnectionManager* connection_manager() {
+  virtual inline void set_device_policy(
+      const policy::DevicePolicy* device_policy) override {
+    device_policy_ = device_policy;
+  }
+
+  virtual inline const policy::DevicePolicy* device_policy() override {
+    return device_policy_;
+  }
+
+  virtual inline ConnectionManager* connection_manager() override {
     return connection_manager_;
   }
 
-  inline virtual HardwareInterface* hardware() {
+  virtual inline HardwareInterface* hardware() override {
     return hardware_;
   }
 
-  inline virtual MetricsLibraryInterface* metrics_lib() {
-    return &mock_metrics_lib_;
+  virtual inline MetricsLibraryInterface* metrics_lib() override {
+    return metrics_lib_;
   }
 
-  inline virtual PrefsInterface* prefs() {
+  virtual inline PrefsInterface* prefs() override {
     return prefs_;
   }
 
-  inline virtual PrefsInterface* powerwash_safe_prefs() {
+  virtual inline PrefsInterface* powerwash_safe_prefs() override {
     return powerwash_safe_prefs_;
   }
 
-  inline virtual PayloadStateInterface* payload_state() {
+  virtual inline PayloadStateInterface* payload_state() override {
     return payload_state_;
   }
 
-  inline virtual GpioHandler* gpio_handler() const {
-    return mock_gpio_handler_;
+  virtual inline GpioHandler* gpio_handler() override {
+    return gpio_handler_;
   }
 
-  inline virtual UpdateAttempterMock* update_attempter() const override {
-    return mock_update_attempter_;
+  virtual inline UpdateAttempter* update_attempter() override {
+    return update_attempter_;
   }
 
-  inline virtual OmahaRequestParams* request_params() {
+  virtual inline OmahaRequestParams* request_params() override {
     return request_params_;
   }
 
-  inline virtual P2PManager* p2p_manager() {
+  virtual inline P2PManager* p2p_manager() override {
     return p2p_manager_;
   }
 
-  inline virtual chromeos_policy_manager::PolicyManager* policy_manager() {
+  virtual inline chromeos_policy_manager::PolicyManager* policy_manager()
+      override {
     return policy_manager_;
   }
 
-  // MockSystemState-specific public method.
-  inline void set_connection_manager(ConnectionManager* connection_manager) {
-    connection_manager_ = connection_manager;
+  virtual inline bool system_rebooted() override {
+    return fake_system_rebooted_;
   }
 
-  inline MetricsLibraryMock* mock_metrics_lib() {
-    return &mock_metrics_lib_;
-  }
+  // Setters for the various members, can be used for overriding the default
+  // implementations. For convenience, setting to a null pointer will restore
+  // the default implementation.
 
   inline void set_clock(ClockInterface* clock) {
-    clock_ = clock;
+    clock_ = clock ? clock : &fake_clock_;
+  }
+
+  inline void set_connection_manager(ConnectionManager* connection_manager) {
+    connection_manager_ = (connection_manager ? connection_manager :
+                           &mock_connection_manager_);
   }
 
   inline void set_hardware(HardwareInterface* hardware) {
-    hardware_ = hardware;
+    hardware_ = hardware ? hardware : &fake_hardware_;
   }
 
-  inline FakeHardware* get_fake_hardware() {
-    return &default_hardware_;
+  inline void set_metrics_lib(MetricsLibraryInterface* metrics_lib) {
+    metrics_lib_ = metrics_lib ? metrics_lib : &mock_metrics_lib_;
   }
 
   inline void set_prefs(PrefsInterface* prefs) {
-    prefs_ = prefs;
+    prefs_ = prefs ? prefs : &mock_prefs_;
   }
 
-  inline void set_powerwash_safe_prefs(PrefsInterface* prefs) {
-    powerwash_safe_prefs_ = prefs;
+  inline void set_powerwash_safe_prefs(PrefsInterface* powerwash_safe_prefs) {
+    powerwash_safe_prefs_ = (powerwash_safe_prefs ? powerwash_safe_prefs :
+                             &mock_powerwash_safe_prefs_);
   }
 
-  inline testing::NiceMock<PrefsMock> *mock_prefs() {
-    return &mock_prefs_;
+  inline void set_payload_state(PayloadStateInterface *payload_state) {
+    payload_state_ = payload_state ? payload_state : &mock_payload_state_;
   }
 
-  inline testing::NiceMock<PrefsMock> *mock_powerwash_safe_prefs() {
-    return &mock_powerwash_safe_prefs_;
+  inline void set_gpio_handler(GpioHandler* gpio_handler) {
+    gpio_handler_ = gpio_handler ? gpio_handler : &mock_gpio_handler_;
   }
 
-  inline MockPayloadState* mock_payload_state() {
-    return &mock_payload_state_;
+  inline void set_update_attempter(UpdateAttempter* update_attempter) {
+    update_attempter_ = (update_attempter ? update_attempter :
+                         &mock_update_attempter_);
   }
 
-  inline void set_request_params(OmahaRequestParams* params) {
-    request_params_ = params;
+  inline void set_request_params(OmahaRequestParams* request_params) {
+    request_params_ = (request_params ? request_params :
+                       &default_request_params_);
   }
 
   inline void set_p2p_manager(P2PManager *p2p_manager) {
-    p2p_manager_ = p2p_manager;
+    p2p_manager_ = p2p_manager ? p2p_manager : &mock_p2p_manager_;
   }
 
   inline void set_policy_manager(
       chromeos_policy_manager::PolicyManager *policy_manager) {
-    policy_manager_ = policy_manager;
+    policy_manager_ = policy_manager ? policy_manager : &fake_policy_manager_;
   }
 
-  inline void set_payload_state(PayloadStateInterface *payload_state) {
-    payload_state_ = payload_state;
+  inline void set_system_rebooted(bool system_rebooted) {
+    fake_system_rebooted_ = system_rebooted;
+  }
+
+  // Getters for the built-in default implementations. These return the actual
+  // concrete type of each implementation. For additional safety, they will fail
+  // whenever the requested default was overridden by a different
+  // implementation.
+
+  inline FakeClock* fake_clock() {
+    CHECK(clock_ == &fake_clock_);
+    return &fake_clock_;
+  }
+
+  inline testing::NiceMock<MockConnectionManager>* mock_connection_manager() {
+    CHECK(connection_manager_ == &mock_connection_manager_);
+    return &mock_connection_manager_;
+  }
+
+  inline FakeHardware* fake_hardware() {
+    CHECK(hardware_ == &fake_hardware_);
+    return &fake_hardware_;
+  }
+
+  inline testing::NiceMock<MetricsLibraryMock>* mock_metrics_lib() {
+    CHECK(metrics_lib_ == &mock_metrics_lib_);
+    return &mock_metrics_lib_;
+  }
+
+  inline testing::NiceMock<PrefsMock> *mock_prefs() {
+    CHECK(prefs_ == &mock_prefs_);
+    return &mock_prefs_;
+  }
+
+  inline testing::NiceMock<PrefsMock> *mock_powerwash_safe_prefs() {
+    CHECK(powerwash_safe_prefs_ == &mock_powerwash_safe_prefs_);
+    return &mock_powerwash_safe_prefs_;
+  }
+
+  inline testing::NiceMock<MockPayloadState>* mock_payload_state() {
+    CHECK(payload_state_ == &mock_payload_state_);
+    return &mock_payload_state_;
+  }
+
+  inline testing::NiceMock<MockGpioHandler>* mock_gpio_handler() {
+    CHECK(gpio_handler_ == &mock_gpio_handler_);
+    return &mock_gpio_handler_;
+  }
+
+  inline testing::NiceMock<UpdateAttempterMock>* mock_update_attempter() {
+    CHECK(update_attempter_ == &mock_update_attempter_);
+    return &mock_update_attempter_;
+  }
+
+  inline OmahaRequestParams* default_request_params() {
+    CHECK(request_params_ == &default_request_params_);
+    return &default_request_params_;
+  }
+
+  inline testing::NiceMock<MockP2PManager>* mock_p2p_manager() {
+    CHECK(p2p_manager_ == &mock_p2p_manager_);
+    return &mock_p2p_manager_;
+  }
+
+  inline chromeos_policy_manager::FakePolicyManager* fake_policy_manager() {
+    CHECK(policy_manager_ == &fake_policy_manager_);
+    return &fake_policy_manager_;
   }
 
  private:
-  // These are Mock objects or objects we own.
+  // Default mock/fake implementations (owned).
+  FakeClock fake_clock_;
+  testing::NiceMock<MockConnectionManager> mock_connection_manager_;
+  FakeHardware fake_hardware_;
   testing::NiceMock<MetricsLibraryMock> mock_metrics_lib_;
   testing::NiceMock<PrefsMock> mock_prefs_;
   testing::NiceMock<PrefsMock> mock_powerwash_safe_prefs_;
-  testing::NiceMock<MockP2PManager> mock_p2p_manager_;
   testing::NiceMock<MockPayloadState> mock_payload_state_;
-  testing::NiceMock<MockGpioHandler>* mock_gpio_handler_;
-  testing::NiceMock<UpdateAttempterMock>* mock_update_attempter_;
-  testing::NiceMock<MockConnectionManager>* mock_connection_manager_;
-  MockDBusWrapper dbus_;
-
-  // These are the other object we own.
-  Clock default_clock_;
-  FakeHardware default_hardware_;
-  chromeos_policy_manager::FakePolicyManager fake_policy_manager_;
+  testing::NiceMock<MockGpioHandler> mock_gpio_handler_;
+  testing::NiceMock<UpdateAttempterMock> mock_update_attempter_;
   OmahaRequestParams default_request_params_;
+  testing::NiceMock<MockP2PManager> mock_p2p_manager_;
+  chromeos_policy_manager::FakePolicyManager fake_policy_manager_;
 
-  // These are pointers to objects which caller can override.
+  // Pointers to objects that client code can override. They are initialized to
+  // the default implementations above.
   ClockInterface* clock_;
+  ConnectionManager* connection_manager_;
   HardwareInterface* hardware_;
+  MetricsLibraryInterface* metrics_lib_;
   PrefsInterface* prefs_;
   PrefsInterface* powerwash_safe_prefs_;
-  ConnectionManager* connection_manager_;
+  PayloadStateInterface* payload_state_;
+  GpioHandler* gpio_handler_;
+  UpdateAttempter* update_attempter_;
   OmahaRequestParams* request_params_;
   P2PManager* p2p_manager_;
-  PayloadStateInterface* payload_state_;
   chromeos_policy_manager::PolicyManager* policy_manager_;
+
+  // Other object pointers (not preinitialized).
+  const policy::DevicePolicy* device_policy_;
+
+  // Other data members.
+  MockDBusWrapper dbus_;
+  bool fake_system_rebooted_;
 };
 
 }  // namespace chromeos_update_engine