Made DesktopEnvironment responsible for creation of the disconnect window.

Refactored the disconnect window such that it can be owned by the desktop environment.

This CL adds two new classes: HostWindow and HostWindowProxy. HostWindow is a non thread-safe base class for platform-specific implementations of the disconnect and continue windows. HostWindowProxy implements thread switching logic (using a ref-counted HostWindowProxy::Core).

The combination of HostWindow and HostWindowProxy allows DesktopEnvironment to create the disconnect window on the network thread and run it on the UI thread.

This is the 1st CL (out of 2) that deprecates the HostUserInterface class and moves responsibility to create the local UI to the corresponding DesktopEnvironment instances. The follow up CL will migrate the continue window logic.
    
BUG=104544
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192473

[alexeypa (please no reviews), 2013-03-29 22:43:49]

Sergey, please take a look. Jamie, PTAL at Mac changes.

[alexeypa (please no reviews), 2013-04-01 21:55:30]

I moved the common code from DisconnectWindow and ContinueWindow to HostWindow
to make the implementations less verbose. That resulted in quite a few changes.
Please treat it as a new CL.

This CL includes DisconnectWindow, HostWindow and related changes. There will be
a follow up fixing ContinueWindow.

PTAL.

[Sergey Ulanov, 2013-04-02 19:04:22]

Previously DisconnectWindow was NonThreadSafe and was always used on UI thread.
Now each of the three implementations need to be thread-aware. We should try to
keep platform-specific code as simple as possible (i.e. single-threaded) and
then have a Proxy class that proxies calls from different thread. Is it possible
to move all threading logic to HostWindow so that it's shared between all
platforms?

https://codereview.chromium.org/13212009/diff/7008/remoting/host/basic_deskto...
File remoting/host/basic_desktop_environment.h (right):

https://codereview.chromium.org/13212009/diff/7008/remoting/host/basic_deskto...
remoting/host/basic_desktop_environment.h:41: const UiStrings* ui_strings);
Maybe just copy ui_strings, to avoid worrying about object lifetime? it's small.

https://codereview.chromium.org/13212009/diff/7008/remoting/host/basic_deskto...
remoting/host/basic_desktop_environment.h:55: const UiStrings* ui_strings()
const { return ui_strings_; }
nit: return const ref here?

https://codereview.chromium.org/13212009/diff/7008/remoting/host/basic_deskto...
remoting/host/basic_desktop_environment.h:55: const UiStrings* ui_strings()
const { return ui_strings_; }
is this method used anywhere?

https://codereview.chromium.org/13212009/diff/7008/remoting/host/disconnect_w...
File remoting/host/disconnect_window_mac.mm (right):

https://codereview.chromium.org/13212009/diff/7008/remoting/host/disconnect_w...
remoting/host/disconnect_window_mac.mm:116:
caller_task_runner()->PostTask(FROM_HERE, disconnect_callback_);
This may not be safe. disconnect_callback_ may be called after HostWindow object
is destroyed and the callback handled may not expect it. You need to post task
for DisconnectWindowMac and check that HostWindow still exists on the caller
thread before calling the callback.

https://codereview.chromium.org/13212009/diff/7008/remoting/host/host_window.h
File remoting/host/host_window.h (right):

https://codereview.chromium.org/13212009/diff/7008/remoting/host/host_window....
remoting/host/host_window.h:1: // Copyright (c) 2012 The Chromium Authors. All
rights reserved.
2013

[alexeypa (please no reviews), 2013-04-03 20:26:42]

> Previously DisconnectWindow was NonThreadSafe and was always used on UI
thread.

Good point. PTAL at the new version.

https://chromiumcodereview.appspot.com/13212009/diff/7008/remoting/host/basic...
File remoting/host/basic_desktop_environment.h (right):

https://chromiumcodereview.appspot.com/13212009/diff/7008/remoting/host/basic...
remoting/host/basic_desktop_environment.h:41: const UiStrings* ui_strings);
On 2013/04/02 19:04:22, sergeyu wrote:
> Maybe just copy ui_strings, to avoid worrying about object lifetime? it's
small.

Done.

https://chromiumcodereview.appspot.com/13212009/diff/7008/remoting/host/basic...
remoting/host/basic_desktop_environment.h:55: const UiStrings* ui_strings()
const { return ui_strings_; }
On 2013/04/02 19:04:22, sergeyu wrote:
> is this method used anywhere?

I removed this code.

https://chromiumcodereview.appspot.com/13212009/diff/7008/remoting/host/disco...
File remoting/host/disconnect_window_mac.mm (right):

https://chromiumcodereview.appspot.com/13212009/diff/7008/remoting/host/disco...
remoting/host/disconnect_window_mac.mm:116:
caller_task_runner()->PostTask(FROM_HERE, disconnect_callback_);
On 2013/04/02 19:04:22, sergeyu wrote:
> This may not be safe. disconnect_callback_ may be called after HostWindow
object
> is destroyed and the callback handled may not expect it. You need to post task
> for DisconnectWindowMac and check that HostWindow still exists on the caller
> thread before calling the callback.

This code uses a weak pointer to an interface so it should be safe.

https://chromiumcodereview.appspot.com/13212009/diff/7008/remoting/host/host_...
File remoting/host/host_window.h (right):

https://chromiumcodereview.appspot.com/13212009/diff/7008/remoting/host/host_...
remoting/host/host_window.h:1: // Copyright (c) 2012 The Chromium Authors. All
rights reserved.
On 2013/04/02 19:04:22, sergeyu wrote:
> 2013

Done.

[Sergey Ulanov, 2013-04-04 00:10:03]

lgtm

https://codereview.chromium.org/13212009/diff/30001/remoting/host/disconnect_...
File remoting/host/disconnect_window_mac.mm (right):

https://codereview.chromium.org/13212009/diff/30001/remoting/host/disconnect_...
remoting/host/disconnect_window_mac.mm:48: window_controller_(NULL) {
s/NULL/nil/

https://codereview.chromium.org/13212009/diff/30001/remoting/host/disconnect_...
File remoting/host/disconnect_window_win.cc (right):

https://codereview.chromium.org/13212009/diff/30001/remoting/host/disconnect_...
remoting/host/disconnect_window_win.cc:279: client_session_control_.reset();
don't need reset here

https://codereview.chromium.org/13212009/diff/30001/remoting/host/host_user_i...
File remoting/host/host_user_interface.h (right):

https://codereview.chromium.org/13212009/diff/30001/remoting/host/host_user_i...
remoting/host/host_user_interface.h:27: class HostUserInterface : public
HostStatusObserver {
Once ContinueWindow is moved to HostWindow we will be able to remove this code,
right?

https://codereview.chromium.org/13212009/diff/30001/remoting/host/host_window.h
File remoting/host/host_window.h (right):

https://codereview.chromium.org/13212009/diff/30001/remoting/host/host_window...
remoting/host/host_window.h:29: static scoped_ptr<HostWindow>
CreateContinueWindow(
nit: Move static methods above Start().

https://codereview.chromium.org/13212009/diff/30001/remoting/host/remoting_me...
File remoting/host/remoting_me2me_host.cc (right):

https://codereview.chromium.org/13212009/diff/30001/remoting/host/remoting_me...
remoting/host/remoting_me2me_host.cc:280: scoped_ptr<HostUserInterface>
host_user_interface_;
I think you can remove it from here now. It doesn't do anything anyway. (still
need it for It2Me).

[alexeypa (please no reviews), 2013-04-04 16:56:40]

Jamie, do you want to take a look at Mac changes?

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/disc...
File remoting/host/disconnect_window_mac.mm (right):

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/disc...
remoting/host/disconnect_window_mac.mm:48: window_controller_(NULL) {
On 2013/04/04 00:10:03, sergeyu wrote:
> s/NULL/nil/

Done.

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/disc...
File remoting/host/disconnect_window_win.cc (right):

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/disc...
remoting/host/disconnect_window_win.cc:279: client_session_control_.reset();
On 2013/04/04 00:10:03, sergeyu wrote:
> don't need reset here

Done.

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/host...
File remoting/host/host_user_interface.h (right):

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/host...
remoting/host/host_user_interface.h:27: class HostUserInterface : public
HostStatusObserver {
On 2013/04/04 00:10:03, sergeyu wrote:
> Once ContinueWindow is moved to HostWindow we will be able to remove this
code,
> right?

Yes, it goes away with the next CL.

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/host...
File remoting/host/host_window.h (right):

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/host...
remoting/host/host_window.h:29: static scoped_ptr<HostWindow>
CreateContinueWindow(
On 2013/04/04 00:10:03, sergeyu wrote:
> nit: Move static methods above Start().

Done.

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/remo...
File remoting/host/remoting_me2me_host.cc (right):

https://chromiumcodereview.appspot.com/13212009/diff/30001/remoting/host/remo...
remoting/host/remoting_me2me_host.cc:280: scoped_ptr<HostUserInterface>
host_user_interface_;
On 2013/04/04 00:10:03, sergeyu wrote:
> I think you can remove it from here now. It doesn't do anything anyway. (still
> need it for It2Me).

Done.

[Jamie, 2013-04-04 17:00:52]

Mac lgtm as far as basic Obj-C is concerned. I haven't looked at it any more
deeply than that, but it looks like Sergey has.

[commit-bot: I haz the power, 2013-04-04 17:04:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/13212009/38001

[commit-bot: I haz the power, 2013-04-04 17:10:39]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-04-04 18:06:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/13212009/38001

[commit-bot: I haz the power, 2013-04-04 18:07:41]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-04-04 19:29:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/13212009/38001

[commit-bot: I haz the power, 2013-04-04 19:33:22]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-04-04 22:44:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/13212009/38001

[commit-bot: I haz the power, 2013-04-04 23:11:56]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-04-04 23:23:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/13212009/66001

[commit-bot: I haz the power, 2013-04-05 03:32:08]

Change committed as 192473