Chromoting: Move host input and window mgmt into DesktopEnvironment

Chromoting: Move host input and window mgmt into DesktopEnvironment

Hide details of window management and continue timers in the DesktopEnvironment
class and expose OnConnect, OnLastDisconnect and OnPause methods.

Move all host input monitor code into DesktopEnvironment as well.

BUG=none
TEST=none

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

[Jamie, 2011-07-07 00:26:48]

LGTM, assuming there's a good reason for the ref-counting.

http://codereview.chromium.org/7312019/diff/1/remoting/host/desktop_environme...
File remoting/host/desktop_environment.h (right):

http://codereview.chromium.org/7312019/diff/1/remoting/host/desktop_environme...
remoting/host/desktop_environment.h:27: : public
base::RefCountedThreadSafe<DesktopEnvironment> {
Why does this now need to be refcounted?

[Sergey Ulanov, 2011-07-07 00:33:17]

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

http://codereview.chromium.org/7312019/diff/1/remoting/host/chromoting_host.h...
remoting/host/chromoting_host.h:189: scoped_ptr<DesktopEnvironment>
desktop_environment_;
scoped_refptr?

http://codereview.chromium.org/7312019/diff/1/remoting/host/desktop_environme...
File remoting/host/desktop_environment.h (right):

http://codereview.chromium.org/7312019/diff/1/remoting/host/desktop_environme...
remoting/host/desktop_environment.h:27: : public
base::RefCountedThreadSafe<DesktopEnvironment> {
It's better to avoid making this object refcounted.
DesktopEnvironment works only on UI thread so you can just limit usage of this
object to UI thread, and let ChromotingHost care about calling
DesktopEnvironment on the UI thread. If it doesn't need to run on multiple
threads then it doesn't need to be ref-counted.
It's hard to cleanly shutdown ref-counted objects that work on more than one
thread.

[garykac, 2011-07-07 20:38:09]

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

http://codereview.chromium.org/7312019/diff/1/remoting/host/chromoting_host.h...
remoting/host/chromoting_host.h:189: scoped_ptr<DesktopEnvironment>
desktop_environment_;
On 2011/07/07 00:33:17, sergeyu wrote:
> scoped_refptr?

not needed (I removed refcounting).

http://codereview.chromium.org/7312019/diff/1/remoting/host/desktop_environme...
File remoting/host/desktop_environment.h (right):

http://codereview.chromium.org/7312019/diff/1/remoting/host/desktop_environme...
remoting/host/desktop_environment.h:27: : public
base::RefCountedThreadSafe<DesktopEnvironment> {
On 2011/07/07 00:33:17, sergeyu wrote:
> It's better to avoid making this object refcounted.
> DesktopEnvironment works only on UI thread so you can just limit usage of this
> object to UI thread, and let ChromotingHost care about calling
> DesktopEnvironment on the UI thread. If it doesn't need to run on multiple
> threads then it doesn't need to be ref-counted.
> It's hard to cleanly shutdown ref-counted objects that work on more than one
> thread.

I've disabled refcounting.

[Sergey Ulanov, 2011-07-08 23:12:24]

http://codereview.chromium.org/7312019/diff/7002/remoting/host/desktop_enviro...
File remoting/host/desktop_environment.cc (right):

http://codereview.chromium.org/7312019/diff/7002/remoting/host/desktop_enviro...
remoting/host/desktop_environment.cc:86: if (!context_->IsUIThread()) {
I was suggesting that we change ChromotingHost to make sure that it calls this
code only from UI thread. This code would just DCHECK() if it is called from the
wrong thread?

http://codereview.chromium.org/7312019/diff/7002/remoting/host/desktop_enviro...
File remoting/host/desktop_environment.h (right):

http://codereview.chromium.org/7312019/diff/7002/remoting/host/desktop_enviro...
remoting/host/desktop_environment.h:108:
DISABLE_RUNNABLE_METHOD_REFCOUNT(remoting::DesktopEnvironment);
Oh, no! DISABLE_RUNNABLE_METHOD_REFCOUNT is evil! Can you guarantee that there
are no pending tasks for the object when it is being destroyed?