Remove chromoting host registration from service process. More IPCs to control

Remove chromoting host registration from service process. More IPCs to control
host state.

BUG=None
TEST=Unittests.

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

[Alpha Left Google, 2010-12-16 21:32:44]

http://codereview.chromium.org/5955001/diff/3001/chrome/browser/service/servi...
File chrome/browser/service/service_process_control.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/browser/service/servi...
chrome/browser/service/service_process_control.h:118: bool
EnableChromotingHost();
This should have a callback to get the results of enabling.

http://codereview.chromium.org/5955001/diff/3001/chrome/common/remoting/chrom...
File chrome/common/remoting/chromoting_host_info.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/common/remoting/chrom...
chrome/common/remoting/chromoting_host_info.h:18: std::string email;
email first then enabled?

http://codereview.chromium.org/5955001/diff/3001/chrome/common/service_messag...
File chrome/common/service_messages_internal.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/common/service_messag...
chrome/common/service_messages_internal.h:65: 
Add a message here so we get response of chromoting host being enabled.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/cloud_print/c...
File chrome/service/cloud_print/cloud_print_proxy.cc (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/cloud_print/c...
chrome/service/cloud_print/cloud_print_proxy.cc:17: #include
"googleurl/src/gurl.h"
why's that?

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
File chrome/service/remoting/chromoting_host_manager.cc (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
chrome/service/remoting/chromoting_host_manager.cc:41: Start();
Maybe move the logic of Start() here? I don't see Start() being called
elsewhere.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
File chrome/service/remoting/chromoting_host_manager.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
chrome/service/remoting/chromoting_host_manager.h:24: class
ChromotingHostManager
Comments here about what this class is doing.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
chrome/service/remoting/chromoting_host_manager.h:27: class Observer {
Also comments here.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
File chrome/service/service_ipc_server.cc (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
chrome/service/service_ipc_server.cc:90:
IPC_MESSAGE_HANDLER(ServiceMsg_SetChromotingHostCredentials,
Use ifdef here.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
chrome/service/service_ipc_server.cc:124: #if defined(ENABLE_REMOTING)
Use one ifdef to include there all.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
File chrome/service/service_ipc_server.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
chrome/service/service_ipc_server.h:47: void OnDisableCloudPrintProxy();
USe ifdef to enclose all chromoting methods.

[Alpha Left Google, 2010-12-16 21:36:36]

Also naming convention. Everything in chrome side refers chromoting to remoting
and I would like to stick to that name. Inside remoting namespace I know we have
chromoting* objects, but outside of that namespace should just use Remoting*

[Sergey Ulanov, 2010-12-17 00:05:43]

http://codereview.chromium.org/5955001/diff/3001/chrome/browser/service/servi...
File chrome/browser/service/service_process_control.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/browser/service/servi...
chrome/browser/service/service_process_control.h:118: bool
EnableChromotingHost();
On 2010/12/16 21:32:44, Alpha wrote:
> This should have a callback to get the results of enabling.
per our discussion, I will add other messages to monitor status and errors
later.

http://codereview.chromium.org/5955001/diff/3001/chrome/common/remoting/chrom...
File chrome/common/remoting/chromoting_host_info.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/common/remoting/chrom...
chrome/common/remoting/chromoting_host_info.h:18: std::string email;
On 2010/12/16 21:32:44, Alpha wrote:
> email first then enabled?
Done, but I don't think this makes any difference :)
also renamed it to login.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/cloud_print/c...
File chrome/service/cloud_print/cloud_print_proxy.cc (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/cloud_print/c...
chrome/service/cloud_print/cloud_print_proxy.cc:17: #include
"googleurl/src/gurl.h"
On 2010/12/16 21:32:44, Alpha wrote:
> why's that?
Because this file uses GURL. Wouldn't compile without it because some headers I
removed from service_process.h

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
File chrome/service/remoting/chromoting_host_manager.cc (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
chrome/service/remoting/chromoting_host_manager.cc:41: Start();
On 2010/12/16 21:32:44, Alpha wrote:
> Maybe move the logic of Start() here? I don't see Start() being called
> elsewhere.
Start() is also called from Enable().

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
File chrome/service/remoting/chromoting_host_manager.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
chrome/service/remoting/chromoting_host_manager.h:24: class
ChromotingHostManager
On 2010/12/16 21:32:44, Alpha wrote:
> Comments here about what this class is doing.

Done.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/remoting/chro...
chrome/service/remoting/chromoting_host_manager.h:27: class Observer {
On 2010/12/16 21:32:44, Alpha wrote:
> Also comments here.

Done.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
File chrome/service/service_ipc_server.cc (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
chrome/service/service_ipc_server.cc:90:
IPC_MESSAGE_HANDLER(ServiceMsg_SetChromotingHostCredentials,
On 2010/12/16 21:32:44, Alpha wrote:
> Use ifdef here.

Done.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
chrome/service/service_ipc_server.cc:124: #if defined(ENABLE_REMOTING)
On 2010/12/16 21:32:44, Alpha wrote:
> Use one ifdef to include there all.

Done.

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
File chrome/service/service_ipc_server.h (right):

http://codereview.chromium.org/5955001/diff/3001/chrome/service/service_ipc_s...
chrome/service/service_ipc_server.h:47: void OnDisableCloudPrintProxy();
On 2010/12/16 21:32:44, Alpha wrote:
> USe ifdef to enclose all chromoting methods.

Done.

[Sergey Ulanov, 2010-12-17 00:06:04]

On 2010/12/16 21:36:36, Alpha wrote:
> Also naming convention. Everything in chrome side refers chromoting to
remoting
> and I would like to stick to that name. Inside remoting namespace I know we
have
> chromoting* objects, but outside of that namespace should just use Remoting*

Done

[Alpha Left Google, 2010-12-21 19:59:58]

LGTM.