shill: Manager: Limit ConnectToBestServices to visible services

Only connect to visible services, since the connectable property of
a WiFi service does not consider its visibility (availability of
endpoints).

BUG=chromium:235093
TEST=Unit tests

Change-Id: Iddd7be5e8f690f6684d2427358055f91093bf3d6
Reviewed-on: https://gerrit.chromium.org/gerrit/49107
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
diff --git a/manager_unittest.cc b/manager_unittest.cc
index b6d1164..4f8d7dd 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -3360,6 +3360,8 @@
   wifi_service0->SetSecurity(Service::kCryptoAes, true, true);
   EXPECT_CALL(*wifi_service0.get(), technology())
       .WillRepeatedly(Return(Technology::kWifi));
+  EXPECT_CALL(*wifi_service0.get(), IsVisible())
+      .WillRepeatedly(Return(false));
 
   scoped_refptr<MockService> wifi_service1(
       new NiceMock<MockService>(control_interface(),
@@ -3368,6 +3370,8 @@
                                 manager()));
   EXPECT_CALL(*wifi_service1.get(), state())
       .WillRepeatedly(Return(Service::kStateIdle));
+  EXPECT_CALL(*wifi_service1.get(), IsVisible())
+      .WillRepeatedly(Return(true));
   EXPECT_CALL(*wifi_service1.get(), IsConnected())
       .WillRepeatedly(Return(false));
   wifi_service1->SetAutoConnect(true);
@@ -3385,6 +3389,8 @@
       .WillRepeatedly(Return(Service::kStateConnected));
   EXPECT_CALL(*wifi_service2.get(), IsConnected())
       .WillRepeatedly(Return(true));
+  EXPECT_CALL(*wifi_service2.get(), IsVisible())
+      .WillRepeatedly(Return(true));
   wifi_service2->SetAutoConnect(true);
   wifi_service2->SetConnectable(true);
   wifi_service2->SetSecurity(Service::kCryptoNone, false, false);
@@ -3408,6 +3414,8 @@
       .WillRepeatedly(Return(Service::kStateConnected));
   EXPECT_CALL(*cell_service.get(), IsConnected())
       .WillRepeatedly(Return(true));
+  EXPECT_CALL(*cell_service.get(), IsVisible())
+      .WillRepeatedly(Return(true));
   wifi_service2->SetAutoConnect(true);
   cell_service->SetConnectable(true);
   EXPECT_CALL(*cell_service.get(), technology())
@@ -3424,6 +3432,8 @@
       .WillRepeatedly(Return(Service::kStateIdle));
   EXPECT_CALL(*vpn_service.get(), IsConnected())
       .WillRepeatedly(Return(false));
+  EXPECT_CALL(*vpn_service.get(), IsVisible())
+      .WillRepeatedly(Return(true));
   wifi_service2->SetAutoConnect(false);
   vpn_service->SetConnectable(true);
   EXPECT_CALL(*vpn_service.get(), technology())
@@ -3433,8 +3443,8 @@
   // The connected services should be at the top.
   EXPECT_TRUE(ServiceOrderIs(wifi_service2, cell_service));
 
-  EXPECT_CALL(*wifi_service0.get(), Connect(_, _)).Times(1);
-  EXPECT_CALL(*wifi_service1.get(), Connect(_, _)).Times(0);  // Lower prio.
+  EXPECT_CALL(*wifi_service0.get(), Connect(_, _)).Times(0);  // Not visible.
+  EXPECT_CALL(*wifi_service1.get(), Connect(_, _));
   EXPECT_CALL(*wifi_service2.get(), Connect(_, _)).Times(0);  // Lower prio.
   EXPECT_CALL(*cell_service.get(), Connect(_, _)).Times(0);  // Is connected.
   EXPECT_CALL(*vpn_service.get(), Connect(_, _)).Times(0);  // Not autoconnect.