Remove unnecessary calls to AddWakeOnPaketConnection and RemoveWakeOnPacketConnection

chrome/browser/chromeos/net/wake_on_wifi_manager.h

 // Copyright 2014 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 CHROME_BROWSER_CHROMEOS_NET_WAKE_ON_WIFI_MANAGER_H_
 #define CHROME_BROWSER_CHROMEOS_NET_WAKE_ON_WIFI_MANAGER_H_
 
-#include <memory>
+#include <map>
 
-#include "base/containers/scoped_ptr_hash_map.h"
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/power/extension_event_observer.h"
+#include "chromeos/network/network_device_handler.h"

[stevenjb, 2016/04/19 23:46:15]

Forward declare instead

[malaykeshav, 2016/04/20 19:48:42]

Done

 #include "chromeos/network/network_state_handler_observer.h"
+#include "components/gcm_driver/gcm_connection_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "net/base/ip_endpoint.h"
 
 class Profile;
 
 namespace base {
 class DictionaryValue;
 }
 
 namespace chromeos {
 
+class ConnectionObserverTest;
+
 // This class is responsible for managing the various wake-on-wifi related bits
 // of functionality in chrome.  It is responsible for communicating the user's
 // preferences to shill as well as listening for connections to the Google GCM
 // servers and sending that connection information down to shill.  This class is
 // owned by ChromeBrowserMainPartsChromeos.  This class is also NOT thread-safe
 // and should only be called on the UI thread.
 class WakeOnWifiManager : public content::NotificationObserver,
                           public NetworkStateHandlerObserver {
  public:
   enum WakeOnWifiFeature {
@@ skipping 21 matching lines @@
   // content::NotificationObserver override.
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
   // NetworkStateHandlerObserver overrides.
   void DeviceListChanged() override;
   void DevicePropertiesUpdated(const DeviceState* device) override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiPacketAdd);
+  FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiPacketRemove);
+  FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiNoneAdd);
+  FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiNoneRemove);
+
   // Sends the user's preference to shill, updates the timer used by the GCM
   // client to send heartbeats, and tells |extension_event_observer_| to block
   // (or not block) suspends based on the value of |current_feature_|.
   void HandleWakeOnWifiFeatureUpdated();
 
   // Requests all the properties for the wifi device from shill.
   void GetWifiDeviceProperties();
 
   // Callback for getting the Wi-Fi device properties.
   void GetDevicePropertiesCallback(const std::string& device_path,
                                    const base::DictionaryValue& properties);
 
   // Called when a Profile is added or destroyed.
   void OnProfileAdded(Profile* profile);
   void OnProfileDestroyed(Profile* profile);
 
   WakeOnWifiFeature current_feature_;
 
   // Set to true once we have received the properties for the wifi device from
   // shill.
   bool wifi_properties_received_;
 
-  class WakeOnPacketConnectionObserver;
-  base::ScopedPtrHashMap<Profile*,
-                         std::unique_ptr<WakeOnPacketConnectionObserver>>
-      connection_observers_;
+  // Simple class that listens for a connection to the GCM server and passes the
+  // connection information down to shill.  Each profile gets its own instance
+  // of this class.
+  class ConnectionObserver : public gcm::GCMConnectionObserver {
+   public:
+    ConnectionObserver(Profile* profile,
+                       bool wifi_properties_received,
+                       WakeOnWifiManager::WakeOnWifiFeature feature,
+                       NetworkDeviceHandler* network_device_handler);
+    ~ConnectionObserver() override;
+
+    void HandleWifiDevicePropertiesReady();
+
+    // gcm::GCMConnectionObserver overrides:
+    void OnConnected(const net::IPEndPoint& ip_endpoint) override;
+    void OnDisconnected() override;
+
+    void set_feature(const WakeOnWifiFeature feature) { feature_ = feature; }
+
+   private:
+    FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiPacketAdd);
+    FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest,
+                             TestWakeOnWifiPacketRemove);
+    FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiNoneAdd);
+    FRIEND_TEST_ALL_PREFIXES(WakeOnWifiObserverTest, TestWakeOnWifiNoneRemove);
+
+    void AddWakeOnPacketConnection();
+    void RemoveWakeOnPacketConnection();
+
+    Profile* profile_;
+    net::IPEndPoint ip_endpoint_;
+    bool wifi_properties_received_;
+    WakeOnWifiManager::WakeOnWifiFeature feature_;
+    NetworkDeviceHandler* network_device_handler_;
+
+    DISALLOW_COPY_AND_ASSIGN(ConnectionObserver);
+  };

[stevenjb, 2016/04/19 23:46:15]

Since we need to expose this in the header for tests, we should move this and
its implementation to separate files. That will also reduce the header
dependencies we need to add to this header.

[malaykeshav, 2016/04/20 19:48:42]

Done

+
+  std::map<Profile*, std::unique_ptr<ConnectionObserver>> connection_observers_;
 
   std::unique_ptr<ExtensionEventObserver> extension_event_observer_;
 
   content::NotificationRegistrar registrar_;
 
   base::WeakPtrFactory<WakeOnWifiManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WakeOnWifiManager);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_NET_WAKE_ON_WIFI_MANAGER_H_