shill: Service: Accept variant dict in Configure

Service::Configure is called indirectly through the Service D-Bus
adaptor "SetProperties" method as well as the Manager D-Bus adaptor
"ConfigureService" methods.  They both use ArgsToKeyValueStore to
convert D-Bus string/variant dicts into a KeyValueStore object.
In order for the StaticIPConfig property to be configured using
either of these methods, we must allow an element of the
KeyValueValueStore (the properties bundle) to contain another
KeyValueStore (Static IP properties).

This CL adds a "key_value_store_properties_" to KeyValueStore.  It
allows DBusAdaptor to unbundle such a data member, and enables this
to be used in Service::Configure and Service::DoPropertiesMatch().

BUG=chromium:450833
TEST=Unit tests

Change-Id: I7bd698fc685f56902fea3c3c8a2bbdf324e2b5ce
Reviewed-on: https://chromium-review.googlesource.com/242491
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>

9 files changed, 215 insertions, 7 deletions

diff --git a/dbus_adaptor.cc b/dbus_adaptor.cc
index cde33067..df6b7344 100644
--- a/dbus_adaptor.cc
+++ b/dbus_adaptor.cc
@@ -251,6 +251,22 @@ void DBusAdaptor::ArgsToKeyValueStore(
     } else if (key_value_pair.second.signature() == string_type.sig()) {
       SLOG(nullptr, 5) << "Got string property " << key;
       out->SetString(key, key_value_pair.second.reader().get_string());
+    } else if (DBusAdaptor::IsKeyValueStore(
+                   key_value_pair.second.signature())) {
+      // Unwrap a recursive KeyValueStore object.
+      KeyValueStore store;
+      Error convert_error;
+      DBusAdaptor::ArgsToKeyValueStore(
+          key_value_pair.second.operator map<string, ::DBus::Variant>(),
+          &store,
+          &convert_error);
+      if (convert_error.IsSuccess()) {
+          out->SetKeyValueStore(key, store);
+      } else {
+          Error::PopulateAndLog(FROM_HERE, error, convert_error.type(),
+                                convert_error.message() + " in sub-key " + key);
+          return;  // Skip remaining args after error.
+      }
     } else if (DBusAdaptor::IsStrings(key_value_pair.second.signature())) {
       SLOG(nullptr, 5) << "Got strings property " << key;
       out->SetStrings(key, key_value_pair.second.operator vector<string>());
diff --git a/dbus_adaptor_unittest.cc b/dbus_adaptor_unittest.cc
index 146e87c6..c2155207 100644
--- a/dbus_adaptor_unittest.cc
+++ b/dbus_adaptor_unittest.cc
@@ -327,6 +327,12 @@ TEST_F(DBusAdaptorTest, ArgsToKeyValueStore) {
   const char kStringsKey[] = "strings_key";
   writer = args[kStringsKey].writer();
   writer << kStrings;
+  map<string, ::DBus::Variant> variant_map;
+  const char kVariantMapSubKey[] = "map_sub_key";
+  variant_map[kVariantMapSubKey] = DBusAdaptor::BoolToVariant(true);
+  const char kVariantMapKey[] = "map_key";
+  writer = args[kVariantMapKey].writer();
+  writer << variant_map;
   DBusAdaptor::ArgsToKeyValueStore(args, &args_kv, &error);
   EXPECT_TRUE(error.IsSuccess());
   EXPECT_EQ(kBool, args_kv.GetBool(kBoolKey));
@@ -334,6 +340,9 @@ TEST_F(DBusAdaptorTest, ArgsToKeyValueStore) {
   EXPECT_EQ(kString, args_kv.GetString(kStringKey));
   EXPECT_EQ(kStringmap, args_kv.GetStringmap(kStringmapKey));
   EXPECT_EQ(kStrings, args_kv.GetStrings(kStringsKey));
+  KeyValueStore property_map;
+  property_map.SetBool(kVariantMapSubKey, true);
+  EXPECT_TRUE(property_map.Equals(args_kv.GetKeyValueStore(kVariantMapKey)));
 }
 
 TEST_F(DBusAdaptorTest, KeyValueStoreToVariant) {
diff --git a/key_value_store.cc b/key_value_store.cc
index d670af3d..3adc1cea 100644
--- a/key_value_store.cc
+++ b/key_value_store.cc
@@ -19,6 +19,7 @@ KeyValueStore::KeyValueStore() {}
 void KeyValueStore::Clear() {
   bool_properties_.clear();
   int_properties_.clear();
+  key_value_store_properties_.clear();
   string_properties_.clear();
   stringmap_properties_.clear();
   strings_properties_.clear();
@@ -29,6 +30,7 @@ bool KeyValueStore::IsEmpty() {
   return
       bool_properties_.empty() &&
       int_properties_.empty() &&
+      key_value_store_properties_.empty() &&
       string_properties_.empty() &&
       stringmap_properties_.empty() &&
       strings_properties_.empty() &&
@@ -38,16 +40,32 @@ bool KeyValueStore::IsEmpty() {
 void KeyValueStore::CopyFrom(const KeyValueStore &b) {
   bool_properties_ = b.bool_properties_;
   int_properties_ = b.int_properties_;
+  key_value_store_properties_ = b.key_value_store_properties_;
   string_properties_ = b.string_properties_;
   stringmap_properties_ = b.stringmap_properties_;
   strings_properties_ = b.strings_properties_;
   uint_properties_ = b.uint_properties_;
 }
 
+bool KeyValueStore::KeyValueStorePropertiesAreEqual(
+    const KeyValueStore &other) const {
+  for (const auto &entry : key_value_store_properties_) {
+    const auto &key = entry.first;
+    const auto &value = entry.second;
+    if (!other.ContainsKeyValueStore(key) ||
+        !value.Equals(other.GetKeyValueStore(key))) {
+      return false;
+    }
+  }
+  return true;
+}
+
 bool KeyValueStore::Equals(const KeyValueStore &other) const {
   // First compare sizes of all maps to detect unequal stores faster.
   if (bool_properties_.size() != other.bool_properties_.size() ||
       int_properties_.size() != other.int_properties_.size() ||
+      key_value_store_properties_.size() !=
+          other.key_value_store_properties_.size() ||
       string_properties_.size() != other.string_properties_.size() ||
       stringmap_properties_.size() != other.stringmap_properties_.size() ||
       strings_properties_.size()!= other.strings_properties_.size() ||
@@ -56,6 +74,7 @@ bool KeyValueStore::Equals(const KeyValueStore &other) const {
 
   return bool_properties_ == other.bool_properties_ &&
          int_properties_ == other.int_properties_ &&
+         KeyValueStorePropertiesAreEqual(other) &&
          string_properties_ == other.string_properties_ &&
          stringmap_properties_ == other.stringmap_properties_ &&
          strings_properties_ == other.strings_properties_ &&
@@ -70,6 +89,10 @@ bool KeyValueStore::ContainsInt(const string &name) const {
   return ContainsKey(int_properties_, name);
 }
 
+bool KeyValueStore::ContainsKeyValueStore(const string &name) const {
+  return ContainsKey(key_value_store_properties_, name);
+}
+
 bool KeyValueStore::ContainsString(const string &name) const {
   return ContainsKey(string_properties_, name);
 }
@@ -98,6 +121,13 @@ int32_t KeyValueStore::GetInt(const string &name) const {
   return it->second;
 }
 
+const KeyValueStore &KeyValueStore::GetKeyValueStore(const string &name) const {
+  const auto it(key_value_store_properties_.find(name));
+  CHECK(it != key_value_store_properties_.end())
+      << "for key value store property " << name;
+  return it->second;
+}
+
 const string &KeyValueStore::GetString(const string &name) const {
   const auto it(string_properties_.find(name));
   CHECK(it != string_properties_.end()) << "for string property " << name;
@@ -132,6 +162,12 @@ void KeyValueStore::SetInt(const string &name, int32_t value) {
   int_properties_[name] = value;
 }
 
+void KeyValueStore::SetKeyValueStore(const string &name,
+                                     const KeyValueStore &value) {
+  key_value_store_properties_[name].Clear();
+  key_value_store_properties_[name].CopyFrom(value);
+}
+
 void KeyValueStore::SetString(const string &name, const string &value) {
   string_properties_[name] = value;
 }
@@ -150,6 +186,14 @@ void KeyValueStore::SetUint(const string &name, uint32_t value) {
   uint_properties_[name] = value;
 }
 
+void KeyValueStore::RemoveInt(const string &name) {
+  int_properties_.erase(name);
+}
+
+void KeyValueStore::RemoveKeyValueStore(const string &name) {
+  key_value_store_properties_.erase(name);
+}
+
 void KeyValueStore::RemoveString(const string &name) {
   string_properties_.erase(name);
 }
@@ -162,10 +206,6 @@ void KeyValueStore::RemoveStrings(const string &name) {
   strings_properties_.erase(name);
 }
 
-void KeyValueStore::RemoveInt(const string &name) {
-  int_properties_.erase(name);
-}
-
 bool KeyValueStore::LookupBool(const string &name, bool default_value) const {
   const auto it(bool_properties_.find(name));
   return it == bool_properties_.end() ? default_value : it->second;
diff --git a/key_value_store.h b/key_value_store.h
index 54d5892d..d52c5cde 100644
--- a/key_value_store.h
+++ b/key_value_store.h
@@ -35,6 +35,7 @@ class KeyValueStore {
 
   bool ContainsBool(const std::string &name) const;
   bool ContainsInt(const std::string &name) const;
+  bool ContainsKeyValueStore(const std::string &name) const;
   bool ContainsString(const std::string &name) const;
   bool ContainsStringmap(const std::string &name) const;
   bool ContainsStrings(const std::string &name) const;
@@ -42,6 +43,7 @@ class KeyValueStore {
 
   bool GetBool(const std::string &name) const;
   int32_t GetInt(const std::string &name) const;
+  const KeyValueStore &GetKeyValueStore(const std::string &name) const;
   const std::string &GetString(const std::string &name) const;
   const std::map<std::string, std::string> &GetStringmap(
       const std::string &name) const;
@@ -50,6 +52,7 @@ class KeyValueStore {
 
   void SetBool(const std::string &name, bool value);
   void SetInt(const std::string &name, int32_t value);
+  void SetKeyValueStore(const std::string &name, const KeyValueStore &value);
   void SetString(const std::string &name, const std::string &value);
   void SetStringmap(const std::string &name,
                     const std::map<std::string, std::string> &value);
@@ -61,6 +64,7 @@ class KeyValueStore {
   void RemoveStringmap(const std::string &name);
   void RemoveStrings(const std::string &name);
   void RemoveInt(const std::string &name);
+  void RemoveKeyValueStore(const std::string &name);
 
   // If |name| is in this store returns its value, otherwise returns
   // |default_value|.
@@ -75,6 +79,10 @@ class KeyValueStore {
   const std::map<std::string, int32_t> &int_properties() const {
     return int_properties_;
   }
+  const std::map<std::string, KeyValueStore>
+      &key_value_store_properties() const {
+    return key_value_store_properties_;
+  }
   const std::map<std::string, std::string> &string_properties() const {
     return string_properties_;
   }
@@ -92,8 +100,12 @@ class KeyValueStore {
   }
 
  private:
+  // Recursively compare KeyValueStore properties with |other|.
+  bool KeyValueStorePropertiesAreEqual(const KeyValueStore &other) const;
+
   std::map<std::string, bool> bool_properties_;
   std::map<std::string, int32_t> int_properties_;
+  std::map<std::string, KeyValueStore> key_value_store_properties_;
   std::map<std::string, std::string> string_properties_;
   std::map<std::string,
            std::map<std::string, std::string>> stringmap_properties_;
diff --git a/key_value_store_unittest.cc b/key_value_store_unittest.cc
index ebac9d02..cfa04ee6 100644
--- a/key_value_store_unittest.cc
+++ b/key_value_store_unittest.cc
@@ -52,6 +52,20 @@ TEST_F(KeyValueStoreTest, Int) {
   EXPECT_FALSE(store_.ContainsInt(kKey));
 }
 
+TEST_F(KeyValueStoreTest, KeyValueStore) {
+  const string kSubKey("foo");
+  const map<string, string> kSubValue{ { "bar0", "baz0" }, { "bar1", "baz1" } };
+  KeyValueStore value;
+  value.SetStringmap(kSubKey, kSubValue);
+  const string kKey("foo");
+  EXPECT_FALSE(store_.ContainsKeyValueStore(kKey));
+  store_.SetKeyValueStore(kKey, value);
+  EXPECT_TRUE(store_.ContainsKeyValueStore(kKey));
+  EXPECT_TRUE(value.Equals(store_.GetKeyValueStore(kKey)));
+  store_.RemoveKeyValueStore(kKey);
+  EXPECT_FALSE(store_.ContainsKeyValueStore(kKey));
+}
+
 TEST_F(KeyValueStoreTest, String) {
   const string kKey("foo");
   const string kDefaultValue("bar");
@@ -116,6 +130,9 @@ TEST_F(KeyValueStoreTest, Clear) {
   const string kIntKey("bar");
   const int kIntValue = 123;
   store_.SetInt(kIntKey, kIntValue);
+  const string kKeyValueStoreKey("bear");
+  const KeyValueStore kKeyValueStoreValue;
+  store_.SetKeyValueStore(kKeyValueStoreKey, kKeyValueStoreValue);
   const string kStringKey("baz");
   const string kStringValue("string");
   store_.SetString(kStringKey, kStringValue);
@@ -131,6 +148,7 @@ TEST_F(KeyValueStoreTest, Clear) {
 
   EXPECT_TRUE(store_.ContainsBool(kBoolKey));
   EXPECT_TRUE(store_.ContainsInt(kIntKey));
+  EXPECT_TRUE(store_.ContainsKeyValueStore(kKeyValueStoreKey));
   EXPECT_TRUE(store_.ContainsString(kStringKey));
   EXPECT_TRUE(store_.ContainsStringmap(kStringmapKey));
   EXPECT_TRUE(store_.ContainsStrings(kStringsKey));
@@ -140,6 +158,7 @@ TEST_F(KeyValueStoreTest, Clear) {
   EXPECT_TRUE(store_.IsEmpty());
   EXPECT_FALSE(store_.ContainsBool(kBoolKey));
   EXPECT_FALSE(store_.ContainsInt(kIntKey));
+  EXPECT_FALSE(store_.ContainsInt(kKeyValueStoreKey));
   EXPECT_FALSE(store_.ContainsString(kStringKey));
   EXPECT_FALSE(store_.ContainsStringmap(kStringmapKey));
   EXPECT_FALSE(store_.ContainsStrings(kStringsKey));
@@ -181,6 +200,23 @@ TEST_F(KeyValueStoreTest, Equals) {
   second.SetInt("intKey", 456);
   EXPECT_FALSE(first.Equals(second));
 
+  KeyValueStore key_value0;
+  key_value0.SetInt("intKey", 123);
+  KeyValueStore key_value1;
+  key_value1.SetInt("intOtherKey", 123);
+
+  first.Clear();
+  second.Clear();
+  first.SetKeyValueStore("keyValueKey", key_value0);
+  second.SetKeyValueStore("keyValueKey", key_value1);
+  EXPECT_FALSE(first.Equals(second));
+
+  first.Clear();
+  second.Clear();
+  first.SetKeyValueStore("keyValueKey", key_value0);
+  second.SetKeyValueStore("keyValueOtherKey", key_value0);
+  EXPECT_FALSE(first.Equals(second));
+
   first.Clear();
   second.Clear();
   first.SetString("stringKey", "string");
@@ -267,6 +303,10 @@ TEST_F(KeyValueStoreTest, CopyFrom) {
   const string kIntKey("bar");
   const int kIntValue = 123;
   donor.SetInt(kIntKey, kIntValue);
+  const string kKeyValueStoreKey("bear");
+  KeyValueStore keyValueStoreValue;
+  keyValueStoreValue.SetInt(kIntKey, kIntValue);
+  donor.SetKeyValueStore(kKeyValueStoreKey, keyValueStoreValue);
   const string kStringKey("baz");
   const string kStringValue("string");
   donor.SetString(kStringKey, kStringValue);
diff --git a/service.cc b/service.cc
index 9efc606f..6ae16f4e 100644
--- a/service.cc
+++ b/service.cc
@@ -649,6 +649,19 @@ void Service::Configure(const KeyValueStore &args, Error *error) {
       error->CopyFrom(set_error);
     }
   }
+  SLOG(this, 5) << "Configuring key value store properties:";
+  for (const auto &key_value_it : args.key_value_store_properties()) {
+    if (ContainsKey(parameters_ignored_for_configure_, key_value_it.first)) {
+      continue;
+    }
+    SLOG(this, 5) << "   " << key_value_it.first;
+    Error set_error;
+    store_.SetKeyValueStoreProperty(key_value_it.first,
+                                    key_value_it.second, &set_error);
+    if (error->IsSuccess() && set_error.IsFailure()) {
+      error->CopyFrom(set_error);
+    }
+  }
   SLOG(this, 5) << "Configuring string properties:";
   for (const auto &string_it : args.string_properties()) {
     if (ContainsKey(parameters_ignored_for_configure_, string_it.first)) {
@@ -729,6 +742,17 @@ bool Service::DoPropertiesMatch(const KeyValueStore &args) const {
       return false;
     }
   }
+  SLOG(this, 5) << "Checking key value store properties:";
+  for (const auto &key_value_it : args.key_value_store_properties()) {
+    SLOG(this, 5) << "   " << key_value_it.first;
+    Error get_error;
+    KeyValueStore value;
+    if (!store_.GetKeyValueStoreProperty(
+            key_value_it.first, &value, &get_error) ||
+        !value.Equals(key_value_it.second)) {
+      return false;
+    }
+  }
   return true;
 }
 
diff --git a/service_under_test.cc b/service_under_test.cc
index 93c0593f..431715ea 100644
--- a/service_under_test.cc
+++ b/service_under_test.cc
@@ -7,12 +7,14 @@
 #include <string>
 
 #include "shill/mock_adaptors.h"
+#include "shill/property_accessor.h"
 
 using std::string;
 
 namespace shill {
 
 // static
+const char ServiceUnderTest::kKeyValueStoreProperty[] = "key_value_store";
 const char ServiceUnderTest::kRpcId[] = "/mock_device_rpc";
 const char ServiceUnderTest::kStringsProperty[] = "strings";
 const char ServiceUnderTest::kStorageId[] = "service";
@@ -23,7 +25,13 @@ ServiceUnderTest::ServiceUnderTest(ControlInterface *control_interface,
                                    Manager *manager)
     : Service(control_interface, dispatcher, metrics, manager,
               Technology::kUnknown) {
-  this->mutable_store()->RegisterStrings(kStringsProperty, &strings_);
+  mutable_store()->RegisterStrings(kStringsProperty, &strings_);
+  mutable_store()->RegisterDerivedKeyValueStore(
+      kKeyValueStoreProperty,
+      KeyValueStoreAccessor(
+          new CustomAccessor<ServiceUnderTest, KeyValueStore>(
+              this, &ServiceUnderTest::GetKeyValueStore,
+              &ServiceUnderTest::SetKeyValueStore)));
 }
 
 ServiceUnderTest::~ServiceUnderTest() {}
@@ -38,4 +46,15 @@ string ServiceUnderTest::GetDeviceRpcId(Error */*error*/) const {
 
 string ServiceUnderTest::GetStorageIdentifier() const { return kStorageId; }
 
+bool ServiceUnderTest::SetKeyValueStore(
+    const KeyValueStore &value, Error *error) {
+  key_value_store_.Clear();
+  key_value_store_.CopyFrom(value);
+  return true;
+}
+
+KeyValueStore ServiceUnderTest::GetKeyValueStore(Error *error) {
+  return key_value_store_;
+}
+
 }  // namespace shill
diff --git a/service_under_test.h b/service_under_test.h
index fa06a47f..dfa8b8cb 100644
--- a/service_under_test.h
+++ b/service_under_test.h
@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "shill/key_value_store.h"
 #include "shill/service.h"
 
 namespace shill {
@@ -21,6 +22,7 @@ class Metrics;
 // This is a simple Service subclass with all the pure-virtual methods stubbed.
 class ServiceUnderTest : public Service {
  public:
+  static const char kKeyValueStoreProperty[];
   static const char kRpcId[];
   static const char kStringsProperty[];
   static const char kStorageId[];
@@ -41,10 +43,15 @@ class ServiceUnderTest : public Service {
   }
   const std::vector<std::string> &strings() const { return strings_; }
 
+  // Getter and setter for a KeyValueStore property for use in testing.
+  bool SetKeyValueStore(const KeyValueStore &value, Error *error);
+  KeyValueStore GetKeyValueStore(Error *error);
+
  private:
-  // The Service superclass has no string array properties but we need one
-  // in order to test Service::Configure.
+  // The Service superclass has no string array or KeyValueStore properties
+  // but we need them in order to test Service::Configure.
   std::vector<std::string> strings_;
+  KeyValueStore key_value_store_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceUnderTest);
 };
diff --git a/service_unittest.cc b/service_unittest.cc
index a8090386..8fb13eff 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -1210,6 +1210,22 @@ TEST_F(ServiceTest, ConfigureProfileProperty) {
   EXPECT_TRUE(error.IsSuccess());
 }
 
+TEST_F(ServiceTest, ConfigureKeyValueStoreProperty) {
+  KeyValueStore key_value_store0;
+  key_value_store0.SetBool("key0", true);
+  KeyValueStore key_value_store1;
+  key_value_store1.SetInt("key1", 1);
+  service_->SetKeyValueStore(key_value_store0, NULL);
+  ASSERT_TRUE(key_value_store0.Equals(service_->GetKeyValueStore(NULL)));
+  KeyValueStore args;
+  args.SetKeyValueStore(
+      ServiceUnderTest::kKeyValueStoreProperty, key_value_store1);
+  Error error;
+  service_->Configure(args, &error);
+  EXPECT_TRUE(error.IsSuccess());
+  EXPECT_TRUE(key_value_store1.Equals(service_->GetKeyValueStore(NULL)));
+}
+
 TEST_F(ServiceTest, DoPropertiesMatch) {
   service_->SetAutoConnect(false);
   const string kGUID0 = "guid_zero";
@@ -1221,6 +1237,11 @@ TEST_F(ServiceTest, DoPropertiesMatch) {
   const vector<string> kStrings0{ "string0", "string1" };
   const vector<string> kStrings1{ "string2", "string3" };
   service_->set_strings(kStrings0);
+  KeyValueStore key_value_store0;
+  key_value_store0.SetBool("key0", true);
+  KeyValueStore key_value_store1;
+  key_value_store1.SetInt("key1", 1);
+  service_->SetKeyValueStore(key_value_store0, NULL);
 
   {
     KeyValueStore args;
@@ -1228,6 +1249,8 @@ TEST_F(ServiceTest, DoPropertiesMatch) {
     args.SetBool(kAutoConnectProperty, false);
     args.SetInt(kPriorityProperty, kPriority0);
     args.SetStrings(ServiceUnderTest::kStringsProperty, kStrings0);
+    args.SetKeyValueStore(ServiceUnderTest::kKeyValueStoreProperty,
+                          key_value_store0);
     EXPECT_TRUE(service_->DoPropertiesMatch(args));
   }
   {
@@ -1236,6 +1259,8 @@ TEST_F(ServiceTest, DoPropertiesMatch) {
     args.SetBool(kAutoConnectProperty, false);
     args.SetInt(kPriorityProperty, kPriority0);
     args.SetStrings(ServiceUnderTest::kStringsProperty, kStrings0);
+    args.SetKeyValueStore(ServiceUnderTest::kKeyValueStoreProperty,
+                          key_value_store0);
     EXPECT_FALSE(service_->DoPropertiesMatch(args));
   }
   {
@@ -1244,6 +1269,8 @@ TEST_F(ServiceTest, DoPropertiesMatch) {
     args.SetBool(kAutoConnectProperty, true);
     args.SetInt(kPriorityProperty, kPriority0);
     args.SetStrings(ServiceUnderTest::kStringsProperty, kStrings0);
+    args.SetKeyValueStore(ServiceUnderTest::kKeyValueStoreProperty,
+                          key_value_store0);
     EXPECT_FALSE(service_->DoPropertiesMatch(args));
   }
   {
@@ -1252,6 +1279,8 @@ TEST_F(ServiceTest, DoPropertiesMatch) {
     args.SetBool(kAutoConnectProperty, false);
     args.SetInt(kPriorityProperty, kPriority1);
     args.SetStrings(ServiceUnderTest::kStringsProperty, kStrings0);
+    args.SetKeyValueStore(ServiceUnderTest::kKeyValueStoreProperty,
+                          key_value_store0);
     EXPECT_FALSE(service_->DoPropertiesMatch(args));
   }
   {
@@ -1260,6 +1289,18 @@ TEST_F(ServiceTest, DoPropertiesMatch) {
     args.SetBool(kAutoConnectProperty, false);
     args.SetInt(kPriorityProperty, kPriority0);
     args.SetStrings(ServiceUnderTest::kStringsProperty, kStrings1);
+    args.SetKeyValueStore(ServiceUnderTest::kKeyValueStoreProperty,
+                          key_value_store0);
+    EXPECT_FALSE(service_->DoPropertiesMatch(args));
+  }
+  {
+    KeyValueStore args;
+    args.SetString(kGuidProperty, kGUID0);
+    args.SetBool(kAutoConnectProperty, false);
+    args.SetInt(kPriorityProperty, kPriority0);
+    args.SetStrings(ServiceUnderTest::kStringsProperty, kStrings0);
+    args.SetKeyValueStore(ServiceUnderTest::kKeyValueStoreProperty,
+                          key_value_store1);
     EXPECT_FALSE(service_->DoPropertiesMatch(args));
   }
 }