Remoting daemon process to support unofficial client ID

Remoting daemon process to support unofficial client ID

Allow the use of unofficial client for the daemon process even
if it is an official build. Which client ID to be used is chosen
by the webapp.

BUG=123693
TEST=Run a dev build webapp with an official daemon


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

[Wez, 2012-04-19 00:13:03]

http://codereview.chromium.org/10116040/diff/1/remoting/host/host_config.h
File remoting/host/host_config.h (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/host_config.h#ne...
remoting/host/host_config.h:25: extern const char
kOAuthUseOfficialClientIdPath[];
nit: ...Path -> ...ConfigPath

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
remoting/host/remoting_me2me_host.cc:305: "W2ieEsG-R1gIA4MMurGrgMc_"
Could this and the official info be a static const kUnofficialOAuth2ClientInfo,
etc?

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
remoting/host/remoting_me2me_host.cc:319: client_info = unofficial_client_info;
If you move this above the #ifdef block then you don't have the fragile else
#endif else-clause arrangement.

http://codereview.chromium.org/10116040/diff/1/remoting/host/signaling_connec...
File remoting/host/signaling_connector.h (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/signaling_connec...
remoting/host/signaling_connector.h:40: OAuthClientInfo client_info;
I realise it's not your fault this structure has no documentation, but I think a
comment explaining the fields is required.

[Sergey Ulanov, 2012-04-19 01:57:32]

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
remoting/host/remoting_me2me_host.cc:316: client_info = official_client_info;
need {} here

http://codereview.chromium.org/10116040/diff/1/remoting/host/signaling_connec...
File remoting/host/signaling_connector.cc (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/signaling_connec...
remoting/host/signaling_connector.cc:38: {
Looks like this was unintentional change

[Alpha Left Google, 2012-04-20 22:13:49]

http://codereview.chromium.org/10116040/diff/1/remoting/host/host_config.h
File remoting/host/host_config.h (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/host_config.h#ne...
remoting/host/host_config.h:25: extern const char
kOAuthUseOfficialClientIdPath[];
On 2012/04/19 00:13:04, Wez wrote:
> nit: ...Path -> ...ConfigPath

Done.

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
remoting/host/remoting_me2me_host.cc:305: "W2ieEsG-R1gIA4MMurGrgMc_"
On 2012/04/19 00:13:04, Wez wrote:
> Could this and the official info be a static const
kUnofficialOAuth2ClientInfo,
> etc?

Done.

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
remoting/host/remoting_me2me_host.cc:316: client_info = official_client_info;
On 2012/04/19 01:57:32, sergeyu wrote:
> need {} here

Now the #ifdef block is much simpler and {} is not needed for a single line
content for if().

http://codereview.chromium.org/10116040/diff/1/remoting/host/remoting_me2me_h...
remoting/host/remoting_me2me_host.cc:319: client_info = unofficial_client_info;
On 2012/04/19 00:13:04, Wez wrote:
> If you move this above the #ifdef block then you don't have the fragile else
> #endif else-clause arrangement.

Done.

http://codereview.chromium.org/10116040/diff/1/remoting/host/signaling_connec...
File remoting/host/signaling_connector.cc (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/signaling_connec...
remoting/host/signaling_connector.cc:38: {
On 2012/04/19 01:57:32, sergeyu wrote:
> Looks like this was unintentional change

Done.

http://codereview.chromium.org/10116040/diff/1/remoting/host/signaling_connec...
File remoting/host/signaling_connector.h (right):

http://codereview.chromium.org/10116040/diff/1/remoting/host/signaling_connec...
remoting/host/signaling_connector.h:40: OAuthClientInfo client_info;
On 2012/04/19 00:13:04, Wez wrote:
> I realise it's not your fault this structure has no documentation, but I think
a
> comment explaining the fields is required.

I'll add comments, please see if they match. *I think* this is what the fields
are for, Sergey probably has the most context.

[Wez, 2012-04-20 22:29:46]

LGTM, but please consider the comment suggestions below.

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

http://codereview.chromium.org/10116040/diff/8001/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:259: // depending on whether this is an
official build or not.
nit: Suggest "If the client-Id type to use is not specified we default based on
the build type"

http://codereview.chromium.org/10116040/diff/8001/remoting/host/signaling_con...
File remoting/host/signaling_connector.h (right):

http://codereview.chromium.org/10116040/diff/8001/remoting/host/signaling_con...
remoting/host/signaling_connector.h:41: // ID to identify the user.
nit: Suggest "The user's account name (i.e. their email address)"

http://codereview.chromium.org/10116040/diff/8001/remoting/host/signaling_con...
remoting/host/signaling_connector.h:45: std::string refresh_token;
nit: Suggest "Token delegating authority to us to act as the user."

http://codereview.chromium.org/10116040/diff/8001/remoting/host/signaling_con...
remoting/host/signaling_connector.h:47: // ID and secret to identify the client.
nit: Suggest "Credentials identifying the application to OAuth"

[commit-bot: I haz the power, 2012-04-23 21:42:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/10116040/14001

[commit-bot: I haz the power, 2012-04-23 21:43:01]

Can't apply patch for file remoting/host/signaling_connector.cc.
While running patch -p1 --forward --force;
patching file remoting/host/signaling_connector.cc
Hunk #2 FAILED at 161.
1 out of 2 hunks FAILED -- saving rejects to file
remoting/host/signaling_connector.cc.rej

[commit-bot: I haz the power, 2012-04-23 21:56:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/10116040/14002

[commit-bot: I haz the power, 2012-04-23 22:24:58]

Try job failure for 10116040-14002 (retry) on mac_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-04-23 23:16:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/10116040/25001

[commit-bot: I haz the power, 2012-04-24 00:55:09]

Change committed as 133582