Replace Get* methods returning unique_ptr with reference alternative Existing code created temporarily objects and returned them to the client. It was not efficient and error-prone as client code could retrieve pointers to internal objects without keeping unique_ptr alive. Change-Id: I9e17c8d9f66dfc9f52ce9ffc9a31992b16b00461 Reviewed-on: https://weave-review.googlesource.com/1672 Reviewed-by: Alex Vakulenko <avakulenko@google.com>

diff --git a/examples/daemon/ledflasher/ledflasher.cc b/examples/daemon/ledflasher/ledflasher.cc
index 4733f18..9e4a9e1 100644
--- a/examples/daemon/ledflasher/ledflasher.cc
+++ b/examples/daemon/ledflasher/ledflasher.cc

@@ -66,10 +66,10 @@
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
     int32_t led_index = 0;
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     bool cmd_value = false;
-    if (params->GetInteger("_led", &led_index) &&
-        params->GetBoolean("_on", &cmd_value)) {
+    if (params.GetInteger("_led", &led_index) &&
+        params.GetBoolean("_on", &cmd_value)) {
       // Display this command in terminal
       LOG(INFO) << cmd->GetName() << " _led: " << led_index
                 << ", _on: " << (cmd_value ? "true" : "false");
@@ -96,9 +96,9 @@
     if (!cmd)
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     int32_t led_index = 0;
-    if (params->GetInteger("_led", &led_index)) {
+    if (params.GetInteger("_led", &led_index)) {
       LOG(INFO) << cmd->GetName() << " _led: " << led_index;
       led_index--;
       led_status_[led_index] = ~led_status_[led_index];

diff --git a/examples/daemon/light/light.cc b/examples/daemon/light/light.cc
index 31e3a24..90680d2 100644
--- a/examples/daemon/light/light.cc
+++ b/examples/daemon/light/light.cc

@@ -122,9 +122,9 @@
     if (!cmd)
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     int32_t brightness_value = 0;
-    if (params->GetInteger("brightness", &brightness_value)) {
+    if (params.GetInteger("brightness", &brightness_value)) {
       // Display this command in terminal.
       LOG(INFO) << cmd->GetName() << " brightness: " << brightness_value;
 
@@ -146,9 +146,9 @@
     if (!cmd)
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     std::string requested_state;
-    if (params->GetString("state", &requested_state)) {
+    if (params.GetString("state", &requested_state)) {
       LOG(INFO) << cmd->GetName() << " state: " << requested_state;
 
       bool new_light_status = requested_state == "on";
@@ -172,9 +172,9 @@
     if (!cmd)
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
-    auto params = cmd->GetParameters();
-    base::DictionaryValue* colorXY = nullptr;
-    if (params->GetDictionary("_colorSetting", &colorXY)) {
+    const auto& params = cmd->GetParameters();
+    const base::DictionaryValue* colorXY = nullptr;
+    if (params.GetDictionary("_colorSetting", &colorXY)) {
       bool updateState = false;
       double X = 0.0;
       double Y = 0.0;

diff --git a/examples/daemon/lock/lock.cc b/examples/daemon/lock/lock.cc
index 860020c..10bea00 100644
--- a/examples/daemon/lock/lock.cc
+++ b/examples/daemon/lock/lock.cc

@@ -72,9 +72,9 @@
     if (!cmd)
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     std::string requested_state;
-    if (params->GetString("lockedState", &requested_state)) {
+    if (params.GetString("lockedState", &requested_state)) {
       LOG(INFO) << cmd->GetName() << " state: " << requested_state;
 
       weave::lockstate::LockState new_lock_status;

diff --git a/examples/daemon/sample/sample.cc b/examples/daemon/sample/sample.cc
index 2ad56e3..811e9fb 100644
--- a/examples/daemon/sample/sample.cc
+++ b/examples/daemon/sample/sample.cc

@@ -69,9 +69,9 @@
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
 
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     std::string name;
-    if (!params->GetString("_name", &name)) {
+    if (!params.GetString("_name", &name)) {
       weave::ErrorPtr error;
       weave::Error::AddTo(&error, FROM_HERE, "example",
                           "invalid_parameter_value", "Name is missing");
@@ -94,7 +94,7 @@
     base::DictionaryValue state;
     state.SetInteger("_sample._ping_count", ++ping_count_);
     device_->SetStateProperties(state, nullptr);
-    LOG(INFO) << "New state: " << *device_->GetState();
+    LOG(INFO) << "New state: " << device_->GetState();
 
     base::DictionaryValue result;
     cmd->Complete(result, nullptr);
@@ -108,9 +108,9 @@
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
 
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     int seconds;
-    if (!params->GetInteger("_seconds", &seconds))
+    if (!params.GetInteger("_seconds", &seconds))
       seconds = 10;
 
     LOG(INFO) << "starting countdown";
@@ -123,9 +123,9 @@
       return;
 
     if (seconds > 0) {
-      auto params = cmd->GetParameters();
+      const auto& params = cmd->GetParameters();
       std::string todo;
-      params->GetString("_todo", &todo);
+      params.GetString("_todo", &todo);
       LOG(INFO) << "countdown tick: " << seconds << " seconds left";
 
       base::DictionaryValue progress;

diff --git a/examples/daemon/speaker/speaker.cc b/examples/daemon/speaker/speaker.cc
index 89595b0..cd7d62f 100644
--- a/examples/daemon/speaker/speaker.cc
+++ b/examples/daemon/speaker/speaker.cc

@@ -72,10 +72,10 @@
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
 
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     // Handle volume parameter
     int32_t volume_value = 0;
-    if (params->GetInteger("volume", &volume_value)) {
+    if (params.GetInteger("volume", &volume_value)) {
       // Display this command in terminal.
       LOG(INFO) << cmd->GetName() << " volume: " << volume_value;
 
@@ -89,7 +89,7 @@
 
     // Handle isMuted parameter
     bool isMuted_status = false;
-    if (params->GetBoolean("isMuted", &isMuted_status)) {
+    if (params.GetBoolean("isMuted", &isMuted_status)) {
       // Display this command in terminal.
       LOG(INFO) << cmd->GetName() << " is "
                 << (isMuted_status ? "muted" : "not muted");
@@ -111,9 +111,9 @@
     if (!cmd)
       return;
     LOG(INFO) << "received command: " << cmd->GetName();
-    auto params = cmd->GetParameters();
+    const auto& params = cmd->GetParameters();
     std::string requested_state;
-    if (params->GetString("state", &requested_state)) {
+    if (params.GetString("state", &requested_state)) {
       LOG(INFO) << cmd->GetName() << " state: " << requested_state;
 
       bool new_speaker_status = requested_state == "on";

diff --git a/include/weave/command.h b/include/weave/command.h
index 59a9305..08ea782 100644
--- a/include/weave/command.h
+++ b/include/weave/command.h

@@ -40,13 +40,13 @@
   virtual Command::Origin GetOrigin() const = 0;
 
   // Returns the command parameters.
-  virtual std::unique_ptr<base::DictionaryValue> GetParameters() const = 0;
+  virtual const base::DictionaryValue& GetParameters() const = 0;
 
   // Returns the command progress.
-  virtual std::unique_ptr<base::DictionaryValue> GetProgress() const = 0;
+  virtual const base::DictionaryValue& GetProgress() const = 0;
 
   // Returns the command results.
-  virtual std::unique_ptr<base::DictionaryValue> GetResults() const = 0;
+  virtual const base::DictionaryValue& GetResults() const = 0;
 
   // Returns the command error.
   virtual const Error* GetError() const = 0;

diff --git a/include/weave/device.h b/include/weave/device.h
index 19012b5..cbcc193 100644
--- a/include/weave/device.h
+++ b/include/weave/device.h

@@ -96,7 +96,7 @@
 
   // Returns value of the single property.
   // |name| is full property name, including package name. e.g. "base.network".
-  virtual std::unique_ptr<base::Value> GetStateProperty(
+  virtual const base::Value* GetStateProperty(
       const std::string& name) const = 0;
 
   // Sets value of the single property.
@@ -106,7 +106,7 @@
                                 ErrorPtr* error) = 0;
 
   // Returns aggregated state properties across all registered packages.
-  virtual std::unique_ptr<base::DictionaryValue> GetState() const = 0;
+  virtual const base::DictionaryValue& GetState() const = 0;
 
   // Returns current state of GCD connection.
   virtual GcdState GetGcdState() const = 0;

diff --git a/include/weave/test/mock_command.h b/include/weave/test/mock_command.h
index 2b1080e..fe1a02a 100644
--- a/include/weave/test/mock_command.h
+++ b/include/weave/test/mock_command.h

@@ -25,9 +25,9 @@
   MOCK_CONST_METHOD0(GetCategory, const std::string&());
   MOCK_CONST_METHOD0(GetState, Command::State());
   MOCK_CONST_METHOD0(GetOrigin, Command::Origin());
-  MOCK_CONST_METHOD0(MockGetParameters, const std::string&());
-  MOCK_CONST_METHOD0(MockGetProgress, const std::string&());
-  MOCK_CONST_METHOD0(MockGetResults, const std::string&());
+  MOCK_CONST_METHOD0(GetParameters, const base::DictionaryValue&());
+  MOCK_CONST_METHOD0(GetProgress, const base::DictionaryValue&());
+  MOCK_CONST_METHOD0(GetResults, const base::DictionaryValue&());
   MOCK_CONST_METHOD0(GetError, const Error*());
   MOCK_METHOD2(SetProgress, bool(const base::DictionaryValue&, ErrorPtr*));
   MOCK_METHOD2(Complete, bool(const base::DictionaryValue&, ErrorPtr*));
@@ -35,10 +35,6 @@
   MOCK_METHOD2(SetError, bool(const Error*, ErrorPtr*));
   MOCK_METHOD2(Abort, bool(const Error*, ErrorPtr*));
   MOCK_METHOD1(Cancel, bool(ErrorPtr*));
-
-  std::unique_ptr<base::DictionaryValue> GetParameters() const override;
-  std::unique_ptr<base::DictionaryValue> GetProgress() const override;
-  std::unique_ptr<base::DictionaryValue> GetResults() const override;
 };
 
 }  // namespace test

diff --git a/include/weave/test/mock_device.h b/include/weave/test/mock_device.h
index f751f97..e5063e7 100644
--- a/include/weave/test/mock_device.h
+++ b/include/weave/test/mock_device.h

@@ -34,13 +34,13 @@
   MOCK_METHOD2(SetStatePropertiesFromJson, bool(const std::string&, ErrorPtr*));
   MOCK_METHOD2(SetStateProperties,
                bool(const base::DictionaryValue&, ErrorPtr*));
-  MOCK_CONST_METHOD1(MockGetStateProperty,
-                     base::Value*(const std::string& name));
+  MOCK_CONST_METHOD1(GetStateProperty,
+                     const base::Value*(const std::string& name));
   MOCK_METHOD3(SetStateProperty,
                bool(const std::string& name,
                     const base::Value& value,
                     ErrorPtr* error));
-  MOCK_CONST_METHOD0(MockGetState, base::DictionaryValue*());
+  MOCK_CONST_METHOD0(GetState, const base::DictionaryValue&());
   MOCK_CONST_METHOD0(GetGcdState, GcdState());
   MOCK_METHOD1(AddGcdStateChangedCallback,
                void(const GcdStateChangedCallback& callback));
@@ -50,15 +50,6 @@
   MOCK_METHOD2(AddPairingChangedCallbacks,
                void(const PairingBeginCallback& begin_callback,
                     const PairingEndCallback& end_callback));
-
-  // Gmock 1.7.0 does not work with unuque_ptr as return value.
-  std::unique_ptr<base::Value> GetStateProperty(
-      const std::string& name) const override {
-    return std::unique_ptr<base::Value>(MockGetStateProperty(name));
-  }
-  std::unique_ptr<base::DictionaryValue> GetState() const override {
-    return std::unique_ptr<base::DictionaryValue>(MockGetState());
-  }
 };
 
 }  // namespace test

diff --git a/libweave.gypi b/libweave.gypi
index 24d6995..a02f082 100644
--- a/libweave.gypi
+++ b/libweave.gypi

@@ -54,7 +54,6 @@
     'weave_test_sources': [
       'src/test/fake_stream.cc',
       'src/test/fake_task_runner.cc',
-      'src/test/mock_command.cc',
       'src/test/unittest_utils.cc',
     ],
     'weave_unittest_sources': [

diff --git a/src/base_api_handler.cc b/src/base_api_handler.cc
index 3d22a10..1423dd1 100644
--- a/src/base_api_handler.cc
+++ b/src/base_api_handler.cc

@@ -99,10 +99,10 @@
   bool discovery_enabled{settings.local_discovery_enabled};
   bool pairing_enabled{settings.local_pairing_enabled};
 
-  auto parameters = command->GetParameters();
-  parameters->GetString("localAnonymousAccessMaxRole", &anonymous_access_role);
-  parameters->GetBoolean("localDiscoveryEnabled", &discovery_enabled);
-  parameters->GetBoolean("localPairingEnabled", &pairing_enabled);
+  const auto& parameters = command->GetParameters();
+  parameters.GetString("localAnonymousAccessMaxRole", &anonymous_access_role);
+  parameters.GetBoolean("localDiscoveryEnabled", &discovery_enabled);
+  parameters.GetBoolean("localPairingEnabled", &pairing_enabled);
 
   AuthScope auth_scope{AuthScope::kNone};
   if (!StringToEnum(anonymous_access_role, &auth_scope)) {
@@ -144,10 +144,10 @@
   std::string description{settings.description};
   std::string location{settings.location};
 
-  auto parameters = command->GetParameters();
-  parameters->GetString("name", &name);
-  parameters->GetString("description", &description);
-  parameters->GetString("location", &location);
+  const auto& parameters = command->GetParameters();
+  parameters.GetString("name", &name);
+  parameters.GetString("description", &description);
+  parameters.GetString("location", &location);
 
   device_info_->UpdateDeviceInfo(name, description, location);
   command->Complete({}, nullptr);

diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc
index 357385d..15a575a 100644
--- a/src/base_api_handler_unittest.cc
+++ b/src/base_api_handler_unittest.cc

@@ -84,7 +84,8 @@
   }
 
   std::unique_ptr<base::DictionaryValue> GetBaseState() {
-    auto state = state_manager_->GetState();
+    std::unique_ptr<base::DictionaryValue> state{
+        state_manager_->GetState().DeepCopy()};
     std::set<std::string> result;
     for (base::DictionaryValue::Iterator it{*state}; !it.IsAtEnd();
          it.Advance()) {

diff --git a/src/commands/cloud_command_proxy.cc b/src/commands/cloud_command_proxy.cc
index 91db18c..3d472c7 100644
--- a/src/commands/cloud_command_proxy.cc
+++ b/src/commands/cloud_command_proxy.cc

@@ -43,7 +43,7 @@
 void CloudCommandProxy::OnResultsChanged() {
   std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue};
   patch->Set(commands::attributes::kCommand_Results,
-             command_instance_->GetResults().release());
+             command_instance_->GetResults().CreateDeepCopy());
   QueueCommandUpdate(std::move(patch));
 }
 
@@ -57,7 +57,7 @@
 void CloudCommandProxy::OnProgressChanged() {
   std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue};
   patch->Set(commands::attributes::kCommand_Progress,
-             command_instance_->GetProgress().release());
+             command_instance_->GetProgress().CreateDeepCopy());
   QueueCommandUpdate(std::move(patch));
 }
 

diff --git a/src/commands/command_instance.cc b/src/commands/command_instance.cc
index aa71b0e..8328299 100644
--- a/src/commands/command_instance.cc
+++ b/src/commands/command_instance.cc

@@ -94,16 +94,16 @@
   return origin_;
 }
 
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetParameters() const {
-  return std::unique_ptr<base::DictionaryValue>(parameters_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetParameters() const {
+  return parameters_;
 }
 
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetProgress() const {
-  return std::unique_ptr<base::DictionaryValue>(progress_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetProgress() const {
+  return progress_;
 }
 
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetResults() const {
-  return std::unique_ptr<base::DictionaryValue>(results_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetResults() const {
+  return results_;
 }
 
 const Error* CommandInstance::GetError() const {

diff --git a/src/commands/command_instance.h b/src/commands/command_instance.h
index e7a6eca..60620a1 100644
--- a/src/commands/command_instance.h
+++ b/src/commands/command_instance.h

@@ -54,9 +54,9 @@
   const std::string& GetName() const override;
   Command::State GetState() const override;
   Command::Origin GetOrigin() const override;
-  std::unique_ptr<base::DictionaryValue> GetParameters() const override;
-  std::unique_ptr<base::DictionaryValue> GetProgress() const override;
-  std::unique_ptr<base::DictionaryValue> GetResults() const override;
+  const base::DictionaryValue& GetParameters() const override;
+  const base::DictionaryValue& GetProgress() const override;
+  const base::DictionaryValue& GetResults() const override;
   const Error* GetError() const override;
   bool SetProgress(const base::DictionaryValue& progress,
                    ErrorPtr* error) override;

diff --git a/src/commands/command_instance_unittest.cc b/src/commands/command_instance_unittest.cc
index 1d32cd9..fb8fe84 100644
--- a/src/commands/command_instance_unittest.cc
+++ b/src/commands/command_instance_unittest.cc

@@ -82,8 +82,8 @@
   EXPECT_EQ("robot.speak", instance.GetName());
   EXPECT_EQ(Command::Origin::kCloud, instance.GetOrigin());
   EXPECT_JSON_EQ("{'phrase': 'iPityDaFool', 'volume': 5}",
-                 *instance.GetParameters());
-  EXPECT_JSON_EQ("{'foo': 239}", *instance.GetResults());
+                 instance.GetParameters());
+  EXPECT_JSON_EQ("{'foo': 239}", instance.GetResults());
 
   CommandInstance instance2{"base.reboot",
                             Command::Origin::kLocal,
@@ -118,7 +118,7 @@
   EXPECT_EQ("abcd", instance->GetID());
   EXPECT_EQ("robot.jump", instance->GetName());
   EXPECT_JSON_EQ("{'height': 53, '_jumpType': '_withKick'}",
-                 *instance->GetParameters());
+                 instance->GetParameters());
 }
 
 TEST_F(CommandInstanceTest, FromJson_ParamsOmitted) {
@@ -126,7 +126,7 @@
   auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
                                             dict_, nullptr, nullptr);
   EXPECT_EQ("base.reboot", instance->GetName());
-  EXPECT_JSON_EQ("{}", *instance->GetParameters());
+  EXPECT_JSON_EQ("{}", instance->GetParameters());
 }
 
 TEST_F(CommandInstanceTest, FromJson_NotObject) {

diff --git a/src/device_manager.cc b/src/device_manager.cc
index c0b8e9a..52b2882 100644
--- a/src/device_manager.cc
+++ b/src/device_manager.cc

@@ -134,7 +134,7 @@
   return state_manager_->SetProperties(dict, error);
 }
 
-std::unique_ptr<base::Value> DeviceManager::GetStateProperty(
+const base::Value* DeviceManager::GetStateProperty(
     const std::string& name) const {
   return state_manager_->GetProperty(name);
 }
@@ -145,7 +145,7 @@
   return state_manager_->SetProperty(name, value, error);
 }
 
-std::unique_ptr<base::DictionaryValue> DeviceManager::GetState() const {
+const base::DictionaryValue& DeviceManager::GetState() const {
   return state_manager_->GetState();
 }
 

diff --git a/src/device_manager.h b/src/device_manager.h
index ccf8778..6c3df05 100644
--- a/src/device_manager.h
+++ b/src/device_manager.h

@@ -52,12 +52,11 @@
                                   ErrorPtr* error) override;
   bool SetStateProperties(const base::DictionaryValue& dict,
                           ErrorPtr* error) override;
-  std::unique_ptr<base::Value> GetStateProperty(
-      const std::string& name) const override;
+  const base::Value* GetStateProperty(const std::string& name) const override;
   bool SetStateProperty(const std::string& name,
                         const base::Value& value,
                         ErrorPtr* error) override;
-  std::unique_ptr<base::DictionaryValue> GetState() const override;
+  const base::DictionaryValue& GetState() const override;
   void Register(const std::string& ticket_id,
                 const DoneCallback& callback) override;
   GcdState GetGcdState() const override;

diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc
index 5c8625a..0c914b7 100644
--- a/src/device_registration_info.cc
+++ b/src/device_registration_info.cc

@@ -485,8 +485,7 @@
   if (!commands)
     return nullptr;
 
-  std::unique_ptr<base::DictionaryValue> state = state_manager_->GetState();
-  CHECK(state);
+  const base::DictionaryValue& state = state_manager_->GetState();
 
   std::unique_ptr<base::DictionaryValue> resource{new base::DictionaryValue};
   if (!GetSettings().cloud_id.empty())
@@ -507,7 +506,7 @@
   }
   resource->Set("channel", channel.release());
   resource->Set("commandDefs", commands.release());
-  resource->Set("state", state.release());
+  resource->Set("state", state.DeepCopy());
 
   return resource;
 }

diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc
index efb69bd..c771366 100644
--- a/src/privet/cloud_delegate.cc
+++ b/src/privet/cloud_delegate.cc

@@ -243,9 +243,8 @@
 
   void OnStateChanged() {
     state_.Clear();
-    auto state = state_manager_->GetState();
-    CHECK(state);
-    state_.MergeDictionary(state.get());
+    const auto& state = state_manager_->GetState();
+    state_.MergeDictionary(&state);
     NotifyOnStateChanged();
   }
 

diff --git a/src/states/state_manager.cc b/src/states/state_manager.cc
index b1e70a3..b170b86 100644
--- a/src/states/state_manager.cc
+++ b/src/states/state_manager.cc

@@ -27,14 +27,17 @@
   callback.Run();  // Force to read current state.
 }
 
-std::unique_ptr<base::DictionaryValue> StateManager::GetState() const {
-  std::unique_ptr<base::DictionaryValue> dict{new base::DictionaryValue};
-  for (const auto& pair : packages_) {
-    auto pkg_value = pair.second->GetValuesAsJson();
-    CHECK(pkg_value);
-    dict->SetWithoutPathExpansion(pair.first, pkg_value.release());
+const base::DictionaryValue& StateManager::GetState() const {
+  if (!cached_dict_valid_) {
+    cached_dict_.Clear();
+    for (const auto& pair : packages_) {
+      cached_dict_.SetWithoutPathExpansion(
+          pair.first, pair.second->GetValuesAsJson().DeepCopy());
+    }
+    cached_dict_valid_ = true;
   }
-  return dict;
+
+  return cached_dict_;
 }
 
 bool StateManager::SetProperty(const std::string& name,
@@ -46,8 +49,7 @@
   return result;
 }
 
-std::unique_ptr<base::Value> StateManager::GetProperty(
-    const std::string& name) const {
+const base::Value* StateManager::GetProperty(const std::string& name) const {
   auto parts = SplitAtFirst(name, ".", true);
   const std::string& package_name = parts.first;
   const std::string& property_name = parts.second;
@@ -93,6 +95,8 @@
   if (!package->SetPropertyValue(property_name, value, error))
     return false;
 
+  cached_dict_valid_ = false;
+
   std::unique_ptr<base::DictionaryValue> prop_set{new base::DictionaryValue};
   prop_set->Set(full_property_name, value.DeepCopy());
   state_change_queue_->NotifyPropertiesUpdated(timestamp, std::move(prop_set));

diff --git a/src/states/state_manager.h b/src/states/state_manager.h
index 55faf76..c48a57c 100644
--- a/src/states/state_manager.h
+++ b/src/states/state_manager.h

@@ -39,11 +39,11 @@
   bool LoadStateDefinitionFromJson(const std::string& json, ErrorPtr* error);
   bool SetProperties(const base::DictionaryValue& dict, ErrorPtr* error);
   bool SetPropertiesFromJson(const std::string& json, ErrorPtr* error);
-  std::unique_ptr<base::Value> GetProperty(const std::string& name) const;
+  const base::Value* GetProperty(const std::string& name) const;
   bool SetProperty(const std::string& name,
                    const base::Value& value,
                    ErrorPtr* error);
-  std::unique_ptr<base::DictionaryValue> GetState() const;
+  const base::DictionaryValue& GetState() const;
 
   // Returns the recorded state changes since last time this method has been
   // called.
@@ -80,6 +80,9 @@
 
   std::vector<base::Closure> on_changed_;
 
+  mutable base::DictionaryValue cached_dict_;
+  mutable bool cached_dict_valid_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(StateManager);
 };
 

diff --git a/src/states/state_manager_unittest.cc b/src/states/state_manager_unittest.cc
index f655e84..6899646 100644
--- a/src/states/state_manager_unittest.cc
+++ b/src/states/state_manager_unittest.cc

@@ -109,7 +109,7 @@
     },
     'device': {}
   })";
-  EXPECT_JSON_EQ(expected, *mgr_->GetState());
+  EXPECT_JSON_EQ(expected, mgr_->GetState());
 }
 
 TEST_F(StateManagerTest, LoadStateDefinition) {
@@ -128,7 +128,7 @@
     'power': {},
     'device': {}
   })";
-  EXPECT_JSON_EQ(expected, *mgr_->GetState());
+  EXPECT_JSON_EQ(expected, mgr_->GetState());
 }
 
 TEST_F(StateManagerTest, Startup) {
@@ -170,7 +170,7 @@
       'battery_level': 44
     }
   })";
-  EXPECT_JSON_EQ(expected, *manager.GetState());
+  EXPECT_JSON_EQ(expected, manager.GetState());
 }
 
 TEST_F(StateManagerTest, SetPropertyValue) {
@@ -189,7 +189,7 @@
       'state_property': 'Test Value'
     }
   })";
-  EXPECT_JSON_EQ(expected, *mgr_->GetState());
+  EXPECT_JSON_EQ(expected, mgr_->GetState());
 }
 
 TEST_F(StateManagerTest, SetPropertyValue_Error_NoName) {
@@ -227,7 +227,7 @@
   ASSERT_TRUE(SetPropertyValue("device.state_property",
                                base::StringValue{"Test Value"}, nullptr));
   std::vector<StateChange> expected_state;
-  const std::string expected_val = 
+  const std::string expected_val =
       "{'device': {'state_property': 'Test Value'}}";
   expected_state.emplace_back(
       timestamp_,
@@ -259,7 +259,7 @@
     },
     'device': {}
   })";
-  EXPECT_JSON_EQ(expected, *mgr_->GetState());
+  EXPECT_JSON_EQ(expected, mgr_->GetState());
 }
 
 TEST_F(StateManagerTest, GetProperty) {

diff --git a/src/states/state_package.cc b/src/states/state_package.cc
index 55650b6..b0ea199 100644
--- a/src/states/state_package.cc
+++ b/src/states/state_package.cc

@@ -39,11 +39,11 @@
   return true;
 }
 
-std::unique_ptr<base::DictionaryValue> StatePackage::GetValuesAsJson() const {
-  return std::unique_ptr<base::DictionaryValue>(values_.DeepCopy());
+const base::DictionaryValue& StatePackage::GetValuesAsJson() const {
+  return values_;
 }
 
-std::unique_ptr<base::Value> StatePackage::GetPropertyValue(
+const base::Value* StatePackage::GetPropertyValue(
     const std::string& property_name,
     ErrorPtr* error) const {
   const base::Value* value = nullptr;
@@ -55,7 +55,7 @@
     return nullptr;
   }
 
-  return std::unique_ptr<base::Value>(value->DeepCopy());
+  return value;
 }
 
 bool StatePackage::SetPropertyValue(const std::string& property_name,

diff --git a/src/states/state_package.h b/src/states/state_package.h
index 7632145..4092948 100644
--- a/src/states/state_package.h
+++ b/src/states/state_package.h

@@ -43,13 +43,12 @@
   //      "message": "Printer low on cyan ink"
   //    }
   //  }
-  std::unique_ptr<base::DictionaryValue> GetValuesAsJson() const;
+  const base::DictionaryValue& GetValuesAsJson() const;
 
   // Gets the value for a specific state property. |property_name| must not
   // include the package name as part of the property name.
-  std::unique_ptr<base::Value> GetPropertyValue(
-      const std::string& property_name,
-      ErrorPtr* error) const;
+  const base::Value* GetPropertyValue(const std::string& property_name,
+                                      ErrorPtr* error) const;
   // Sets the value for a specific state property. |property_name| must not
   // include the package name as part of the property name.
   bool SetPropertyValue(const std::string& property_name,

diff --git a/src/states/state_package_unittest.cc b/src/states/state_package_unittest.cc
index c116b37..d09625a 100644
--- a/src/states/state_package_unittest.cc
+++ b/src/states/state_package_unittest.cc

@@ -131,7 +131,7 @@
   })";
   EXPECT_JSON_EQ(expected, GetTypes(package));
 
-  EXPECT_JSON_EQ("{}", *package.GetValuesAsJson());
+  EXPECT_JSON_EQ("{}", package.GetValuesAsJson());
 }
 
 TEST(StatePackage, AddValuesFromJson_OnEmpty) {
@@ -148,7 +148,7 @@
     'iso': 200,
     'light': true
   })";
-  EXPECT_JSON_EQ(expected, *package.GetValuesAsJson());
+  EXPECT_JSON_EQ(expected, package.GetValuesAsJson());
 }
 
 TEST_F(StatePackageTest, AddSchemaFromJson_AddMore) {
@@ -200,7 +200,7 @@
     'iso': 200,
     'light': true
   })";
-  EXPECT_JSON_EQ(expected, *package_->GetValuesAsJson());
+  EXPECT_JSON_EQ(expected, package_->GetValuesAsJson());
 }
 
 TEST_F(StatePackageTest, AddValuesFromJson_AddMore) {
@@ -222,7 +222,7 @@
     'iso': 200,
     'light': true
   })";
-  EXPECT_JSON_EQ(expected, *package_->GetValuesAsJson());
+  EXPECT_JSON_EQ(expected, package_->GetValuesAsJson());
 }
 
 TEST_F(StatePackageTest, AddSchemaFromJson_Error_Redefined) {
@@ -283,7 +283,7 @@
     'iso': 200,
     'light': true
   })";
-  EXPECT_JSON_EQ(expected, *package_->GetValuesAsJson());
+  EXPECT_JSON_EQ(expected, package_->GetValuesAsJson());
 }
 
 }  // namespace weave

diff --git a/src/test/mock_command.cc b/src/test/mock_command.cc
deleted file mode 100644
index 65c2bab..0000000
--- a/src/test/mock_command.cc
+++ /dev/null

@@ -1,29 +0,0 @@
-// Copyright 2015 The Weave Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <weave/test/mock_command.h>
-
-#include <memory>
-#include <string>
-
-#include <base/values.h>
-#include <weave/test/unittest_utils.h>
-
-namespace weave {
-namespace test {
-
-std::unique_ptr<base::DictionaryValue> MockCommand::GetParameters() const {
-  return CreateDictionaryValue(MockGetParameters());
-}
-
-std::unique_ptr<base::DictionaryValue> MockCommand::GetProgress() const {
-  return CreateDictionaryValue(MockGetProgress());
-}
-
-std::unique_ptr<base::DictionaryValue> MockCommand::GetResults() const {
-  return CreateDictionaryValue(MockGetResults());
-}
-
-}  // namespace test
-}  // namespace weave