HostConfig interface updates.

- Simplified mutation semantic: Update() replaced with
  Save(). Set methods can be called at any time.
- JsonHostConfig now inherits from InMemoryHostConfig to
  avoid code duplication.
- Added kHostEnabledConfigPath.

BUG=None
TEST=Unittests.

Review URL: http://codereview.chromium.org/5738008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69454 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 40 insertions, 96 deletions

diff --git a/remoting/host/access_verifier_unittest.cc b/remoting/host/access_verifier_unittest.cc
index 75d5795..682cf1f 100644
--- a/remoting/host/access_verifier_unittest.cc
+++ b/remoting/host/access_verifier_unittest.cc
@@ -17,23 +17,12 @@ const char kTestJid[] = "host@domain.com";
 
 class AccessVerifierTest : public testing::Test {
  protected:
-  class TestConfigUpdater :
-      public base::RefCountedThreadSafe<TestConfigUpdater> {
-   public:
-    void DoUpdate(scoped_refptr<InMemoryHostConfig> target) {
-      target->SetString(kXmppLoginConfigPath, kTestJid);
-    }
-  };
-
   virtual void SetUp() {
     config_ = new InMemoryHostConfig();
   }
 
   void InitConfig() {
-    scoped_refptr<TestConfigUpdater> config_updater(new TestConfigUpdater());
-    config_->Update(
-        NewRunnableMethod(config_updater.get(), &TestConfigUpdater::DoUpdate,
-                          config_));
+    config_->SetString(kXmppLoginConfigPath, kTestJid);
   }
 
   scoped_refptr<InMemoryHostConfig> config_;
diff --git a/remoting/host/heartbeat_sender_unittest.cc b/remoting/host/heartbeat_sender_unittest.cc
index 3dd65ff..6e1f4fe 100644
--- a/remoting/host/heartbeat_sender_unittest.cc
+++ b/remoting/host/heartbeat_sender_unittest.cc
@@ -54,21 +54,10 @@ class MockIqRequest : public IqRequest {
 
 class HeartbeatSenderTest : public testing::Test {
  protected:
-  class TestConfigUpdater :
-      public base::RefCountedThreadSafe<TestConfigUpdater> {
-   public:
-    void DoUpdate(scoped_refptr<InMemoryHostConfig> target) {
-      target->SetString(kHostIdConfigPath, kHostId);
-      target->SetString(kPrivateKeyConfigPath, kTestHostKeyPair);
-    }
-  };
-
   virtual void SetUp() {
     config_ = new InMemoryHostConfig();
-    scoped_refptr<TestConfigUpdater> config_updater(new TestConfigUpdater());
-    config_->Update(
-        NewRunnableMethod(config_updater.get(), &TestConfigUpdater::DoUpdate,
-                          config_));
+    config_->SetString(kHostIdConfigPath, kHostId);
+    config_->SetString(kPrivateKeyConfigPath, kTestHostKeyPair);
 
     jingle_thread_.message_loop_ = &message_loop_;
 
diff --git a/remoting/host/host_config.cc b/remoting/host/host_config.cc
index a05600d..cc02b15 100644
--- a/remoting/host/host_config.cc
+++ b/remoting/host/host_config.cc
@@ -6,6 +6,7 @@
 
 namespace remoting {
 
+const char kHostEnabledConfigPath[] = "enabled";
 const char kXmppLoginConfigPath[] = "xmpp_login";
 const char kXmppAuthTokenConfigPath[] = "xmpp_auth_token";
 const char kHostIdConfigPath[] = "host_id";
diff --git a/remoting/host/host_config.h b/remoting/host/host_config.h
index 7223767..bf62b7a 100644
--- a/remoting/host/host_config.h
+++ b/remoting/host/host_config.h
@@ -15,6 +15,8 @@ namespace remoting {
 
 // Following constants define names for configuration parameters.
 
+// Status of the host, whether it is enabled or disabled.
+extern const char kHostEnabledConfigPath[];
 // Login used to authenticate in XMPP network.
 extern const char kXmppLoginConfigPath[];
 // Auth token used to authenticate in XMPP network.
@@ -34,26 +36,24 @@ class HostConfig : public base::RefCountedThreadSafe<HostConfig> {
   virtual ~HostConfig() { }
 
   virtual bool GetString(const std::string& path, std::string* out_value) = 0;
+  virtual bool GetBoolean(const std::string& path, bool* out_value) = 0;
 
   DISALLOW_COPY_AND_ASSIGN(HostConfig);
 };
 
 // MutableHostConfig extends HostConfig for mutability.
-//
-// TODO(sergeyu): Simplify this interface.
 class MutableHostConfig : public HostConfig {
  public:
   MutableHostConfig() { };
 
-  // Update() must be used to update config values.
-  // It acquires lock, calls the specified task, releases the lock and
-  // then schedules the config to be written to storage.
-  virtual void Update(Task* task) = 0;
-
-  // SetString() updates specified config value. This methods must only
-  // be called from task specified in Update().
+  // SetString() updates specified config value. Save() must be called to save
+  // the changes on the disk.
   virtual void SetString(const std::string& path,
                          const std::string& in_value) = 0;
+  virtual void SetBoolean(const std::string& path, bool in_value) = 0;
+
+  // Save's changes.
+  virtual void Save() = 0;
 
   DISALLOW_COPY_AND_ASSIGN(MutableHostConfig);
 };
diff --git a/remoting/host/host_key_pair.cc b/remoting/host/host_key_pair.cc
index cabac20..1255892 100644
--- a/remoting/host/host_key_pair.cc
+++ b/remoting/host/host_key_pair.cc
@@ -54,13 +54,6 @@ void HostKeyPair::Save(MutableHostConfig* host_config) {
   // Check that the key initialized.
   DCHECK(key_.get() != NULL);
 
-  host_config->Update(
-      NewRunnableMethod(this,
-                        &HostKeyPair::DoSave,
-                        make_scoped_refptr(host_config)));
-}
-
-void HostKeyPair::DoSave(MutableHostConfig* host_config) const {
   std::vector<uint8> key_buf;
   key_->ExportPrivateKey(&key_buf);
   std::string key_str(key_buf.begin(), key_buf.end());
diff --git a/remoting/host/host_key_pair.h b/remoting/host/host_key_pair.h
index 92ee4aa..4ae3fde 100644
--- a/remoting/host/host_key_pair.h
+++ b/remoting/host/host_key_pair.h
@@ -34,8 +34,6 @@ class HostKeyPair {
   std::string GetSignature(const std::string& message) const;
 
  private:
-  void DoSave(MutableHostConfig* host_config) const;
-
   scoped_ptr<base::RSAPrivateKey> key_;
 };
 
diff --git a/remoting/host/in_memory_host_config.cc b/remoting/host/in_memory_host_config.cc
index e0cf120..7827b8f 100644
--- a/remoting/host/in_memory_host_config.cc
+++ b/remoting/host/in_memory_host_config.cc
@@ -21,18 +21,24 @@ bool InMemoryHostConfig::GetString(const std::string& path,
   return values_->GetString(path, out_value);
 }
 
-void InMemoryHostConfig::Update(Task* task) {
-  {
-    AutoLock auto_lock(lock_);
-    task->Run();
-  }
-  delete task;
+bool InMemoryHostConfig::GetBoolean(const std::string& path, bool* out_value) {
+  AutoLock auto_lock(lock_);
+  return values_->GetBoolean(path, out_value);
+}
+
+void InMemoryHostConfig::Save() {
+  // Save is NOP for in-memory host config.
 }
 
 void InMemoryHostConfig::SetString(const std::string& path,
                                    const std::string& in_value) {
-  lock_.AssertAcquired();
+  AutoLock auto_lock(lock_);
   values_->SetString(path, in_value);
 }
 
+void InMemoryHostConfig::SetBoolean(const std::string& path, bool in_value) {
+  AutoLock auto_lock(lock_);
+  values_->SetBoolean(path, in_value);
+}
+
 }  // namespace remoting
diff --git a/remoting/host/in_memory_host_config.h b/remoting/host/in_memory_host_config.h
index 511998d..1078c50 100644
--- a/remoting/host/in_memory_host_config.h
+++ b/remoting/host/in_memory_host_config.h
@@ -25,15 +25,19 @@ class InMemoryHostConfig : public MutableHostConfig {
 
   // MutableHostConfig interface.
   virtual bool GetString(const std::string& path, std::string* out_value);
-
-  virtual void Update(Task* task);
+  virtual bool GetBoolean(const std::string& path, bool* out_value);
 
   virtual void SetString(const std::string& path, const std::string& in_value);
+  virtual void SetBoolean(const std::string& path, bool in_value);
 
- private:
+  virtual void Save();
+
+ protected:
+  // |lock_| must be locked whenever |values_| is used.
   Lock lock_;
   scoped_ptr<DictionaryValue> values_;
 
+ private:
   DISALLOW_COPY_AND_ASSIGN(InMemoryHostConfig);
 };
 
diff --git a/remoting/host/json_host_config.cc b/remoting/host/json_host_config.cc
index f97b343..ab639f0 100644
--- a/remoting/host/json_host_config.cc
+++ b/remoting/host/json_host_config.cc
@@ -17,7 +17,6 @@ JsonHostConfig::JsonHostConfig(
     const FilePath& filename,
     base::MessageLoopProxy* file_message_loop_proxy)
     : filename_(filename),
-      values_(new DictionaryValue()),
       message_loop_proxy_(file_message_loop_proxy) {
 }
 
@@ -41,32 +40,15 @@ bool JsonHostConfig::Read() {
   return true;
 }
 
-bool JsonHostConfig::GetString(const std::string& path,
-                               std::string* out_value) {
-  AutoLock auto_lock(lock_);
-  return values_->GetString(path, out_value);
-}
-
-void JsonHostConfig::Update(Task* task) {
-  {
-    AutoLock auto_lock(lock_);
-    task->Run();
-  }
-  delete task;
+void JsonHostConfig::Save() {
   message_loop_proxy_->PostTask(
       FROM_HERE, NewRunnableMethod(this, &JsonHostConfig::DoWrite));
 }
 
-void JsonHostConfig::SetString(const std::string& path,
-                               const std::string& in_value) {
-  lock_.AssertAcquired();
-  values_->SetString(path, in_value);
-}
-
 void JsonHostConfig::DoWrite() {
   std::string file_content;
-  base::JSONWriter::Write(values_.get(), true, &file_content);
   AutoLock auto_lock(lock_);
+  base::JSONWriter::Write(values_.get(), true, &file_content);
   // TODO(sergeyu): Move ImportantFileWriter to base and use it here.
   file_util::WriteFile(filename_, file_content.c_str(), file_content.size());
 }
diff --git a/remoting/host/json_host_config.h b/remoting/host/json_host_config.h
index dc892e9..b4e2912 100644
--- a/remoting/host/json_host_config.h
+++ b/remoting/host/json_host_config.h
@@ -11,7 +11,7 @@
 #include "base/lock.h"
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
-#include "remoting/host/host_config.h"
+#include "remoting/host/in_memory_host_config.h"
 
 class DictionaryValue;
 class Task;
@@ -23,7 +23,7 @@ class MessageLoopProxy;
 namespace remoting {
 
 // JsonHostConfig implements MutableHostConfig for JSON file.
-class JsonHostConfig : public MutableHostConfig {
+class JsonHostConfig : public InMemoryHostConfig {
  public:
   JsonHostConfig(const FilePath& pref_filename,
                  base::MessageLoopProxy* file_message_loop_proxy);
@@ -32,19 +32,12 @@ class JsonHostConfig : public MutableHostConfig {
   virtual bool Read();
 
   // MutableHostConfig interface.
-  virtual bool GetString(const std::string& path, std::string* out_value);
-
-  virtual void Update(Task* task);
-
-  virtual void SetString(const std::string& path, const std::string& in_value);
+  virtual void Save();
 
  private:
   void DoWrite();
 
-  // |lock_| must be locked whenever we access values_;
-  Lock lock_;
   FilePath filename_;
-  scoped_ptr<DictionaryValue> values_;
   scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
 
   DISALLOW_COPY_AND_ASSIGN(JsonHostConfig);
diff --git a/remoting/host/json_host_config_unittest.cc b/remoting/host/json_host_config_unittest.cc
index 14ee9bd..20ea501 100644
--- a/remoting/host/json_host_config_unittest.cc
+++ b/remoting/host/json_host_config_unittest.cc
@@ -22,15 +22,6 @@ const char* kTestConfig =
 "  \"host_name\" : \"TEST_MACHINE_NAME\",\n"
 "  \"private_key\" : \"TEST_PRIVATE_KEY\"\n"
 "}\n";
-
-// Helper for Update test.
-class TestConfigUpdater : public base::RefCountedThreadSafe<TestConfigUpdater> {
- public:
-  void DoUpdate(scoped_refptr<JsonHostConfig> target,
-                  std::string new_auth_token_value) {
-    target->SetString(kXmppAuthTokenConfigPath, new_auth_token_value);
-  }
-};
 }  // namespace
 
 class JsonHostConfigTest : public testing::Test {
@@ -92,10 +83,8 @@ TEST_F(JsonHostConfigTest, Write) {
   ASSERT_TRUE(target->Read());
 
   std::string new_auth_token_value = "NEW_AUTH_TOKEN";
-  scoped_refptr<TestConfigUpdater> config_updater(new TestConfigUpdater());
-  target->Update(
-      NewRunnableMethod(config_updater.get(), &TestConfigUpdater::DoUpdate,
-                        target, new_auth_token_value));
+  target->SetString(kXmppAuthTokenConfigPath, new_auth_token_value);
+  target->Save();
 
   message_loop_.RunAllPending();