buffet: Fix issues in StateChangeQueue Addressed additional code review issues brought after the original code has been published. - NotifyPropertiesUpdated() now takes timestamp and property set as separate parameters to simplify implementation of StateManager. - StateChangeQueue::max_queue_size_ is made 'const' - StateChangeQueue now maintains a time-to-event map instead of a vector of StateChange. This allows to keep the events sorted by time stamp. Also adding discrete events with the same time stamp coalesces the property changes. - added a unit test for coalescing property set changes based on the timestamp. BUG=None TEST=FEATURES=test emerge-link buffet Change-Id: I309816d1f040558620fa68a844b05251d0e4319b Reviewed-on: https://chromium-review.googlesource.com/226300 Reviewed-by: Christopher Wiley <wiley@chromium.org> Reviewed-by: Anton Muhin <antonm@chromium.org> Commit-Queue: Alex Vakulenko <avakulenko@chromium.org> Tested-by: Alex Vakulenko <avakulenko@chromium.org>

diff --git a/buffet/manager.cc b/buffet/manager.cc
index d82faf6..bc9fdf8 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc

@@ -126,7 +126,10 @@
 void Manager::HandleUpdateState(
     chromeos::ErrorPtr* error,
     const chromeos::VariantDictionary& property_set) {
-  state_manager_->UpdateProperties(property_set, error);
+  base::Time timestamp = base::Time::Now();
+  for (const auto& pair : property_set) {
+    state_manager_->SetPropertyValue(pair.first, pair.second, timestamp, error);
+  }
 }
 
 void Manager::HandleAddCommand(

diff --git a/buffet/states/mock_state_change_queue_interface.h b/buffet/states/mock_state_change_queue_interface.h
index eca3720..11a84b4 100644
--- a/buffet/states/mock_state_change_queue_interface.h
+++ b/buffet/states/mock_state_change_queue_interface.h

@@ -16,7 +16,9 @@
 class MockStateChangeQueueInterface : public StateChangeQueueInterface {
  public:
   MOCK_CONST_METHOD0(IsEmpty, bool());
-  MOCK_METHOD1(NotifyPropertiesUpdated, bool(const StateChange&));
+  MOCK_METHOD2(NotifyPropertiesUpdated,
+               bool(base::Time timestamp,
+                    chromeos::VariantDictionary changed_properties));
   MOCK_METHOD0(GetAndClearRecordedStateChanges, std::vector<StateChange>());
 };
 

diff --git a/buffet/states/state_change_queue.cc b/buffet/states/state_change_queue.cc
index a288c0f..663d799 100644
--- a/buffet/states/state_change_queue.cc
+++ b/buffet/states/state_change_queue.cc

@@ -13,9 +13,21 @@
   CHECK_GT(max_queue_size_, 0) << "Max queue size must not be zero";
 }
 
-bool StateChangeQueue::NotifyPropertiesUpdated(const StateChange& change) {
+bool StateChangeQueue::NotifyPropertiesUpdated(
+    base::Time timestamp,
+    chromeos::VariantDictionary changed_properties) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  state_changes_.push_back(change);
+  auto it = state_changes_.lower_bound(timestamp);
+  if (it == state_changes_.end() || it->first != timestamp) {
+    // This timestamp doesn't exist. Insert a new element.
+    state_changes_.emplace_hint(it, timestamp, std::move(changed_properties));
+  } else {
+    // Merge the old property set and the new one.
+    // For properties that exist in both old and new property sets, keep the
+    // new values.
+    changed_properties.insert(it->second.begin(), it->second.end());
+    it->second = std::move(changed_properties);
+  }
   while (state_changes_.size() > max_queue_size_) {
     // Queue is full.
     // Merge the two oldest records into one. The merge strategy is:
@@ -26,8 +38,8 @@
     auto element_old = state_changes_.begin();
     auto element_new = std::next(element_old);
     // This will skip elements that exist in both [old] and [new].
-    element_new->property_set.insert(element_old->property_set.begin(),
-                                     element_old->property_set.end());
+    element_new->second.insert(element_old->second.begin(),
+                               element_old->second.end());
     state_changes_.erase(element_old);
   }
   return true;
@@ -35,8 +47,13 @@
 
 std::vector<StateChange> StateChangeQueue::GetAndClearRecordedStateChanges() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // Return the accumulated state changes and clear the current queue.
-  return std::move(state_changes_);
+  std::vector<StateChange> changes;
+  changes.reserve(state_changes_.size());
+  for (const auto& pair : state_changes_) {
+    changes.emplace_back(pair.first, std::move(pair.second));
+  }
+  state_changes_.clear();
+  return changes;
 }
 
 }  // namespace buffet

diff --git a/buffet/states/state_change_queue.h b/buffet/states/state_change_queue.h
index a5cde95..2ca46e6 100644
--- a/buffet/states/state_change_queue.h
+++ b/buffet/states/state_change_queue.h

@@ -5,6 +5,7 @@
 #ifndef BUFFET_STATES_STATE_CHANGE_QUEUE_H_
 #define BUFFET_STATES_STATE_CHANGE_QUEUE_H_
 
+#include <map>
 #include <vector>
 
 #include <base/macros.h>
@@ -21,7 +22,9 @@
 
   // Overrides from StateChangeQueueInterface.
   bool IsEmpty() const override { return state_changes_.empty(); }
-  bool NotifyPropertiesUpdated(const StateChange& change) override;
+  bool NotifyPropertiesUpdated(
+      base::Time timestamp,
+      chromeos::VariantDictionary changed_properties) override;
   std::vector<StateChange> GetAndClearRecordedStateChanges() override;
 
  private:
@@ -32,10 +35,10 @@
 
   // Maximum queue size. If it is full, the oldest state update records are
   // merged together until the queue size is within the size limit.
-  size_t max_queue_size_;
+  const size_t max_queue_size_;
 
   // Accumulated list of device state change notifications.
-  std::vector<StateChange> state_changes_;
+  std::map<base::Time, chromeos::VariantDictionary> state_changes_;
 
   DISALLOW_COPY_AND_ASSIGN(StateChangeQueue);
 };

diff --git a/buffet/states/state_change_queue_interface.h b/buffet/states/state_change_queue_interface.h
index 5810bff..84704dd 100644
--- a/buffet/states/state_change_queue_interface.h
+++ b/buffet/states/state_change_queue_interface.h

@@ -14,12 +14,14 @@
 
 // A simple notification record event to track device state changes.
 // The |timestamp| records the time of the state change.
-// |property_set| contains a property set with the new property values.
-// The property set contains only the properties updated at the time the event
-// was recorded.
+// |changed_properties| contains a property set with the new property values
+// which were updated at the time the event was recorded.
 struct StateChange {
+  StateChange(base::Time time,
+              chromeos::VariantDictionary properties)
+    : timestamp(time), changed_properties(std::move(properties)) {}
   base::Time timestamp;
-  chromeos::VariantDictionary property_set;
+  chromeos::VariantDictionary changed_properties;
 };
 
 // An abstract interface to StateChangeQueue to record and retrieve state
@@ -30,7 +32,9 @@
   virtual bool IsEmpty() const = 0;
 
   // Called by StateManager when device state properties are updated.
-  virtual bool NotifyPropertiesUpdated(const StateChange& change) = 0;
+  virtual bool NotifyPropertiesUpdated(
+      base::Time timestamp,
+      chromeos::VariantDictionary changed_properties) = 0;
 
   // Returns the recorded state changes since last time this method was called.
   virtual std::vector<StateChange> GetAndClearRecordedStateChanges() = 0;

diff --git a/buffet/states/state_change_queue_unittest.cc b/buffet/states/state_change_queue_unittest.cc
index 8205f77..7aa6c9f 100644
--- a/buffet/states/state_change_queue_unittest.cc
+++ b/buffet/states/state_change_queue_unittest.cc

@@ -28,63 +28,111 @@
 }
 
 TEST_F(StateChangeQueueTest, UpdateOne) {
-  StateChange change;
-  change.timestamp = base::Time::Now();
-  change.property_set.emplace("prop.name", int{23});
-  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change));
+  StateChange change{
+    base::Time::Now(),
+    chromeos::VariantDictionary{{"prop.name", int{23}}}
+  };
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change.timestamp,
+                                              change.changed_properties));
   EXPECT_FALSE(queue_->IsEmpty());
   auto changes = queue_->GetAndClearRecordedStateChanges();
   ASSERT_EQ(1, changes.size());
   EXPECT_EQ(change.timestamp, changes.front().timestamp);
-  EXPECT_EQ(change.property_set, changes.front().property_set);
+  EXPECT_EQ(change.changed_properties, changes.front().changed_properties);
   EXPECT_TRUE(queue_->IsEmpty());
   EXPECT_TRUE(queue_->GetAndClearRecordedStateChanges().empty());
 }
 
 TEST_F(StateChangeQueueTest, UpdateMany) {
-  StateChange change1;
-  change1.timestamp = base::Time::Now();
-  change1.property_set.emplace("prop.name1", int{23});
-  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change1));
-  StateChange change2;
-  change2.timestamp = base::Time::Now();
-  change2.property_set.emplace("prop.name1", int{17});
-  change2.property_set.emplace("prop.name2", double{1.0});
-  change2.property_set.emplace("prop.name3", bool{false});
-  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change2));
+  StateChange change1{
+    base::Time::Now(),
+    chromeos::VariantDictionary{{"prop.name1", int{23}}}
+  };
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change1.timestamp,
+                                              change1.changed_properties));
+  StateChange change2{
+    base::Time::Now(),
+    chromeos::VariantDictionary{
+      {"prop.name1", int{17}},
+      {"prop.name2", double{1.0}},
+      {"prop.name3", bool{false}},
+    }
+  };
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change2.timestamp,
+                                              change2.changed_properties));
   EXPECT_FALSE(queue_->IsEmpty());
   auto changes = queue_->GetAndClearRecordedStateChanges();
   ASSERT_EQ(2, changes.size());
-  EXPECT_EQ(change1.timestamp, changes.front().timestamp);
-  EXPECT_EQ(change1.property_set, changes.front().property_set);
-  EXPECT_EQ(change2.timestamp, changes.back().timestamp);
-  EXPECT_EQ(change2.property_set, changes.back().property_set);
+  EXPECT_EQ(change1.timestamp, changes[0].timestamp);
+  EXPECT_EQ(change1.changed_properties, changes[0].changed_properties);
+  EXPECT_EQ(change2.timestamp, changes[1].timestamp);
+  EXPECT_EQ(change2.changed_properties, changes[1].changed_properties);
   EXPECT_TRUE(queue_->IsEmpty());
   EXPECT_TRUE(queue_->GetAndClearRecordedStateChanges().empty());
 }
 
+TEST_F(StateChangeQueueTest, GroupByTimestamp) {
+  base::Time timestamp = base::Time::Now();
+  base::TimeDelta time_delta = base::TimeDelta::FromMinutes(1);
+
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+      timestamp,
+      chromeos::VariantDictionary{{"prop.name1", int{1}}}));
+
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+      timestamp,
+      chromeos::VariantDictionary{{"prop.name2", int{2}}}));
+
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+      timestamp,
+      chromeos::VariantDictionary{{"prop.name1", int{3}}}));
+
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+      timestamp + time_delta,
+      chromeos::VariantDictionary{{"prop.name1", int{4}}}));
+
+  auto changes = queue_->GetAndClearRecordedStateChanges();
+  ASSERT_EQ(2, changes.size());
+
+  chromeos::VariantDictionary expected1{
+    {"prop.name1", int{3}},
+    {"prop.name2", int{2}},
+  };
+  chromeos::VariantDictionary expected2{
+    {"prop.name1", int{4}},
+  };
+  EXPECT_EQ(timestamp, changes[0].timestamp);
+  EXPECT_EQ(expected1, changes[0].changed_properties);
+  EXPECT_EQ(timestamp + time_delta, changes[1].timestamp);
+  EXPECT_EQ(expected2, changes[1].changed_properties);
+}
+
 TEST_F(StateChangeQueueTest, MaxQueueSize) {
   queue_.reset(new StateChangeQueue(2));
   base::Time start_time = base::Time::Now();
-  base::TimeDelta time_delta = base::TimeDelta::FromMinutes(1);
+  base::TimeDelta time_delta1 = base::TimeDelta::FromMinutes(1);
+  base::TimeDelta time_delta2 = base::TimeDelta::FromMinutes(3);
 
-  StateChange change;
-  change.timestamp = start_time;
-  change.property_set.emplace("prop.name1", int{1});
-  change.property_set.emplace("prop.name2", int{2});
-  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change));
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+      start_time,
+      chromeos::VariantDictionary{
+        {"prop.name1", int{1}},
+        {"prop.name2", int{2}},
+      }));
 
-  change.timestamp += time_delta;
-  change.property_set.clear();
-  change.property_set.emplace("prop.name1", int{3});
-  change.property_set.emplace("prop.name3", int{4});
-  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change));
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+      start_time + time_delta1,
+      chromeos::VariantDictionary{
+        {"prop.name1", int{3}},
+        {"prop.name3", int{4}},
+      }));
 
-  change.timestamp += time_delta;
-  change.property_set.clear();
-  change.property_set.emplace("prop.name10", int{10});
-  change.property_set.emplace("prop.name11", int{11});
-  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(change));
+  ASSERT_TRUE(queue_->NotifyPropertiesUpdated(
+      start_time + time_delta2,
+      chromeos::VariantDictionary{
+        {"prop.name10", int{10}},
+        {"prop.name11", int{11}},
+      }));
 
   auto changes = queue_->GetAndClearRecordedStateChanges();
   ASSERT_EQ(2, changes.size());
@@ -94,15 +142,15 @@
     {"prop.name2", int{2}},
     {"prop.name3", int{4}},
   };
-  EXPECT_EQ(start_time + time_delta, changes.front().timestamp);
-  EXPECT_EQ(expected1, changes.front().property_set);
+  EXPECT_EQ(start_time + time_delta1, changes[0].timestamp);
+  EXPECT_EQ(expected1, changes[0].changed_properties);
 
   chromeos::VariantDictionary expected2{
     {"prop.name10", int{10}},
     {"prop.name11", int{11}},
   };
-  EXPECT_EQ(start_time + 2 * time_delta, changes.back().timestamp);
-  EXPECT_EQ(expected2, changes.back().property_set);
+  EXPECT_EQ(start_time + time_delta2, changes[1].timestamp);
+  EXPECT_EQ(expected2, changes[1].changed_properties);
 }
 
 }  // namespace buffet

diff --git a/buffet/states/state_manager.cc b/buffet/states/state_manager.cc
index f31801f..7511928 100644
--- a/buffet/states/state_manager.cc
+++ b/buffet/states/state_manager.cc

@@ -79,9 +79,10 @@
   return dict;
 }
 
-bool StateManager::UpdatePropertyValue(const std::string& full_property_name,
-                                       const chromeos::Any& value,
-                                       chromeos::ErrorPtr* error) {
+bool StateManager::SetPropertyValue(const std::string& full_property_name,
+                                    const chromeos::Any& value,
+                                    const base::Time& timestamp,
+                                    chromeos::ErrorPtr* error) {
   std::string package_name;
   std::string property_name;
   bool split = chromeos::string_utils::SplitAtFirst(
@@ -106,34 +107,11 @@
                             package_name.c_str());
     return false;
   }
-  return package->SetPropertyValue(property_name, value, error);
-}
-
-bool StateManager::SetPropertyValue(const std::string& full_property_name,
-                                    const chromeos::Any& value,
-                                    chromeos::ErrorPtr* error) {
-  if (!UpdatePropertyValue(full_property_name, value, error))
+  if (!package->SetPropertyValue(property_name, value, error))
     return false;
 
-  StateChange change;
-  change.timestamp = base::Time::Now();
-  change.property_set.emplace(full_property_name, value);
-  state_change_queue_->NotifyPropertiesUpdated(change);
-  return true;
-}
-
-bool StateManager::UpdateProperties(
-    const chromeos::VariantDictionary& property_set,
-    chromeos::ErrorPtr* error) {
-  for (const auto& pair : property_set) {
-    if (!UpdatePropertyValue(pair.first, pair.second, error))
-      return false;
-  }
-
-  StateChange change;
-  change.timestamp = base::Time::Now();
-  change.property_set = property_set;
-  state_change_queue_->NotifyPropertiesUpdated(change);
+  chromeos::VariantDictionary prop_set{{full_property_name, value}};
+  state_change_queue_->NotifyPropertiesUpdated(timestamp, prop_set);
   return true;
 }
 

diff --git a/buffet/states/state_manager.h b/buffet/states/state_manager.h
index f17e8a6..53695e2 100644
--- a/buffet/states/state_manager.h
+++ b/buffet/states/state_manager.h

@@ -21,6 +21,7 @@
 namespace base {
 class DictionaryValue;
 class FilePath;
+class Time;
 }  // namespace base
 
 namespace buffet {
@@ -45,11 +46,7 @@
   // name of the property to update in format "package.property".
   bool SetPropertyValue(const std::string& full_property_name,
                         const chromeos::Any& value,
-                        chromeos::ErrorPtr* error);
-
-  // Updates a number of state properties in one shot.
-  // |property_set| is a (full_property_name)-to-(property_value) map.
-  bool UpdateProperties(const chromeos::VariantDictionary& property_set,
+                        const base::Time& timestamp,
                         chromeos::ErrorPtr* error);
 
   // Returns all the categories the state properties are registered from.
@@ -64,11 +61,6 @@
   std::vector<StateChange> GetAndClearRecordedStateChanges();
 
  private:
-  // Helper method to be used with SetPropertyValue() and UpdateProperties()
-  bool UpdatePropertyValue(const std::string& full_property_name,
-                           const chromeos::Any& value,
-                           chromeos::ErrorPtr* error);
-
   // Loads a device state fragment from a JSON object. |category| represents
   // a device daemon providing the state fragment or empty string for the
   // base state fragment.

diff --git a/buffet/states/state_manager_unittest.cc b/buffet/states/state_manager_unittest.cc
index b4c3094..df2260a 100644
--- a/buffet/states/state_manager_unittest.cc
+++ b/buffet/states/state_manager_unittest.cc

@@ -44,11 +44,6 @@
   })");
 }
 
-MATCHER_P(IsStateChange, prop_set, "") {
-  return arg.property_set == prop_set &&
-         std::abs((arg.timestamp - base::Time::Now()).InSeconds()) < 2;
-}
-
 }  // anonymous namespace
 
 class StateManagerTest : public ::testing::Test {
@@ -56,7 +51,8 @@
   void SetUp() override {
     // Initial expectations.
     EXPECT_CALL(mock_state_change_queue_, IsEmpty()).Times(0);
-    EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(_)).Times(0);
+    EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(_, _))
+      .Times(0);
     EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges())
       .Times(0);
     mgr_.reset(new StateManager(&mock_state_change_queue_));
@@ -109,33 +105,22 @@
   chromeos::VariantDictionary expected_prop_set{
     {"terminator.target", std::string{"John Connor"}},
   };
+  base::Time timestamp = base::Time::Now();
   EXPECT_CALL(mock_state_change_queue_,
-              NotifyPropertiesUpdated(IsStateChange(expected_prop_set)))
+              NotifyPropertiesUpdated(timestamp, expected_prop_set))
       .WillOnce(Return(true));
   ASSERT_TRUE(mgr_->SetPropertyValue("terminator.target",
-                                     std::string{"John Connor"}, nullptr));
+                                     std::string{"John Connor"},
+                                     timestamp,
+                                     nullptr));
   EXPECT_EQ("{'base':{'manufacturer':'Skynet','serialNumber':'T1000'},"
             "'terminator':{'target':'John Connor'}}",
             ValueToString(mgr_->GetStateValuesAsJson(nullptr).get()));
 }
 
-TEST_F(StateManagerTest, UpdateProperties) {
-  chromeos::VariantDictionary prop_set{
-    {"base.serialNumber", std::string{"T1000.1"}},
-    {"terminator.target", std::string{"Sarah Connor"}},
-  };
-  EXPECT_CALL(mock_state_change_queue_,
-              NotifyPropertiesUpdated(IsStateChange(prop_set)))
-      .WillOnce(Return(true));
-  ASSERT_TRUE(mgr_->UpdateProperties(prop_set, nullptr));
-  EXPECT_EQ("{'base':{'manufacturer':'Skynet','serialNumber':'T1000.1'},"
-            "'terminator':{'target':'Sarah Connor'}}",
-            ValueToString(mgr_->GetStateValuesAsJson(nullptr).get()));
-}
-
 TEST_F(StateManagerTest, SetPropertyValue_Error_NoName) {
   chromeos::ErrorPtr error;
-  ASSERT_FALSE(mgr_->SetPropertyValue("", int{0}, &error));
+  ASSERT_FALSE(mgr_->SetPropertyValue("", int{0}, base::Time::Now(), &error));
   EXPECT_EQ(errors::state::kDomain, error->GetDomain());
   EXPECT_EQ(errors::state::kPropertyNameMissing, error->GetCode());
   EXPECT_EQ("Property name is missing", error->GetMessage());
@@ -143,7 +128,8 @@
 
 TEST_F(StateManagerTest, SetPropertyValue_Error_NoPackage) {
   chromeos::ErrorPtr error;
-  ASSERT_FALSE(mgr_->SetPropertyValue("target", int{0}, &error));
+  ASSERT_FALSE(mgr_->SetPropertyValue("target", int{0}, base::Time::Now(),
+                                      &error));
   EXPECT_EQ(errors::state::kDomain, error->GetDomain());
   EXPECT_EQ(errors::state::kPackageNameMissing, error->GetCode());
   EXPECT_EQ("Package name is missing in the property name",
@@ -152,7 +138,8 @@
 
 TEST_F(StateManagerTest, SetPropertyValue_Error_UnknownPackage) {
   chromeos::ErrorPtr error;
-  ASSERT_FALSE(mgr_->SetPropertyValue("power.level", int{0}, &error));
+  ASSERT_FALSE(mgr_->SetPropertyValue("power.level", int{0}, base::Time::Now(),
+                                      &error));
   EXPECT_EQ(errors::state::kDomain, error->GetDomain());
   EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
   EXPECT_EQ("Unknown state property package 'power'", error->GetMessage());
@@ -160,28 +147,33 @@
 
 TEST_F(StateManagerTest, SetPropertyValue_Error_UnknownProperty) {
   chromeos::ErrorPtr error;
-  ASSERT_FALSE(mgr_->SetPropertyValue("base.level", int{0}, &error));
+  ASSERT_FALSE(mgr_->SetPropertyValue("base.level", int{0}, base::Time::Now(),
+                                      &error));
   EXPECT_EQ(errors::state::kDomain, error->GetDomain());
   EXPECT_EQ(errors::state::kPropertyNotDefined, error->GetCode());
   EXPECT_EQ("State property 'base.level' is not defined", error->GetMessage());
 }
 
-TEST_F(StateManagerTest, RetrievePropertyChanges) {
-  EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(_))
+TEST_F(StateManagerTest, GetAndClearRecordedStateChanges) {
+  base::Time timestamp = base::Time::Now();
+  EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(timestamp, _))
       .WillOnce(Return(true));
   ASSERT_TRUE(mgr_->SetPropertyValue("terminator.target",
-                                     std::string{"John Connor"}, nullptr));
+                                     std::string{"John Connor"},
+                                     timestamp,
+                                     nullptr));
   std::vector<StateChange> expected_val;
-  expected_val.emplace_back();
-  expected_val.back().timestamp = base::Time::Now();
-  expected_val.back().property_set.emplace("terminator.target",
-                                           std::string{"John Connor"});
+  expected_val.emplace_back(
+      timestamp,
+      chromeos::VariantDictionary{{"terminator.target",
+                                   std::string{"John Connor"}}});
   EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges())
       .WillOnce(Return(expected_val));
   auto changes = mgr_->GetAndClearRecordedStateChanges();
   ASSERT_EQ(1, changes.size());
   EXPECT_EQ(expected_val.back().timestamp, changes.back().timestamp);
-  EXPECT_EQ(expected_val.back().property_set, changes.back().property_set);
+  EXPECT_EQ(expected_val.back().changed_properties,
+            changes.back().changed_properties);
 }