shill: refactor WakeOnWiFi

A future CL will refactor WiFi to take WakeOnWiFi as a dependency
rather than instantiating it during the constructor, so that we
can avoid making SimpleAlarmTimers when there is no message loop
in tests. Since MockWakeOnWiFi inherits from the real WakeOnWiFi,
it would still instantiate SimpleAlarmTimers on construction.
This means we should factor out the public interface and have
both implementations inherit from that instead.

BUG=b:37434548
TEST=unit tests

Change-Id: I7c1f48b33866c51a661c8c96d9b20ed27305a311
Reviewed-on: https://chromium-review.googlesource.com/1072556
Commit-Ready: Kirtika Ruchandani <kirtika@chromium.org>
Tested-by: Kirtika Ruchandani <kirtika@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>
diff --git a/wifi/mock_wake_on_wifi.cc b/wifi/mock_wake_on_wifi.cc
index f288669..c5aebaa 100644
--- a/wifi/mock_wake_on_wifi.cc
+++ b/wifi/mock_wake_on_wifi.cc
@@ -16,17 +16,10 @@
 
 #include "shill/wifi/mock_wake_on_wifi.h"
 
-#include <gmock/gmock.h>
-
 namespace shill {
 
-MockWakeOnWiFi::MockWakeOnWiFi(NetlinkManager* netlink_manager,
-                               EventDispatcher* dispatcher,
-                               Metrics* metrics,
-                               const std::string& hardware_address)
-    : WakeOnWiFi(netlink_manager, dispatcher, metrics, hardware_address,
-                 RecordWakeReasonCallback()) {}
+MockWakeOnWiFi::MockWakeOnWiFi() = default;
 
-MockWakeOnWiFi::~MockWakeOnWiFi() {}
+MockWakeOnWiFi::~MockWakeOnWiFi() = default;
 
 }  // namespace shill
diff --git a/wifi/mock_wake_on_wifi.h b/wifi/mock_wake_on_wifi.h
index 0931647..3d72159 100644
--- a/wifi/mock_wake_on_wifi.h
+++ b/wifi/mock_wake_on_wifi.h
@@ -22,18 +22,33 @@
 
 #include <gmock/gmock.h>
 
+#include "shill/error.h"
 #include "shill/net/nl80211_message.h"
-#include "shill/wifi/wake_on_wifi.h"
+#include "shill/property_store.h"
+#include "shill/wifi/wake_on_wifi_interface.h"
 
 namespace shill {
 
-class MockWakeOnWiFi : public WakeOnWiFi {
+class MockWakeOnWiFi : public WakeOnWiFiInterface {
  public:
-  MockWakeOnWiFi(NetlinkManager* netlink_manager, EventDispatcher* dispatcher,
-                 Metrics* metrics, const std::string& hardware_address);
+  MockWakeOnWiFi();
   ~MockWakeOnWiFi() override;
 
-  MOCK_METHOD0(OnAfterResume, void());
+  MOCK_METHOD1(InitPropertyStore, void(PropertyStore* store));
+  MOCK_METHOD0(StartMetricsTimer, void());
+  MOCK_METHOD2(AddWakeOnPacketConnection,
+               void(const std::string& ip_endpoint, Error* error));
+  MOCK_METHOD2(AddWakeOnPacketOfTypes,
+               void(const std::vector<std::string>& packet_types,
+                    Error* error));
+  MOCK_METHOD2(RemoveWakeOnPacketConnection,
+               void(const std::string& ip_endpoint, Error* error));
+  MOCK_METHOD2(RemoveWakeOnPacketOfTypes,
+               void(const std::vector<std::string>& packet_types,
+                    Error* error));
+  MOCK_METHOD1(RemoveAllWakeOnPacketConnections, void(Error* error));
+  MOCK_METHOD1(ParseWakeOnWiFiCapabilities,
+               void(const Nl80211Message& nl80211_message));
   MOCK_METHOD7(OnBeforeSuspend,
                void(bool is_connected,
                     const std::vector<ByteString>& ssid_whitelist,
@@ -41,6 +56,7 @@
                     const base::Closure& renew_dhcp_lease_callback,
                     const base::Closure& remove_supplicant_networks_callback,
                     bool have_dhcp_lease, uint32_t time_to_next_lease_renewal));
+  MOCK_METHOD0(OnAfterResume, void());
   MOCK_METHOD6(OnDarkResume,
                void(bool is_connected,
                     const std::vector<ByteString>& ssid_whitelist,
@@ -57,15 +73,9 @@
                void(const std::vector<ByteString>& ssid_whitelist,
                     const base::Closure& remove_supplicant_networks_callback,
                     const InitiateScanCallback& initiate_scan_callback));
-  MOCK_METHOD1(OnWakeupReasonReceived,
-               void(const NetlinkMessage& netlink_message));
-  MOCK_METHOD0(NotifyWakeupReasonReceived, void());
-  MOCK_METHOD1(NotifyWakeOnWiFiOnDarkResume,
-               void(WakeOnWiFi::WakeOnWiFiTrigger reason));
-  MOCK_METHOD1(OnWiphyIndexReceived, void(uint32_t));
-  MOCK_METHOD1(ParseWakeOnWiFiCapabilities,
-               void(const Nl80211Message& nl80211_message));
   MOCK_METHOD1(OnScanStarted, void(bool is_active_scan));
+  MOCK_METHOD0(InDarkResume, bool());
+  MOCK_METHOD1(OnWiphyIndexReceived, void(uint32_t));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockWakeOnWiFi);
diff --git a/wifi/wake_on_wifi.h b/wifi/wake_on_wifi.h
index 7fa83ba..14da6c4 100644
--- a/wifi/wake_on_wifi.h
+++ b/wifi/wake_on_wifi.h
@@ -39,6 +39,7 @@
 #include "shill/net/ip_address.h"
 #include "shill/net/netlink_manager.h"
 #include "shill/refptr_types.h"
+#include "shill/wifi/wake_on_wifi_interface.h"
 #include "shill/wifi/wifi.h"
 
 namespace shill {
@@ -169,63 +170,51 @@
 // dark resume, and both the wake to scan timer and DHCP lease renewal timers
 // are stopped.
 
-class WakeOnWiFi {
+class WakeOnWiFi : public WakeOnWiFiInterface {
  public:
-  using InitiateScanCallback = base::Callback<void(const WiFi::FreqSet&)>;
-  // Callback used to report the wake reason for the current dark resume to
-  // powerd.
-  using RecordWakeReasonCallback = base::Callback<void(const std::string&)>;
-
-  // Types of triggers that we can program the NIC to wake the WiFi device.
-  enum WakeOnWiFiTrigger {
-    kWakeTriggerUnsupported = 0,  // Used for reporting, not programming NIC.
-    kWakeTriggerPattern = 1,
-    kWakeTriggerDisconnect = 2,
-    kWakeTriggerSSID = 3
-  };
-
   WakeOnWiFi(NetlinkManager* netlink_manager, EventDispatcher* dispatcher,
              Metrics* metrics, const std::string& hardware_address,
              RecordWakeReasonCallback record_wake_reason_callback);
-  virtual ~WakeOnWiFi();
+  ~WakeOnWiFi() override;
 
   // Registers |store| with properties related to wake on WiFi.
-  void InitPropertyStore(PropertyStore* store);
+  void InitPropertyStore(PropertyStore* store) override;
 
   // Starts |metrics_timer_| so that wake on WiFi related metrics are
   // periodically collected.
-  void StartMetricsTimer();
+  void StartMetricsTimer() override;
 
   // Enables the NIC to wake on packets received from |ip_endpoint|.
   // Note: The actual programming of the NIC only happens before the system
   // suspends, in |OnBeforeSuspend|.
-  void AddWakeOnPacketConnection(const std::string& ip_endpoint, Error* error);
+  void AddWakeOnPacketConnection(const std::string& ip_endpoint,
+                                 Error* error) override;
   // Enables the NIC to wake on packets(IPv4/IPv6) with IP protocol
   // belonging to |packet_types|.
   // Note: The actual programming of the NIC only happens before the system
   // suspends, in |OnBeforeSuspend|.
   void AddWakeOnPacketOfTypes(const std::vector<std::string>& packet_types,
-                              Error* error);
+                              Error* error) override;
   // Remove rule to wake on packets received from |ip_endpoint| from the NIC.
   // Note: The actual programming of the NIC only happens before the system
   // suspends, in |OnBeforeSuspend|.
   void RemoveWakeOnPacketConnection(const std::string& ip_endpoint,
-                                    Error* error);
+                                    Error* error) override;
   // Remove rule to wake on packets(IPv4/IPv6) with IP protocol
   // belonging to |packet_types|.
   // Note: The actual programming of the NIC only happens before the system
   // suspends, in |OnBeforeSuspend|.
   void RemoveWakeOnPacketOfTypes(const std::vector<std::string>& packet_types,
-                                 Error* error);
+                                 Error* error) override;
   // Remove all rules to wake on incoming packets from the NIC.
   // Note: The actual programming of the NIC only happens before the system
   // suspends, in |OnBeforeSuspend|.
-  void RemoveAllWakeOnPacketConnections(Error* error);
+  void RemoveAllWakeOnPacketConnections(Error* error) override;
   // Given a NL80211_CMD_NEW_WIPHY message |nl80211_message|, parses the
   // wake on WiFi capabilities of the NIC and set relevant members of this
   // WakeOnWiFi object to reflect the supported capbilities.
-  virtual void ParseWakeOnWiFiCapabilities(
-      const Nl80211Message& nl80211_message);
+  void ParseWakeOnWiFiCapabilities(
+      const Nl80211Message& nl80211_message) override;
   // Callback invoked when the system reports its wakeup reason.
   //
   // Arguments:
@@ -234,7 +223,7 @@
   //
   // Note: Assumes only one wakeup reason is received. If more than one is
   // received, the only first one parsed will be handled.
-  virtual void OnWakeupReasonReceived(const NetlinkMessage& netlink_message);
+  void OnWakeupReasonReceived(const NetlinkMessage& netlink_message);
   // Performs pre-suspend actions relevant to wake on WiFi functionality.
   //
   // Arguments:
@@ -250,16 +239,15 @@
   //  - |have_dhcp_lease|: whether or not there is a DHCP lease to renew.
   //  - |time_to_next_lease_renewal|: number of seconds until next DHCP lease
   //    renewal is due.
-  virtual void OnBeforeSuspend(
-      bool is_connected,
-      const std::vector<ByteString>& ssid_whitelist,
-      const ResultCallback& done_callback,
-      const base::Closure& renew_dhcp_lease_callback,
-      const base::Closure& remove_supplicant_networks_callback,
-      bool have_dhcp_lease,
-      uint32_t time_to_next_lease_renewal);
+  void OnBeforeSuspend(bool is_connected,
+                       const std::vector<ByteString>& ssid_whitelist,
+                       const ResultCallback& done_callback,
+                       const base::Closure& renew_dhcp_lease_callback,
+                       const base::Closure& remove_supplicant_networks_callback,
+                       bool have_dhcp_lease,
+                       uint32_t time_to_next_lease_renewal) override;
   // Performs post-resume actions relevant to wake on wireless functionality.
-  virtual void OnAfterResume();
+  void OnAfterResume() override;
   // Performs and post actions to be performed in dark resume.
   //
   // Arguments:
@@ -273,39 +261,39 @@
   //  - |initate_scan_callback|: callback to invoke to initiate a scan.
   //  - |remove_supplicant_networks_callback|: callback to invoke
   //    to remove all networks from WPA supplicant.
-  virtual void OnDarkResume(
+  void OnDarkResume(
       bool is_connected,
       const std::vector<ByteString>& ssid_whitelist,
       const ResultCallback& done_callback,
       const base::Closure& renew_dhcp_lease_callback,
       const InitiateScanCallback& initiate_scan_callback,
-      const base::Closure& remove_supplicant_networks_callback);
+      const base::Closure& remove_supplicant_networks_callback) override;
   // Called when we the current service is connected, and we have IP
   // reachability. Calls WakeOnWiFi::BeforeSuspendActions if we are in dark
   // resume to end the current dark resume. Otherwise, does nothing.
-  virtual void OnConnectedAndReachable(bool start_lease_renewal_timer,
-                                       uint32_t time_to_next_lease_renewal);
+  void OnConnectedAndReachable(bool start_lease_renewal_timer,
+                               uint32_t time_to_next_lease_renewal) override;
   // Callback invoked to report whether this WiFi device is connected to
   // a service after waking from suspend.
-  virtual void ReportConnectedToServiceAfterWake(bool is_connected,
-          int seconds_in_suspend);
+  void ReportConnectedToServiceAfterWake(bool is_connected,
+                                         int seconds_in_suspend) override;
   // Called in WiFi::ScanDoneTask when there are no WiFi services available
   // for auto-connect after a scan. |initiate_scan_callback| is used for dark
   // resume scan retries.
-  virtual void OnNoAutoConnectableServicesAfterScan(
+  void OnNoAutoConnectableServicesAfterScan(
       const std::vector<ByteString>& ssid_whitelist,
       const base::Closure& remove_supplicant_networks_callback,
-      const InitiateScanCallback& initiate_scan_callback);
+      const InitiateScanCallback& initiate_scan_callback) override;
   // Called by WiFi when it is notified by the kernel that a scan has started.
   // If |is_active_scan| is true, the scan is an active scan. Otherwise, the
   // scan is a passive scan.
-  virtual void OnScanStarted(bool is_active_scan);
+  void OnScanStarted(bool is_active_scan) override;
 
-  bool in_dark_resume() { return in_dark_resume_; }
+  bool InDarkResume() override { return in_dark_resume_; }
 
-  bool connected_before_suspend() { return connected_before_suspend_;}
+  bool connected_before_suspend() { return connected_before_suspend_; }
 
-  virtual void OnWiphyIndexReceived(uint32_t index);
+  void OnWiphyIndexReceived(uint32_t index) override;
 
  private:
   friend class WakeOnWiFiTest;  // access to several members for tests
@@ -614,9 +602,9 @@
   int num_set_wake_on_packet_retries_;
   // Keeps track of triggers that the NIC will be programmed to wake from
   // while suspended.
-  std::set<WakeOnWiFi::WakeOnWiFiTrigger> wake_on_wifi_triggers_;
+  std::set<WakeOnWiFiTrigger> wake_on_wifi_triggers_;
   // Keeps track of what wake on wifi triggers this WiFi device supports.
-  std::set<WakeOnWiFi::WakeOnWiFiTrigger> wake_on_wifi_triggers_supported_;
+  std::set<WakeOnWiFiTrigger> wake_on_wifi_triggers_supported_;
   // Max number of patterns this WiFi device can be programmed to wake on at one
   // time.
   size_t wake_on_wifi_max_patterns_;
diff --git a/wifi/wake_on_wifi_interface.h b/wifi/wake_on_wifi_interface.h
new file mode 100644
index 0000000..cee2dde
--- /dev/null
+++ b/wifi/wake_on_wifi_interface.h
@@ -0,0 +1,102 @@
+//
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef SHILL_WIFI_WAKE_ON_WIFI_INTERFACE_H_
+#define SHILL_WIFI_WAKE_ON_WIFI_INTERFACE_H_
+
+#include <string>
+#include <vector>
+
+#include "shill/callbacks.h"
+#include "shill/wifi/wifi.h"
+
+namespace shill {
+
+class ByteString;
+class Error;
+class EventDispatcher;
+class Metrics;
+class Nl80211Message;
+class PropertyStore;
+
+// Base class for WakeOnWiFi implementations. This is so stub and mock
+// implementations don't pull in e.g. WakeOnWiFi members.
+//
+// This is just the interface; for explanations of each method and a
+// detailed diagram of the state machine, look at wake_on_wifi.h.
+
+class WakeOnWiFiInterface {
+ public:
+  using InitiateScanCallback = base::Callback<void(const WiFi::FreqSet&)>;
+  // Callback used to report the wake reason for the current dark resume to
+  // powerd.
+  using RecordWakeReasonCallback = base::Callback<void(const std::string&)>;
+
+  // Types of triggers that we can program the NIC to wake the WiFi device.
+  enum WakeOnWiFiTrigger {
+    kWakeTriggerUnsupported = 0,  // Used for reporting, not programming NIC.
+    kWakeTriggerPattern = 1,
+    kWakeTriggerDisconnect = 2,
+    kWakeTriggerSSID = 3
+  };
+
+  virtual ~WakeOnWiFiInterface() = default;
+
+  virtual void InitPropertyStore(PropertyStore* store) = 0;
+  virtual void StartMetricsTimer() = 0;
+  virtual void AddWakeOnPacketConnection(const std::string& ip_endpoint,
+                                         Error* error) = 0;
+  virtual void AddWakeOnPacketOfTypes(
+      const std::vector<std::string>& packet_types, Error* error) = 0;
+  virtual void RemoveWakeOnPacketConnection(const std::string& ip_endpoint,
+                                            Error* error) = 0;
+  virtual void RemoveWakeOnPacketOfTypes(
+      const std::vector<std::string>& packet_types, Error* error) = 0;
+  virtual void RemoveAllWakeOnPacketConnections(Error* error) = 0;
+  virtual void ParseWakeOnWiFiCapabilities(
+      const Nl80211Message& nl80211_message) = 0;
+  virtual void OnBeforeSuspend(
+      bool is_connected,
+      const std::vector<ByteString>& ssid_whitelist,
+      const ResultCallback& done_callback,
+      const base::Closure& renew_dhcp_lease_callback,
+      const base::Closure& remove_supplicant_networks_callback,
+      bool have_dhcp_lease,
+      uint32_t time_to_next_lease_renewal) = 0;
+  virtual void OnAfterResume() = 0;
+  virtual void OnDarkResume(
+      bool is_connected,
+      const std::vector<ByteString>& ssid_whitelist,
+      const ResultCallback& done_callback,
+      const base::Closure& renew_dhcp_lease_callback,
+      const InitiateScanCallback& initiate_scan_callback,
+      const base::Closure& remove_supplicant_networks_callback) = 0;
+  virtual void OnConnectedAndReachable(bool start_lease_renewal_timer,
+                                       uint32_t time_to_next_lease_renewal) = 0;
+  virtual void ReportConnectedToServiceAfterWake(bool is_connected,
+                                                 int seconds_in_suspend) = 0;
+  virtual void OnNoAutoConnectableServicesAfterScan(
+      const std::vector<ByteString>& ssid_whitelist,
+      const base::Closure& remove_supplicant_networks_callback,
+      const InitiateScanCallback& initiate_scan_callback) = 0;
+  virtual void OnScanStarted(bool is_active_scan) = 0;
+  virtual bool InDarkResume() = 0;
+  virtual void OnWiphyIndexReceived(uint32_t index) = 0;
+};
+
+}  // namespace shill
+
+#endif  // SHILL_WIFI_WAKE_ON_WIFI_INTERFACE_H_
diff --git a/wifi/wifi.cc b/wifi/wifi.cc
index e3505ff..fa1667d 100644
--- a/wifi/wifi.cc
+++ b/wifi/wifi.cc
@@ -1511,7 +1511,7 @@
   // Unsets this flag if it was set in InitiateScanInDarkResume since that scan
   // has completed.
   manager()->set_suppress_autoconnect(false);
-  if (wake_on_wifi_->in_dark_resume()) {
+  if (wake_on_wifi_->InDarkResume()) {
     metrics()->NotifyDarkResumeScanResultsReceived();
   }
   // Post |UpdateScanStateAfterScanDone| so it runs after any pending scan
diff --git a/wifi/wifi.h b/wifi/wifi.h
index c2014a6..8032bc8 100644
--- a/wifi/wifi.h
+++ b/wifi/wifi.h
@@ -121,7 +121,7 @@
 class SupplicantInterfaceProxyInterface;
 class SupplicantProcessProxyInterface;
 class TDLSManager;
-class WakeOnWiFi;
+class WakeOnWiFiInterface;
 class WiFiProvider;
 class WiFiService;
 
@@ -724,7 +724,7 @@
   // Wiphy interface index of this WiFi device.
   uint32_t wiphy_index_;
 
-  std::unique_ptr<WakeOnWiFi> wake_on_wifi_;
+  std::unique_ptr<WakeOnWiFiInterface> wake_on_wifi_;
 
   std::unique_ptr<TDLSManager> tdls_manager_;
 
diff --git a/wifi/wifi_unittest.cc b/wifi/wifi_unittest.cc
index 4d403fc..fdd5a2e 100644
--- a/wifi/wifi_unittest.cc
+++ b/wifi/wifi_unittest.cc
@@ -691,8 +691,7 @@
   }
 
   void InstallMockWakeOnWiFi() {
-    wake_on_wifi_ = new MockWakeOnWiFi(
-        &netlink_manager_, event_dispatcher_.get(), &metrics_, kDeviceAddress);
+    wake_on_wifi_ = new MockWakeOnWiFi();
     wifi_->wake_on_wifi_.reset(wake_on_wifi_);
   }