Fix Me2Me host to properly reload config when it changes.

BUG=140578


Review URL: https://chromiumcodereview.appspot.com/10831184

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

3 files changed, 40 insertions, 21 deletions

diff --git a/remoting/host/composite_host_config.cc b/remoting/host/composite_host_config.cc
index cf130a9..01f456f 100644
--- a/remoting/host/composite_host_config.cc
+++ b/remoting/host/composite_host_config.cc
@@ -14,18 +14,22 @@ CompositeHostConfig::CompositeHostConfig() {
 CompositeHostConfig::~CompositeHostConfig() {
 }
 
-bool CompositeHostConfig::AddConfigPath(const FilePath& path) {
-  scoped_ptr<JsonHostConfig> config(new JsonHostConfig(path));
-  if (!config->Read()) {
-    return false;
+void CompositeHostConfig::AddConfigPath(const FilePath& path) {
+  configs_.push_back(new JsonHostConfig(path));
+}
+
+bool CompositeHostConfig::Read() {
+  bool result = true;
+  for (ScopedVector<JsonHostConfig>::iterator it = configs_.begin();
+       it != configs_.end(); ++it) {
+    result = result && (*it)->Read();
   }
-  configs_.push_back(config.release());
-  return true;
+  return result;
 }
 
 bool CompositeHostConfig::GetString(const std::string& path,
                                     std::string* out_value) const {
-  for (std::vector<HostConfig*>::const_iterator it = configs_.begin();
+  for (ScopedVector<JsonHostConfig>::const_iterator it = configs_.begin();
        it != configs_.end(); ++it) {
     if ((*it)->GetString(path, out_value))
       return true;
@@ -35,7 +39,7 @@ bool CompositeHostConfig::GetString(const std::string& path,
 
 bool CompositeHostConfig::GetBoolean(const std::string& path,
                                      bool* out_value) const {
-  for (std::vector<HostConfig*>::const_iterator it = configs_.begin();
+  for (ScopedVector<JsonHostConfig>::const_iterator it = configs_.begin();
        it != configs_.end(); ++it) {
     if ((*it)->GetBoolean(path, out_value))
       return true;
diff --git a/remoting/host/composite_host_config.h b/remoting/host/composite_host_config.h
index 46b90b4..7bb9f31 100644
--- a/remoting/host/composite_host_config.h
+++ b/remoting/host/composite_host_config.h
@@ -14,17 +14,22 @@ class FilePath;
 
 namespace remoting {
 
+class JsonHostConfig;
+
 // CompositeConfig reads multiple configuration files and merges them together.
 class CompositeHostConfig : public HostConfig {
  public:
   CompositeHostConfig();
   virtual ~CompositeHostConfig();
 
-  // Add configuration file specified stored at |path|. Returns false if the
-  // file with the specified name doesn't exist or can't be opened. When the
-  // same parameter is present in more than one file priority is given to those
-  // that are added first.
-  bool AddConfigPath(const FilePath& path);
+  // Add configuration file specified stored at |path|. When the same parameter
+  // is present in more than one file priority is given to those that are added
+  // first.
+  void AddConfigPath(const FilePath& path);
+
+  // Reads all configuration files. Returns false if it fails to load any of the
+  // files.
+  bool Read();
 
   // HostConfig interface.
   virtual bool GetString(const std::string& path,
@@ -33,7 +38,7 @@ class CompositeHostConfig : public HostConfig {
                           bool* out_value) const OVERRIDE;
 
  private:
-  ScopedVector<HostConfig> configs_;
+  ScopedVector<JsonHostConfig> configs_;
 
   DISALLOW_COPY_AND_ASSIGN(CompositeHostConfig);
 };
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 3f90b65..658d8b5 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -116,24 +116,19 @@ class HostProcess
     FilePath default_config_dir = remoting::GetConfigDir();
     if (cmd_line->HasSwitch(kAuthConfigSwitchName)) {
       FilePath path = cmd_line->GetSwitchValuePath(kAuthConfigSwitchName);
-      if (!config_.AddConfigPath(path)) {
-        return false;
-      }
+      config_.AddConfigPath(path);
     }
 
     host_config_path_ = default_config_dir.Append(kDefaultHostConfigFile);
     if (cmd_line->HasSwitch(kHostConfigSwitchName)) {
       host_config_path_ = cmd_line->GetSwitchValuePath(kHostConfigSwitchName);
     }
-    if (!config_.AddConfigPath(host_config_path_)) {
-      return false;
-    }
+    config_.AddConfigPath(host_config_path_);
 
     return true;
   }
 
   void ConfigUpdated() {
-    // The timer should be set and cleaned up on the same thread.
     DCHECK(message_loop_.message_loop_proxy()->BelongsToCurrentThread());
 
     // Call ConfigUpdatedDelayed after a short delay, so that this object won't
@@ -145,7 +140,10 @@ class HostProcess
   }
 
   void ConfigUpdatedDelayed() {
+    DCHECK(message_loop_.message_loop_proxy()->BelongsToCurrentThread());
+
     if (LoadConfig()) {
+      // PostTask to create new authenticator factory in case PIN has changed.
       context_->network_task_runner()->PostTask(
           FROM_HERE,
           base::Bind(&HostProcess::CreateAuthenticatorFactory,
@@ -197,6 +195,7 @@ class HostProcess
   }
 
   void CreateAuthenticatorFactory() {
+    DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
     scoped_ptr<protocol::AuthenticatorFactory> factory(
         new protocol::Me2MeHostAuthenticatorFactory(
             key_pair_.GenerateCertificate(),
@@ -251,6 +250,17 @@ class HostProcess
 
   // Read host config, returning true if successful.
   bool LoadConfig() {
+    DCHECK(message_loop_.message_loop_proxy()->BelongsToCurrentThread());
+
+    // TODO(sergeyu): There is a potential race condition: this function is
+    // called on the main thread while the class members it mutates are used on
+    // the network thread. Fix it. http://crbug.com/140986 .
+
+    if (!config_.Read()) {
+      LOG(ERROR) << "Failed to read config file.";
+      return false;
+    }
+
     if (!config_.GetString(kHostIdConfigPath, &host_id_)) {
       LOG(ERROR) << "host_id is not defined in the config.";
       return false;