NetworkPortalDetector/NetworkStateInformer: Switch over to use NetworkStateHandler

chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc

Index: chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
diff --git a/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc b/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
index 5b28e7e39c84d8e6f1d31fd107da88108acf8366..f38843676a1c0b4113c8d31532838db859a00ca1 100644
--- a/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
+++ b/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
@@ -9,15 +9,33 @@
 #include "chrome/browser/captive_portal/captive_portal_detector.h"
 #include "chrome/browser/captive_portal/testing_utils.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
-#include "chrome/browser/chromeos/cros/network_library.h"
-#include "chrome/browser/chromeos/cros/network_library_impl_base.h"
 #include "chrome/browser/chromeos/net/network_portal_detector_impl.h"
 #include "chrome/test/base/testing_profile.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/dbus/shill_service_client.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
+#include "dbus/object_path.h"
 #include "net/base/net_errors.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
+namespace {
+  void ErrorCallbackFunction(

[ygorshenin1, 2013/05/06 09:14:39]

Wrong indentation, empty line needed after line #26.

[gauravsh, 2013/05/07 23:38:10]

Done.

+    const std::string& error_name,
+    const std::string& error_message) {
+    LOG(ERROR) << "Shill Error: " << error_name << " : " << error_message;
+  }
+
+}  // namespace
+
+const char* kStubEthernet = "stub_ethernet";
+const char* kStubWireless1 = "stub_wifi1";
+const char* kStubWireless2 = "stub_wifi2";
+
 class NetworkPortalDetectorImplTest
     : public testing::Test,
       public captive_portal::CaptivePortalDetectorTestBase {
@@ -25,18 +43,46 @@ class NetworkPortalDetectorImplTest
   NetworkPortalDetectorImplTest() {}
   virtual ~NetworkPortalDetectorImplTest() {}
 
+ void SetupDefaultShillState() {
+    MessageLoop::current()->RunUntilIdle();
+    ShillServiceClient::TestInterface* service_test =
+        DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
+    service_test->ClearServices();
+    const bool add_to_watchlist = true;
+    service_test->AddService(kStubEthernet,
+                             kStubEthernet,
+                             flimflam::kTypeEthernet, flimflam::kStateIdle,
+                             add_to_watchlist);
+    service_test->AddService(kStubWireless1,
+                             kStubWireless1,
+                             flimflam::kTypeWifi, flimflam::kStateIdle,
+                             add_to_watchlist);
+    service_test->AddService(kStubWireless2,
+                             kStubWireless2,
+                             flimflam::kTypeWifi, flimflam::kStateIdle,
+                             add_to_watchlist);
+  }
+
+  virtual void SetupNetworkStateHandler() {

[ygorshenin1, 2013/05/06 09:14:39]

Why this method is virtual?

[gauravsh, 2013/05/07 23:38:10]

Done.

+    NetworkStateHandler::Initialize();
+    SetupDefaultShillState();
+    NetworkStateHandler::Get()->InitShillPropertyHandler();

[pneubeck (no reviews), 2013/05/02 22:11:13]

This last line shouldn't be necessary, if you instead swap the order of the
first two statements.

[gauravsh, 2013/05/07 23:38:10]

Done.

+  }
+
   virtual void SetUp() {
-    CrosLibrary::Initialize(true);
-    network_library_ = CrosLibrary::Get()->GetNetworkLibrary();
-    DCHECK(network_library_);
+    DBusThreadManager::InitializeWithStub();
+    SetupNetworkStateHandler();
 
-    ethernet_network_ = network_library_->FindNetworkByPath("eth1");
+    ethernet_network_ =

[pneubeck (no reviews), 2013/05/02 22:11:13]

References to NetworkState should never be stored, not even in unit tests. 
Please use only the service paths as identifier and refetch the state where
necessary.

[gauravsh, 2013/05/07 23:38:10]

Done.

+        NetworkStateHandler::Get()->GetNetworkState(kStubEthernet);
     DCHECK(ethernet_network_);
 
-    wifi1_network_ = network_library_->FindNetworkByPath("wifi1");
+    wifi1_network_ =
+        NetworkStateHandler::Get()->GetNetworkState(kStubWireless1);
     DCHECK(wifi1_network_);
 
-    wifi2_network_ = network_library_->FindNetworkByPath("wifi2");
+    wifi2_network_ =
+        NetworkStateHandler::Get()->GetNetworkState(kStubWireless2);
     DCHECK(wifi2_network_);
 
     profile_.reset(new TestingProfile());
@@ -53,22 +99,23 @@ class NetworkPortalDetectorImplTest
 
   virtual void TearDown() {
     network_portal_detector_->Shutdown();
-    CrosLibrary::Shutdown();
+    profile_.reset();
+    NetworkStateHandler::Shutdown();
+    DBusThreadManager::Shutdown();
   }
 
   void CheckPortalState(NetworkPortalDetector::CaptivePortalStatus status,
                         int response_code,
-                        const Network* network) {
+                        const NetworkState* network) {
     NetworkPortalDetector::CaptivePortalState state =
         network_portal_detector()->GetCaptivePortalState(network);
     ASSERT_EQ(status, state.status);
     ASSERT_EQ(response_code, state.response_code);
   }
 
-  NetworkLibrary* network_library() { return network_library_; }
-  Network* ethernet_network() { return ethernet_network_; }
-  Network* wifi1_network() { return wifi1_network_; }
-  Network* wifi2_network() { return wifi2_network_; }
+  const NetworkState* ethernet_network() { return ethernet_network_; }
+  const NetworkState* wifi1_network() { return wifi1_network_; }
+  const NetworkState* wifi2_network() { return wifi2_network_; }
 
   Profile* profile() { return profile_.get(); }
 
@@ -133,33 +180,33 @@ class NetworkPortalDetectorImplTest
     network_portal_detector()->set_time_ticks_for_testing(time_ticks);
   }
 
-  void SetBehindPortal(Network* network) {
-    Network::TestApi test_api(network);
-        test_api.SetBehindPortal();
-    static_cast<NetworkLibraryImplBase*>(
-        network_library())->CallConnectToNetwork(network);
+  void SetBehindPortal(const NetworkState* network) {
+    DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
+        dbus::ObjectPath(network->path()),
+        flimflam::kStateProperty, base::StringValue(flimflam::kStatePortal),
+        base::Bind(&base::DoNothing),
+        base::Bind(&ErrorCallbackFunction));
     MessageLoop::current()->RunUntilIdle();
   }
 
-  void SetConnected(Network* network) {
-    Network::TestApi test_api(network);
-    test_api.SetConnected();
-    static_cast<NetworkLibraryImplBase*>(
-        network_library())->CallConnectToNetwork(network);
+  void SetConnected(const NetworkState* network) {
+    DBusThreadManager::Get()->GetShillServiceClient()->SetProperty(
+        dbus::ObjectPath(network->path()),
+        flimflam::kStateProperty, base::StringValue(flimflam::kStateOnline),
+        base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction)

[ygorshenin1, 2013/05/06 09:14:39]

Please, combine lines #196 and #197.

[gauravsh, 2013/05/07 23:38:10]

Done.

+      );
     MessageLoop::current()->RunUntilIdle();
   }
 
  private:
-  NetworkLibrary* network_library_;
-
   // Pointer to a fake ethernet network.
-  Network* ethernet_network_;
+  const NetworkState* ethernet_network_;
 
   // Pointer to a fake wifi1 network.
-  Network* wifi1_network_;
+  const NetworkState* wifi1_network_;
 
   // Pointer to a fake wifi2 network.
-  Network* wifi2_network_;
+  const NetworkState* wifi2_network_;
 
   MessageLoop message_loop_;