Fix Me2Me host to properly reload config when it changes.

Fix Me2Me host to properly reload config when it changes.

BUG=140578
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150262

[simonmorris, 2012-08-07 00:43:34]

lgtm with optional suggestions

http://codereview.chromium.org/10831184/diff/1004/remoting/host/composite_hos...
File remoting/host/composite_host_config.h (right):

http://codereview.chromium.org/10831184/diff/1004/remoting/host/composite_hos...
remoting/host/composite_host_config.h:31: // Reloads all configuration files.
Returns false if it fails to load one
one -> any

http://codereview.chromium.org/10831184/diff/1004/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10831184/diff/1004/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:149: if (config_.Reload() && LoadConfig())
{
I think it's more natural to include the call to Reload() within LoadConfig().
Then LoadConfig() is doing the loading, and the call to Reload() won't be
forgotten if LoadConfig() is called from somewhere else.

[Sergey Ulanov, 2012-08-07 00:57:03]

ptal.

http://codereview.chromium.org/10831184/diff/1004/remoting/host/composite_hos...
File remoting/host/composite_host_config.h (right):

http://codereview.chromium.org/10831184/diff/1004/remoting/host/composite_hos...
remoting/host/composite_host_config.h:31: // Reloads all configuration files.
Returns false if it fails to load one
On 2012/08/07 00:43:35, simonmorris wrote:
> one -> any

Done.

http://codereview.chromium.org/10831184/diff/1004/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10831184/diff/1004/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:149: if (config_.Reload() && LoadConfig())
{
On 2012/08/07 00:43:35, simonmorris wrote:
> I think it's more natural to include the call to Reload() within LoadConfig().
> Then LoadConfig() is doing the loading, and the call to Reload() won't be
> forgotten if LoadConfig() is called from somewhere else.

Done. Also changed CompositeHostConfig::AddConfigPath() not to load config.
Instead they are loaded only from CompositeHostConfig::Read().

[simonmorris, 2012-08-07 01:03:02]

lgtm after comments are addressed

http://codereview.chromium.org/10831184/diff/6001/remoting/host/composite_hos...
File remoting/host/composite_host_config.h (right):

http://codereview.chromium.org/10831184/diff/6001/remoting/host/composite_hos...
remoting/host/composite_host_config.h:30: // Reloads all configuration files.
Returns false if it fails to load any
Reloads -> Reads

http://codereview.chromium.org/10831184/diff/6001/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10831184/diff/6001/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:128: return true;
true -> LoadConfig() ?

[Sergey Ulanov, 2012-08-07 01:22:43]

http://codereview.chromium.org/10831184/diff/6001/remoting/host/composite_hos...
File remoting/host/composite_host_config.h (right):

http://codereview.chromium.org/10831184/diff/6001/remoting/host/composite_hos...
remoting/host/composite_host_config.h:30: // Reloads all configuration files.
Returns false if it fails to load any
On 2012/08/07 01:03:02, simonmorris wrote:
> Reloads -> Reads

Done.

http://codereview.chromium.org/10831184/diff/6001/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10831184/diff/6001/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:128: return true;
On 2012/08/07 01:03:02, simonmorris wrote:
> true -> LoadConfig() ?

Run() will call LoadConfig() - don't think we need to do it here.

[commit-bot: I haz the power, 2012-08-07 01:23:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10831184/10001

[commit-bot: I haz the power, 2012-08-07 04:03:56]

Change committed as 150262

[simonmorris, 2012-08-07 15:48:58]

fyi

http://codereview.chromium.org/10831184/diff/6001/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10831184/diff/6001/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:128: return true;
On 2012/08/07 01:22:43, sergeyu wrote:
> On 2012/08/07 01:03:02, simonmorris wrote:
> > true -> LoadConfig() ?
> 
> Run() will call LoadConfig() - don't think we need to do it here. 

OK.