Refactor ContinueWindow::Show() to accept a callback parameter.

Refactor ContinueWindow::Show() to accept a callback parameter.

This gets rid of the DesktopEnvironment::OnPause() notification from the Host.
Instead, that code is moved to DesktopEnvironment::ContinueSession() and
provided as a callback function parameter to ContinueWindow::Show().

This will simplify a future CL to move the ContinueWindow/DisconnectWindow code
into an IT2Me-specific HostObserver subclass.


BUG=None
TEST=No change in functionality.


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

[Wez, 2011-11-23 18:26:02]

LGTM

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

http://codereview.chromium.org/8624009/diff/3001/remoting/host/desktop_enviro...
remoting/host/desktop_environment.cc:159: &DesktopEnvironment::ContinueSession,
base::Unretained(this)));
The intent is for ShowContinueWindow() and ContinueSession() to be moved out of
DesktopEnvironment in a follow-up CL, right?

http://codereview.chromium.org/8624009/diff/3001/remoting/host/desktop_enviro...
remoting/host/desktop_environment.cc:173: host_->Shutdown(base::Closure());
Shouldn't this be a call to OnShutdownHostTimer(), for consistency?

[Sergey Ulanov, 2011-11-23 19:23:15]

http://codereview.chromium.org/8624009/diff/3001/remoting/host/continue_window.h
File remoting/host/continue_window.h (right):

http://codereview.chromium.org/8624009/diff/3001/remoting/host/continue_windo...
remoting/host/continue_window.h:26: virtual void Show(ChromotingHost* host,
I think the point of this CL is to remove dependency on ChromotingHost from
ContinueWindow. So do we still need this parameter?

[Lambros, 2011-11-23 19:39:27]

http://codereview.chromium.org/8624009/diff/3001/remoting/host/continue_window.h
File remoting/host/continue_window.h (right):

http://codereview.chromium.org/8624009/diff/3001/remoting/host/continue_windo...
remoting/host/continue_window.h:26: virtual void Show(ChromotingHost* host,
On 2011/11/23 19:23:15, sergeyu wrote:
> I think the point of this CL is to remove dependency on ChromotingHost from
> ContinueWindow. So do we still need this parameter?
My main purpose was to get rid of the unnecessary OnPause() notification, which
was slightly hindering the IT2Me HostObserver refactoring.  Removing this
dependency is a nice side-effect :)  The |host| parameter could be removed and
replaced with a UiStrings parameter.  I'll do this cleanup in a followup CL.

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

http://codereview.chromium.org/8624009/diff/3001/remoting/host/desktop_enviro...
remoting/host/desktop_environment.cc:159: &DesktopEnvironment::ContinueSession,
base::Unretained(this)));
On 2011/11/23 18:26:02, Wez wrote:
> The intent is for ShowContinueWindow() and ContinueSession() to be moved out
of
> DesktopEnvironment in a follow-up CL, right?
Yep.

http://codereview.chromium.org/8624009/diff/3001/remoting/host/desktop_enviro...
remoting/host/desktop_environment.cc:173: host_->Shutdown(base::Closure());
On 2011/11/23 18:26:02, Wez wrote:
> Shouldn't this be a call to OnShutdownHostTimer(), for consistency?
It could be.  I simply copied the implementation straight out of the
ContinueWindowXxx classes.  I'll do a followup CL for this, and any other
cleanups I notice after the refactoring.

[commit-bot: I haz the power, 2011-11-23 21:36:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lambroslambrou@chromium.org/8624009/3001

[commit-bot: I haz the power, 2011-11-23 23:39:51]

Change committed as 111443