Updated remoting host to create instances of GCD-related classes.

Updated remoting host to create instances of GCD-related classes.

BUG=471928
Committed: https://crrev.com/1d33a9e1e52db20c8f142970b242e3ce997ff133
Cr-Commit-Position: refs/heads/master@{#333640}

[John Williams, 2015-05-06 18:51:31]

jrw@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Sergey Ulanov, 2015-05-07 21:49:55]

https://codereview.chromium.org/1123163002/diff/1/remoting/host/host_signalin...
File remoting/host/host_signaling_manager.h (right):

https://codereview.chromium.org/1123163002/diff/1/remoting/host/host_signalin...
remoting/host/host_signaling_manager.h:33: class HostSignalingManager {
This class doesn't exist anymore. Please sync with trunk

https://codereview.chromium.org/1123163002/diff/1/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

https://codereview.chromium.org/1123163002/diff/1/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:365: scoped_ptr<OAuthTokenGetter>
oauth_token_getter_;
Comment that this object must outlive gcd_rest_client_ and
host_signaling_manager_

https://codereview.chromium.org/1123163002/diff/1/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:1340: DCHECK(!host_id_.empty());  //
|ApplyConfig| should already have been run.
s/|ApplyConfig|/ApplyConfig()/ 
FunctionName() vs |variable_name|

https://codereview.chromium.org/1123163002/diff/1/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:1508: host_signaling_manager_ =
CreateHostSignalingManager();
Do you need to call CreateOAuthTokenGetter() here?
I think you want to merge with trunk where I removed
CreateHostSignalingManager() and replaced it with IntializeSignaling() I suggest
moving initialization of GcdRestClient and OAuthTokenGetter inside that
function.

[John Williams, 2015-05-20 00:22:45]

https://codereview.chromium.org/1123163002/diff/1/remoting/host/host_signalin...
File remoting/host/host_signaling_manager.h (right):

https://codereview.chromium.org/1123163002/diff/1/remoting/host/host_signalin...
remoting/host/host_signaling_manager.h:33: class HostSignalingManager {
On 2015/05/07 21:49:55, Sergey Ulanov wrote:
> This class doesn't exist anymore. Please sync with trunk

Done.

https://codereview.chromium.org/1123163002/diff/1/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

https://codereview.chromium.org/1123163002/diff/1/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:365: scoped_ptr<OAuthTokenGetter>
oauth_token_getter_;
On 2015/05/07 21:49:55, Sergey Ulanov wrote:
> Comment that this object must outlive gcd_rest_client_ and
> host_signaling_manager_

Done.

https://codereview.chromium.org/1123163002/diff/1/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:1340: DCHECK(!host_id_.empty());  //
|ApplyConfig| should already have been run.
On 2015/05/07 21:49:55, Sergey Ulanov wrote:
> s/|ApplyConfig|/ApplyConfig()/ 
> FunctionName() vs |variable_name|

Done.

https://codereview.chromium.org/1123163002/diff/1/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:1508: host_signaling_manager_ =
CreateHostSignalingManager();
On 2015/05/07 21:49:55, Sergey Ulanov wrote:
> Do you need to call CreateOAuthTokenGetter() here?
> I think you want to merge with trunk where I removed
> CreateHostSignalingManager() and replaced it with IntializeSignaling() I
suggest
> moving initialization of GcdRestClient and OAuthTokenGetter inside that
> function.

Done.

[Sergey Ulanov, 2015-06-03 22:25:35]

https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
File remoting/host/remoting_me2me_host.cc (right):

https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
remoting/host/remoting_me2me_host.cc:368: scoped_ptr<GcdRestClient>
gcd_rest_client_;
Do we really need this here? Can OAuthTokenGetter own it?

https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
remoting/host/remoting_me2me_host.cc:1378: 
remove extra empty line

[John Williams, 2015-06-06 02:55:06]

https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
File remoting/host/remoting_me2me_host.cc (right):

https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
remoting/host/remoting_me2me_host.cc:368: scoped_ptr<GcdRestClient>
gcd_rest_client_;
On 2015/06/03 22:25:35, Sergey Ulanov wrote:
> Do we really need this here? Can OAuthTokenGetter own it?

GcdStateUpdater could own it.  It might even make sense to get rid of
GcdRestClient altogether and put the functionality directly in GcdStateUpdater,
but I would rather not do that until everything is working and I'm sure the host
daemon doesn't need to make any other GCD calls.

https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
remoting/host/remoting_me2me_host.cc:1378: 
On 2015/06/03 22:25:35, Sergey Ulanov wrote:
> remove extra empty line

Done.

[Sergey Ulanov, 2015-06-08 17:02:32]

https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
File remoting/host/remoting_me2me_host.cc (right):

https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
remoting/host/remoting_me2me_host.cc:368: scoped_ptr<GcdRestClient>
gcd_rest_client_;
On 2015/06/06 02:55:06, John Williams wrote:
> On 2015/06/03 22:25:35, Sergey Ulanov wrote:
> > Do we really need this here? Can OAuthTokenGetter own it?
> 
> GcdStateUpdater could own it.  It might even make sense to get rid of
> GcdRestClient altogether and put the functionality directly in
GcdStateUpdater,
> but I would rather not do that until everything is working and I'm sure the
host
> daemon doesn't need to make any other GCD calls.

I still think it's better to move ownership of GcdRestClient to GcdStateUpdater.
GcdRestClient is stateless, so you can always create another instance if you
need to use it somewhere else.

[John Williams, 2015-06-09 19:32:21]

On 2015/06/08 17:02:32, Sergey Ulanov wrote:
>
https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
> File remoting/host/remoting_me2me_host.cc (right):
> 
>
https://codereview.chromium.org/1123163002/diff/20001/remoting/host/remoting_...
> remoting/host/remoting_me2me_host.cc:368: scoped_ptr<GcdRestClient>
> gcd_rest_client_;
> On 2015/06/06 02:55:06, John Williams wrote:
> > On 2015/06/03 22:25:35, Sergey Ulanov wrote:
> > > Do we really need this here? Can OAuthTokenGetter own it?
> > 
> > GcdStateUpdater could own it.  It might even make sense to get rid of
> > GcdRestClient altogether and put the functionality directly in
> GcdStateUpdater,
> > but I would rather not do that until everything is working and I'm sure the
> host
> > daemon doesn't need to make any other GCD calls.
> 
> I still think it's better to move ownership of GcdRestClient to
GcdStateUpdater.
> GcdRestClient is stateless, so you can always create another instance if you
> need to use it somewhere else.

Done.

I was thinking you meant to move the creation, rather than just the ownership,
to GcdRestClient.  Are you OK with creating GcdRestClient in
remoting_me2me_host.cc?

[Sergey Ulanov, 2015-06-09 20:06:55]

lgtm

[John Williams, 2015-06-09 22:50:35]

The CQ bit was checked by jrw@chromium.org

[commit-bot: I haz the power, 2015-06-09 22:52:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1123163002/100001

[commit-bot: I haz the power, 2015-06-09 23:24:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-09 23:24:33]

Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[John Williams, 2015-06-10 00:11:17]

The CQ bit was checked by jrw@chromium.org

[commit-bot: I haz the power, 2015-06-10 00:13:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1123163002/100001

[commit-bot: I haz the power, 2015-06-10 01:29:29]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-06-10 01:31:27]

Patchset 6 (id:??) landed as
https://crrev.com/1d33a9e1e52db20c8f142970b242e3ce997ff133
Cr-Commit-Position: refs/heads/master@{#333640}