Use GUID in captive portal API instead of service path

Also changes the fake implementations and other tests to explicitly specify guids.

BUG=none
For components/wifi/fake_wifi_service.cc:
TBR=mef@chromium.org

Review URL: https://codereview.chromium.org/377063003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282995 0039d316-1c4b-4281-b951-d872f2087c98

ash/system/chromeos/network/network_state_notifier_unittest.cc

@@ -71,8 +71,8 @@ class NetworkStateNotifierTest : public AshTestBase {
         DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
     service_test->ClearServices();
     const bool add_to_visible = true;
-    // Create wifi and cellular networks and set to online.
-    service_test->AddService("wifi1", "wifi1",
+    // Create a wifi network and set to online.
+    service_test->AddService("/service/wifi1", "wifi1_guid", "wifi1",
                              shill::kTypeWifi, shill::kStateIdle,
                              add_to_visible);
     service_test->SetServiceProperty("wifi1",

chrome/browser/chromeos/customization_document_unittest.cc

@@ -238,11 +238,13 @@ class ServicesCustomizationDocumentTest : public testing::Test {
     NetworkPortalDetector::CaptivePortalState online_state;
     online_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
     online_state.response_code = 204;
-    network_portal_detector_.SetDefaultNetworkPathForTesting(
-        default_network_path,
-        default_network ? default_network->guid() : "");
-    network_portal_detector_.SetDetectionResultsForTesting(
-        default_network_path, online_state);
+    std::string guid =
+        default_network ? default_network->guid() : std::string();
+    network_portal_detector_.SetDefaultNetworkForTesting(guid);
+    if (!guid.empty()) {
+      network_portal_detector_.SetDetectionResultsForTesting(
+          guid, online_state);
+    }
 
     TestingBrowserProcess::GetGlobal()->SetLocalState(&local_state_);
     ServicesCustomizationDocument::RegisterPrefs(local_state_.registry());

chrome/browser/chromeos/login/managed/managed_user_test_base.cc

@@ -51,6 +51,9 @@ namespace chromeos {
 namespace {
 
 const char kCurrentPage[] = "$('managed-user-creation').currentPage_";
+
+const char kStubEthernetGuid[] = "eth0";
+
 }
 
 ManagedUsersSyncTestAdapter::ManagedUsersSyncTestAdapter(Profile* profile)
@@ -195,11 +198,9 @@ void ManagedUserTestBase::SetUpInProcessBrowserTestFixture() {
   NetworkPortalDetector::CaptivePortalState online_state;
   online_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
   online_state.response_code = 204;
-  network_portal_detector_->SetDefaultNetworkPathForTesting(
-      kStubEthernetServicePath,
-      kStubEthernetServicePath /* guid */);
-  network_portal_detector_->SetDetectionResultsForTesting(
-      kStubEthernetServicePath, online_state);
+  network_portal_detector_->SetDefaultNetworkForTesting(kStubEthernetGuid);
+  network_portal_detector_->SetDetectionResultsForTesting(kStubEthernetGuid,
+                                                          online_state);
 }
 
 void ManagedUserTestBase::CleanUpOnMainThread() {

chrome/browser/chromeos/login/managed/managed_user_test_base.h

@@ -20,8 +20,6 @@
 
 namespace chromeos {
 
-const char kStubEthernetServicePath[] = "eth0";
-
 const char kTestManager[] = "test-manager@gmail.com";
 const char kTestOtherUser[] = "test-user@gmail.com";
 

chrome/browser/chromeos/login/screens/update_screen.cc

@@ -454,8 +454,7 @@ void UpdateScreen::OnActorDestroyed(UpdateScreenActor* actor) {
     actor_ = NULL;
 }
 
-void UpdateScreen::OnConnectToNetworkRequested(
-    const std::string& service_path) {
+void UpdateScreen::OnConnectToNetworkRequested() {
   if (state_ == STATE_ERROR) {
     LOG(WARNING) << "Hiding error message since AP was reselected";
     StartUpdateCheck();

chrome/browser/chromeos/login/screens/update_screen.h

@@ -44,8 +44,7 @@ class UpdateScreen: public UpdateEngineClient::Observer,
   // UpdateScreenActor::Delegate implementation:
   virtual void CancelUpdate() OVERRIDE;
   virtual void OnActorDestroyed(UpdateScreenActor* actor) OVERRIDE;
-  virtual void OnConnectToNetworkRequested(
-      const std::string& service_path) OVERRIDE;
+  virtual void OnConnectToNetworkRequested() OVERRIDE;
 
   // Starts network check. Made virtual to simplify mocking.
   virtual void StartNetworkCheck();

chrome/browser/chromeos/login/screens/update_screen_actor.h

@@ -22,11 +22,15 @@ class UpdateScreenActor {
   class Delegate {
    public:
     virtual ~Delegate() {}
+
     // Force cancel update.
     virtual void CancelUpdate() = 0;
+
+    // Called prior to destroying |actor|.
     virtual void OnActorDestroyed(UpdateScreenActor* actor) = 0;
-    virtual void OnConnectToNetworkRequested(
-        const std::string& service_path) = 0;
+
+    // Called any time a new network connect request occurs.
+    virtual void OnConnectToNetworkRequested() = 0;
   };
 
   virtual ~UpdateScreenActor() {}

chrome/browser/chromeos/login/screens/update_screen_browsertest.cc

@@ -32,8 +32,8 @@ namespace chromeos {
 
 namespace {
 
-const char kStubEthernetServicePath[] = "eth0";
-const char kStubWifiServicePath[] = "wlan0";
+const char kStubEthernetGuid[] = "eth0";
+const char kStubWifiGuid[] = "wlan0";
 
 }  // namespace
 
@@ -64,9 +64,9 @@ class UpdateScreenTest : public WizardInProcessBrowserTest {
     NetworkPortalDetector::CaptivePortalState online_state;
     online_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
     online_state.response_code = 204;
-    SetDefaultNetworkPath(kStubEthernetServicePath);
-    SetDetectionResults(kStubEthernetServicePath, online_state);
-    SetDetectionResults(kStubWifiServicePath, online_state);
+    SetDefaultNetwork(kStubEthernetGuid);
+    SetDetectionResults(kStubEthernetGuid, online_state);
+    SetDetectionResults(kStubWifiGuid, online_state);
   }
 
   virtual void SetUpOnMainThread() OVERRIDE {
@@ -102,19 +102,16 @@ class UpdateScreenTest : public WizardInProcessBrowserTest {
     WizardInProcessBrowserTest::TearDownInProcessBrowserTestFixture();
   }
 
-  void SetDefaultNetworkPath(const std::string& service_path) {
+  void SetDefaultNetwork(const std::string& guid) {
     DCHECK(network_portal_detector_);
-    network_portal_detector_->SetDefaultNetworkPathForTesting(
-        service_path,
-        service_path /* guid */);
+    network_portal_detector_->SetDefaultNetworkForTesting(guid);
   }
 
   void SetDetectionResults(
-      const std::string& service_path,
+      const std::string& guid,
       const NetworkPortalDetector::CaptivePortalState& state) {
     DCHECK(network_portal_detector_);
-    network_portal_detector_->SetDetectionResultsForTesting(service_path,
-                                                            state);
+    network_portal_detector_->SetDetectionResultsForTesting(guid, state);
   }
 
   void NotifyPortalDetectionCompleted() {
@@ -248,7 +245,7 @@ IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestTemproraryOfflineNetwork) {
   NetworkPortalDetector::CaptivePortalState portal_state;
   portal_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL;
   portal_state.response_code = 200;
-  SetDetectionResults(kStubEthernetServicePath, portal_state);
+  SetDetectionResults(kStubEthernetGuid, portal_state);
 
   // Update screen will show error message about portal state because
   // ethernet is behind captive portal.
@@ -268,7 +265,7 @@ IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestTemproraryOfflineNetwork) {
   NetworkPortalDetector::CaptivePortalState online_state;
   online_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
   online_state.response_code = 204;
-  SetDetectionResults(kStubEthernetServicePath, online_state);
+  SetDetectionResults(kStubEthernetGuid, online_state);
 
   // Second notification from portal detector will be about online state,
   // so update screen will hide error message and proceed to update.
@@ -294,7 +291,7 @@ IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestTwoOfflineNetworks) {
   NetworkPortalDetector::CaptivePortalState portal_state;
   portal_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL;
   portal_state.response_code = 200;
-  SetDetectionResults(kStubEthernetServicePath, portal_state);
+  SetDetectionResults(kStubEthernetGuid, portal_state);
 
   // Update screen will show error message about portal state because
   // ethernet is behind captive portal.
@@ -316,8 +313,8 @@ IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestTwoOfflineNetworks) {
   proxy_state.status =
       NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PROXY_AUTH_REQUIRED;
   proxy_state.response_code = -1;
-  SetDefaultNetworkPath(kStubWifiServicePath);
-  SetDetectionResults(kStubWifiServicePath, proxy_state);
+  SetDefaultNetwork(kStubWifiGuid);
+  SetDetectionResults(kStubWifiGuid, proxy_state);
 
   // Update screen will show message about proxy error because wifie
   // network requires proxy authentication.
@@ -329,7 +326,7 @@ IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestTwoOfflineNetworks) {
 }
 
 IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestVoidNetwork) {
-  SetDefaultNetworkPath("");
+  SetDefaultNetwork(std::string());
 
   // Cancels pending update request.
   EXPECT_CALL(*mock_screen_observer_,
@@ -373,7 +370,7 @@ IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestAPReselection) {
   NetworkPortalDetector::CaptivePortalState portal_state;
   portal_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL;
   portal_state.response_code = 200;
-  SetDetectionResults(kStubEthernetServicePath, portal_state);
+  SetDetectionResults(kStubEthernetGuid, portal_state);
 
   // Update screen will show error message about portal state because
   // ethernet is behind captive portal.
@@ -401,7 +398,7 @@ IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestAPReselection) {
               OnExit(ScreenObserver::UPDATE_ERROR_CHECKING_FOR_UPDATE))
       .Times(1);
 
-  update_screen_->OnConnectToNetworkRequested(kStubEthernetServicePath);
+  update_screen_->OnConnectToNetworkRequested();
   base::MessageLoop::current()->RunUntilIdle();
 }
 

chrome/browser/chromeos/login/test/oobe_base_test.cc

@@ -57,9 +57,8 @@ void OobeBaseTest::SetUpInProcessBrowserTestFixture() {
   host_resolver()->AddRule("*", "127.0.0.1");
   network_portal_detector_ = new NetworkPortalDetectorTestImpl();
   NetworkPortalDetector::InitializeForTesting(network_portal_detector_);
-  network_portal_detector_->SetDefaultNetworkPathForTesting(
-      FakeShillManagerClient::kFakeEthernetNetworkPath,
-      FakeShillManagerClient::kFakeEthernetNetworkPath /* guid */);
+  network_portal_detector_->SetDefaultNetworkForTesting(
+      FakeShillManagerClient::kFakeEthernetNetworkGuid);
 
   ExtensionApiTest::SetUpInProcessBrowserTestFixture();
 }
@@ -110,7 +109,8 @@ void OobeBaseTest::SimulateNetworkOffline() {
   NetworkPortalDetector::CaptivePortalState offline_state;
   offline_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_OFFLINE;
   network_portal_detector_->SetDetectionResultsForTesting(
-      FakeShillManagerClient::kFakeEthernetNetworkPath, offline_state);
+      FakeShillManagerClient::kFakeEthernetNetworkGuid,
+      offline_state);
   network_portal_detector_->NotifyObserversForTesting();
 }
 
@@ -124,7 +124,8 @@ void OobeBaseTest::SimulateNetworkOnline() {
   online_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE;
   online_state.response_code = 204;
   network_portal_detector_->SetDetectionResultsForTesting(
-      FakeShillManagerClient::kFakeEthernetNetworkPath, online_state);
+      FakeShillManagerClient::kFakeEthernetNetworkGuid,
+      online_state);
   network_portal_detector_->NotifyObserversForTesting();
 }
 
@@ -137,7 +138,8 @@ void OobeBaseTest::SimulateNetworkPortal() {
   NetworkPortalDetector::CaptivePortalState portal_state;
   portal_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL;
   network_portal_detector_->SetDetectionResultsForTesting(
-      FakeShillManagerClient::kFakeEthernetNetworkPath, portal_state);
+      FakeShillManagerClient::kFakeEthernetNetworkGuid,
+      portal_state);
   network_portal_detector_->NotifyObserversForTesting();
 }
 

chrome/browser/chromeos/login/ui/captive_portal_window_browsertest.cc

@@ -187,11 +187,11 @@ class CaptivePortalWindowCtorDtorTest : public LoginManagerTest {
     NetworkPortalDetector::CaptivePortalState portal_state;
     portal_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_PORTAL;
     portal_state.response_code = 200;
-    network_portal_detector_->SetDefaultNetworkPathForTesting(
-        FakeShillManagerClient::kFakeEthernetNetworkPath,
-        FakeShillManagerClient::kFakeEthernetNetworkPath /* guid */);
+    network_portal_detector_->SetDefaultNetworkForTesting(
+        FakeShillManagerClient::kFakeEthernetNetworkGuid);
     network_portal_detector_->SetDetectionResultsForTesting(
-        FakeShillManagerClient::kFakeEthernetNetworkPath, portal_state);
+        FakeShillManagerClient::kFakeEthernetNetworkGuid,
+        portal_state);
   }
 
  protected:

chrome/browser/chromeos/login/wizard_controller_browsertest.cc

@@ -432,11 +432,10 @@ class WizardControllerFlowTest : public WizardControllerTest {
     const NetworkState* default_network =
         NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
 
-    network_portal_detector_->SetDefaultNetworkPathForTesting(
-        default_network->path(),
+    network_portal_detector_->SetDefaultNetworkForTesting(
         default_network->guid());
     network_portal_detector_->SetDetectionResultsForTesting(
-        default_network->path(),
+        default_network->guid(),
         online_state);
   }
 

chrome/browser/chromeos/net/delay_network_call.cc

@@ -35,7 +35,7 @@ void chromeos::DelayNetworkCall(const base::Closure& callback,
   if (!delay_network_call && NetworkPortalDetector::IsInitialized()) {
     NetworkPortalDetector* detector = NetworkPortalDetector::Get();
     NetworkPortalDetector::CaptivePortalStatus status =
-        detector->GetCaptivePortalState(default_network->path()).status;
+        detector->GetCaptivePortalState(default_network->guid()).status;
     if (status != NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE) {
       delay_network_call = true;
       DVLOG(1) << "DelayNetworkCall: Captive portal status for "

chrome/browser/chromeos/net/network_portal_detector_impl.cc

@@ -252,7 +252,7 @@ void NetworkPortalDetectorImpl::AddAndFireObserver(Observer* observer) {
   CaptivePortalState portal_state;
   const NetworkState* network = DefaultNetwork();
   if (network)
-    portal_state = GetCaptivePortalState(network->path());
+    portal_state = GetCaptivePortalState(network->guid());
   observer->OnPortalDetectionCompleted(network, portal_state);
 }
 
@@ -275,18 +275,20 @@ void NetworkPortalDetectorImpl::Enable(bool start_detection) {
   const NetworkState* network = DefaultNetwork();
   if (!start_detection || !network)
     return;
-  portal_state_map_.erase(network->path());
+  VLOG(1) << "Starting detection for: "
+          << "name=" << network->name() << ", id=" << network->guid();
+  portal_state_map_.erase(network->guid());
   StartDetection();
 }
 
 NetworkPortalDetectorImpl::CaptivePortalState
-NetworkPortalDetectorImpl::GetCaptivePortalState(
-    const std::string& service_path) {
+NetworkPortalDetectorImpl::GetCaptivePortalState(const std::string& guid) {
   DCHECK(CalledOnValidThread());
-  CaptivePortalStateMap::const_iterator it =
-      portal_state_map_.find(service_path);
-  if (it == portal_state_map_.end())
+  CaptivePortalStateMap::const_iterator it = portal_state_map_.find(guid);
+  if (it == portal_state_map_.end()) {
+    VLOG(1) << "CaptivePortalState not found for: " << guid;
     return CaptivePortalState();
+  }
   return it->second;
 }
 
@@ -312,8 +314,8 @@ void NetworkPortalDetectorImpl::DefaultNetworkChanged(
   DCHECK(CalledOnValidThread());
 
   if (!default_network) {
+    VLOG(1) << "DefaultNetworkChanged: None.";
     default_network_name_.clear();
-    default_network_id_.clear();
 
     StopDetection();
 
@@ -324,15 +326,21 @@ void NetworkPortalDetectorImpl::DefaultNetworkChanged(
   }
 
   default_network_name_ = default_network->name();
-  default_network_id_ = default_network->guid();
 
-  bool network_changed = (default_service_path_ != default_network->path());
-  default_service_path_ = default_network->path();
+  bool network_changed = (default_network_id_ != default_network->guid());
+  default_network_id_ = default_network->guid();
 
   bool connection_state_changed =
       (default_connection_state_ != default_network->connection_state());
   default_connection_state_ = default_network->connection_state();
 
+  VLOG(1) << "DefaultNetworkChanged: "
+          << "name=" << default_network_name_ << ", "
+          << "id=" << default_network_id_ << ", "
+          << "state=" << default_connection_state_ << ", "
+          << "changed=" << network_changed << ", "
+          << "state_changed=" << connection_state_changed;
+
   if (network_changed || connection_state_changed)
     StopDetection();
 
@@ -341,7 +349,7 @@ void NetworkPortalDetectorImpl::DefaultNetworkChanged(
     // Initiate Captive Portal detection if network's captive
     // portal state is unknown (e.g. for freshly created networks),
     // offline or if network connection state was changed.
-    CaptivePortalState state = GetCaptivePortalState(default_network->path());
+    CaptivePortalState state = GetCaptivePortalState(default_network->guid());
     if (state.status == CAPTIVE_PORTAL_STATUS_UNKNOWN ||
         state.status == CAPTIVE_PORTAL_STATUS_OFFLINE ||
         (!network_changed && connection_state_changed)) {
@@ -533,7 +541,7 @@ void NetworkPortalDetectorImpl::OnDetectionCompleted(
   }
 
   CaptivePortalStateMap::const_iterator it =
-      portal_state_map_.find(network->path());
+      portal_state_map_.find(network->guid());
   if (it == portal_state_map_.end() || it->second.status != state.status ||
       it->second.response_code != state.response_code) {
     VLOG(1) << "Updating Chrome Captive Portal state: "
@@ -552,7 +560,7 @@ void NetworkPortalDetectorImpl::OnDetectionCompleted(
       RecordPortalToOnlineTransition(state.time - it->second.time);
     }
 
-    portal_state_map_[network->path()] = state;
+    portal_state_map_[network->guid()] = state;
   }
   NotifyDetectionCompleted(network, state);
 }