Fixed continue window bugs and added 60s auto-shutdown timeout.

remoting/host/desktop_environment.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/desktop_environment.h"
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "remoting/host/capturer.h"
 #include "remoting/host/chromoting_host.h"
 #include "remoting/host/chromoting_host_context.h"
 #include "remoting/host/continue_window.h"
 #include "remoting/host/curtain.h"
 #include "remoting/host/disconnect_window.h"
 #include "remoting/host/event_executor.h"
 #include "remoting/host/local_input_monitor.h"
 
-static const int kContinueWindowTimeoutMs = 10 * 60 * 1000;
+// Milliseconds before the continue window is shown.
+static const int kContinueWindowShowTimeoutMs = 10  * 1000;

[Sergey Ulanov, 2011/08/24 01:56:45]

Revert this change

[Jamie, 2011/08/24 17:42:03]

Done.

+
+// Milliseconds before the continue window is automatically dismissed and
+// the connection is closed.
+static const int kContinueWindowHideTimeoutMs = 5 * 1000;

[Sergey Ulanov, 2011/08/24 01:56:45]

Should be 60 * 1000 instead of 5 seconds

[Jamie, 2011/08/24 17:42:03]

Done.

 
 namespace remoting {
 
 // UIThreadProxy proxies DesktopEnvironment method calls to the UI
 // thread. This is neccessary so that DesktopEnvironment can be
 // deleted synchronously even while there are pending tasks on the
 // message queue.
 //
 // TODO(sergeyu): Merge this code with remoting::TaskThreadProxy. The
 // problem solved by this class is very simular to the one solved by
 // ScopedRunnableMethodFactory. The main difference is that this class
 // is thread-safe. Change the interface to make it look more like
 // ScopedRunnableMethodFactory and rename it to avoid confusion with
 // MessageLoopProxy.
 class UIThreadProxy : public base::RefCountedThreadSafe<UIThreadProxy> {
  public:
-  UIThreadProxy(base::MessageLoopProxy* message_loop)
+  explicit UIThreadProxy(base::MessageLoopProxy* message_loop)
       : message_loop_(message_loop) {
   }
 
   // TODO(sergeyu): Rename this method.
   void Detach() {
     DCHECK(message_loop_->BelongsToCurrentThread());
     message_loop_ = NULL;
   }
 
   void CallOnUIThread(const tracked_objects::Location& from_here,
@@ skipping 66 matching lines @@
                                        LocalInputMonitor* local_input_monitor)
     : host_(NULL),
       context_(context),
       capturer_(capturer),
       event_executor_(event_executor),
       curtain_(curtain),
       disconnect_window_(disconnect_window),
       continue_window_(continue_window),
       local_input_monitor_(local_input_monitor),
       is_monitoring_local_inputs_(false),
-      continue_timer_started_(false),
+      continue_timer_state_(INACTIVE),
       proxy_(new UIThreadProxy(context->ui_message_loop())) {
 }
 
 DesktopEnvironment::~DesktopEnvironment() {
 }
 
 void DesktopEnvironment::Shutdown() {
   DCHECK(context_->ui_message_loop()->BelongsToCurrentThread());
 
   MonitorLocalInputs(false);
@@ skipping 30 matching lines @@
 void DesktopEnvironment::ProcessOnLastDisconnect() {
   DCHECK(context_->ui_message_loop()->BelongsToCurrentThread());
 
   MonitorLocalInputs(false);
   ShowDisconnectWindow(false, std::string());
   ShowContinueWindow(false);
   StartContinueWindowTimer(false);
 }
 
 void DesktopEnvironment::ProcessOnPause(bool pause) {
-  StartContinueWindowTimer(!pause);
+  if (!pause) {
+    continue_timer_state_ = INACTIVE;
+    StartContinueWindowTimer(true);
+  }
 }
 
 void DesktopEnvironment::MonitorLocalInputs(bool enable) {
   DCHECK(context_->ui_message_loop()->BelongsToCurrentThread());
 
   if (enable == is_monitoring_local_inputs_)
     return;
   if (enable) {
     local_input_monitor_->Start(host_);
   } else {
@@ skipping 19 matching lines @@
   if (show) {
     continue_window_->Show(host_);
   } else {
     continue_window_->Hide();
   }
 }
 
 void DesktopEnvironment::StartContinueWindowTimer(bool start) {
   DCHECK(context_->ui_message_loop()->BelongsToCurrentThread());
 
-  if (start && !continue_timer_started_) {
+  if (start && continue_timer_state_ == INACTIVE) {
     continue_timer_target_time_ = base::Time::Now() +
-        base::TimeDelta::FromMilliseconds(kContinueWindowTimeoutMs);
+        base::TimeDelta::FromMilliseconds(kContinueWindowShowTimeoutMs);
     proxy_->CallOnUIThreadDelayed(
         FROM_HERE, base::Bind(&DesktopEnvironment::ContinueWindowTimerFunc,
                               base::Unretained(this)),
-        kContinueWindowTimeoutMs);
+        kContinueWindowShowTimeoutMs);
   }
 
-  continue_timer_started_ = start;
+  continue_timer_state_ = start ? SHOW_DIALOG : INACTIVE;

[Sergey Ulanov, 2011/08/24 01:56:45]

What if this function is called with when continue_timer_state == HIDE_DIALOG?
Should we really reset it to SHOW_DIALOG?

I think it's better to add continue_timer_start_ = SHOW_DIALOG in the if block
above and add else block to handle other cases

[Jamie, 2011/08/24 17:42:03]

Done.

 }
 
 void DesktopEnvironment::ContinueWindowTimerFunc() {
   DCHECK(context_->ui_message_loop()->BelongsToCurrentThread());
 
   // This function may be called prematurely if timer was stopped and
   // then started again. In that case we just ignore this call.
-  if (continue_timer_target_time_ > base::Time::Now())
+  if (continue_timer_state_ == INACTIVE ||
+      continue_timer_target_time_ > base::Time::Now())
     return;
 
-  continue_timer_started_ = false;
-  host_->PauseSession(true);
-  ShowContinueWindow(true);
+  if (continue_timer_state_ == SHOW_DIALOG) {
+    host_->PauseSession(true);
+    ShowContinueWindow(true);
+    continue_timer_target_time_ = base::Time::Now() +
+        base::TimeDelta::FromMilliseconds(kContinueWindowHideTimeoutMs);
+    proxy_->CallOnUIThreadDelayed(
+        FROM_HERE, base::Bind(&DesktopEnvironment::ContinueWindowTimerFunc,
+                              base::Unretained(this)),
+        kContinueWindowHideTimeoutMs);
+    continue_timer_state_ = HIDE_DIALOG;
+  } else {
+    continue_timer_state_ = INACTIVE;
+    ShowContinueWindow(false);
+    host_->Shutdown(NULL);

[Sergey Ulanov, 2011/08/24 01:56:45]

I don't like the idea of calling Shutdown() from here directly, but it's Ok to
have it here for now since UI code calls Shutdown() anyway.

+  }
 }
 
 
 }  // namespace remoting