buffet: StorageInterface::Save accepts const reference Out codebase mostly written with assumption that const parameters passed as const reference. Looking at storage_->Save(&dict) reader may believe that dict is the destination for Save. BUG=none TEST=gizmo-emerge buffet Change-Id: I15b2b1a48e3053979ecd28a8e8775997e7b7e06d Reviewed-on: https://chromium-review.googlesource.com/270793 Commit-Queue: Vitaly Buka <vitalybuka@chromium.org> Tested-by: Vitaly Buka <vitalybuka@chromium.org> Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>

diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 0ef58d7..80e7e9b 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc

@@ -236,7 +236,7 @@
   dict.SetString(storage_keys::kAnonymousAccessRole,
                  config_->anonymous_access_role());
 
-  return storage_->Save(&dict);
+  return storage_->Save(dict);
 }
 
 void DeviceRegistrationInfo::ScheduleStartDevice(const base::TimeDelta& later) {

diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index ec68b21..417aff4 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc

@@ -186,7 +186,7 @@
  protected:
   void SetUp() override {
     storage_ = std::make_shared<MemStorage>();
-    storage_->Save(&data_);
+    storage_->Save(data_);
     transport_ = std::make_shared<chromeos::http::fake::Transport>();
     command_manager_ = std::make_shared<CommandManager>();
     state_manager_ = std::make_shared<StateManager>(&mock_state_change_queue_);
@@ -250,7 +250,7 @@
   data.SetString(storage_keys::kLocation, "m");
   data.SetString(storage_keys::kAnonymousAccessRole, "user");
 
-  storage_->Save(&data);
+  storage_->Save(data);
 
   // This test isn't really trying to test Load, it is just the easiest
   // way to initialize the properties in dev_reg_.
@@ -258,7 +258,7 @@
 
   // Clear the storage to get a clean test.
   base::DictionaryValue empty;
-  storage_->Save(&empty);
+  storage_->Save(empty);
   EXPECT_TRUE(DeviceRegistrationInfo::TestHelper::Save(dev_reg_.get()));
   EXPECT_TRUE(storage_->Load()->Equals(&data));
 }
@@ -287,7 +287,7 @@
   EXPECT_EQ(0, transport_->GetRequestCount());
 
   SetDefaultDeviceRegistration(&data_);
-  storage_->Save(&data_);
+  storage_->Save(data_);
   EXPECT_TRUE(dev_reg_->Load());
 
   transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
@@ -301,7 +301,7 @@
 
 TEST_F(DeviceRegistrationInfoTest, CheckAuthenticationFailure) {
   SetDefaultDeviceRegistration(&data_);
-  storage_->Save(&data_);
+  storage_->Save(data_);
   EXPECT_TRUE(dev_reg_->Load());
   EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus());
 
@@ -319,7 +319,7 @@
 
 TEST_F(DeviceRegistrationInfoTest, CheckDeregistration) {
   SetDefaultDeviceRegistration(&data_);
-  storage_->Save(&data_);
+  storage_->Save(data_);
   EXPECT_TRUE(dev_reg_->Load());
   EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus());
 
@@ -338,7 +338,7 @@
 
 TEST_F(DeviceRegistrationInfoTest, GetDeviceInfo) {
   SetDefaultDeviceRegistration(&data_);
-  storage_->Save(&data_);
+  storage_->Save(data_);
   EXPECT_TRUE(dev_reg_->Load());
 
   transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
@@ -360,7 +360,7 @@
 
 TEST_F(DeviceRegistrationInfoTest, GetDeviceId) {
   SetDefaultDeviceRegistration(&data_);
-  storage_->Save(&data_);
+  storage_->Save(data_);
   EXPECT_TRUE(dev_reg_->Load());
 
   transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
@@ -513,7 +513,7 @@
             dev_reg_->GetRegistrationStatus());
   // Put some credentials into our state, make sure we call that offline.
   SetDefaultDeviceRegistration(&data_);
-  storage_->Save(&data_);
+  storage_->Save(data_);
   EXPECT_TRUE(dev_reg_->Load());
   EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus());
 }

diff --git a/buffet/storage_impls.cc b/buffet/storage_impls.cc
index d95538a..e45ddbf 100644
--- a/buffet/storage_impls.cc
+++ b/buffet/storage_impls.cc

@@ -23,10 +23,10 @@
   return std::unique_ptr<base::Value>(base::JSONReader::Read(json));
 }
 
-bool FileStorage::Save(const base::Value* config) {
+bool FileStorage::Save(const base::Value& config) {
   std::string json;
   base::JSONWriter::WriteWithOptions(
-      config, base::JSONWriter::OPTIONS_PRETTY_PRINT, &json);
+      &config, base::JSONWriter::OPTIONS_PRETTY_PRINT, &json);
   return base::ImportantFileWriter::WriteFileAtomically(file_path_, json);
 }
 
@@ -35,8 +35,8 @@
   return std::unique_ptr<base::Value>(cache_->DeepCopy());
 }
 
-bool MemStorage::Save(const base::Value* config) {
-  cache_.reset(config->DeepCopy());
+bool MemStorage::Save(const base::Value& config) {
+  cache_.reset(config.DeepCopy());
   ++save_count_;
   return true;
 }

diff --git a/buffet/storage_impls.h b/buffet/storage_impls.h
index 8b90a6c..f4034be 100644
--- a/buffet/storage_impls.h
+++ b/buffet/storage_impls.h

@@ -19,7 +19,7 @@
   explicit FileStorage(const base::FilePath& file_path);
   virtual ~FileStorage() = default;
   std::unique_ptr<base::Value> Load() override;
-  bool Save(const base::Value* config) override;
+  bool Save(const base::Value& config) override;
 
  private:
   base::FilePath file_path_;
@@ -32,7 +32,7 @@
   MemStorage() = default;
   virtual ~MemStorage() = default;
   std::unique_ptr<base::Value> Load() override;
-  bool Save(const base::Value* config) override;
+  bool Save(const base::Value& config) override;
   int save_count() { return save_count_; }
   void reset_save_count() { save_count_ = 0; }
 

diff --git a/buffet/storage_interface.h b/buffet/storage_interface.h
index 26d71c1..4f05235 100644
--- a/buffet/storage_interface.h
+++ b/buffet/storage_interface.h

@@ -24,7 +24,7 @@
   // If saved successfully, returns true. Could fail when writing to
   // physical storage like file system for various reasons (out of disk space,
   // access permissions, etc).
-  virtual bool Save(const base::Value* config) = 0;
+  virtual bool Save(const base::Value& config) = 0;
 };
 
 }  // namespace buffet