commit ecc7f793ede87ef5789c554801e0533deda8acc0
author Gilad Arnold <garnold@chromium.org> Wed Apr 09 12:56:13 2014 -0700
committer chrome-internal-fetch <chrome-internal-fetch@google.com> Sat Apr 19 03:36:12 2014 +0000
tree f3f65959c4cc9edf3ff6e370e23a0f1cb0937d97
parent df3dd2438aef108ac66dc42a4eabae8c79ee88a3

PM: RealShillProvider should properly cache the connection type.

This exposes (in unit test) and fixes a bug where the RealShillProvider
neglected to update the cached value of the connection type, although it
was marking this value as valid and used it in subsequent calls.

BUG=None
TEST=Unit test added; fails before, passes after.

Change-Id: Ifbb206adcc82706a786ba1828220bb015badbfbc
Reviewed-on: https://chromium-review.googlesource.com/193872
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>

policy_manager/real_shill_provider.cc

diff --git a/policy_manager/real_shill_provider.cc b/policy_manager/real_shill_provider.cc
index 46d8958..a503bc2 100644
--- a/policy_manager/real_shill_provider.cc
+++ b/policy_manager/real_shill_provider.cc
@@ -158,12 +158,9 @@
       return NULL;
     }
 
-    ConnectionType conn_type;
-    if (provider_->is_conn_type_valid_) {
-      conn_type = provider_->conn_type_;
-    } else {
+    if (!provider_->is_conn_type_valid_) {
       if (!connector_->GetConnectionType(provider_->default_service_path_,
-                                         &conn_type)) {
+                                         &provider_->conn_type_)) {
         if (errmsg)
           *errmsg = base::StringPrintf(
               "Could not retrieve type of default connection (%s)",
@@ -173,7 +170,7 @@
       provider_->is_conn_type_valid_ = true;
     }
 
-    return new ConnectionType(conn_type);
+    return new ConnectionType(provider_->conn_type_);
   }
 
  private:

policy_manager/real_shill_provider_unittest.cc

diff --git a/policy_manager/real_shill_provider_unittest.cc b/policy_manager/real_shill_provider_unittest.cc
index 417fc16..d9aef4e 100644
--- a/policy_manager/real_shill_provider_unittest.cc
+++ b/policy_manager/real_shill_provider_unittest.cc
@@ -372,6 +372,19 @@
   g_hash_table_unref(service_properties);
 }
 
+// Ensure that the connection type is properly cached in the provider.
+TEST_F(PmRealShillProviderTest, ConnectionTypeCached) {
+  SetupConnectionAndTestType(kFakeEthernetServicePath,
+                             kFakeEthernetServiceProxy,
+                             shill::kTypeEthernet,
+                             ConnectionType::kEthernet);
+
+  scoped_ptr<const ConnectionType> conn_type(
+      provider_->var_conn_type()->GetValue(default_timeout_, NULL));
+  PMTEST_ASSERT_NOT_NULL(conn_type.get());
+  EXPECT_EQ(ConnectionType::kEthernet, *conn_type);
+}
+
 // Fake two DBus signal prompting about a default connection change, but
 // otherwise give the same service path. Check connection status and the time is
 // was last changed, make sure it is the same time when the first signal was

policy_manager/variable.h

diff --git a/policy_manager/variable.h b/policy_manager/variable.h
index 2ef0fe1..4507280 100644
--- a/policy_manager/variable.h
+++ b/policy_manager/variable.h
@@ -173,6 +173,7 @@
   FRIEND_TEST(PmRealShillProviderTest, ReadBaseValues);
   FRIEND_TEST(PmRealShillProviderTest, ReadChangedValuesConnectedViaVpn);
   FRIEND_TEST(PmRealShillProviderTest, ReadChangedValuesConnectedTwoSignals);
+  FRIEND_TEST(PmRealShillProviderTest, ConnectionTypeCached);
   FRIEND_TEST(PmRealShillProviderTest, NoInitConnStatusReadBaseValues);
   friend class PmRealTimeProviderTest;
   FRIEND_TEST(PmRealTimeProviderTest, CurrDateValid);