Ensure the Chromoting Host process eventually terminates when shut down.

remoting/host/remoting_me2me_host.cc

 // Copyright (c) 2012 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.
 //
 // This file implements a standalone host process for Me2Me.
 
 #include <string>
 
 #include "base/at_exit.h"
 #include "base/bind.h"
@@ skipping 43 matching lines @@
 #include "remoting/host/host_main.h"
 #include "remoting/host/host_status_logger.h"
 #include "remoting/host/ipc_constants.h"
 #include "remoting/host/ipc_desktop_environment.h"
 #include "remoting/host/ipc_host_event_logger.h"
 #include "remoting/host/logging.h"
 #include "remoting/host/me2me_desktop_environment.h"
 #include "remoting/host/pairing_registry_delegate.h"
 #include "remoting/host/policy_hack/policy_watcher.h"
 #include "remoting/host/session_manager_factory.h"
+#include "remoting/host/shutdown_watchdog.h"
 #include "remoting/host/signaling_connector.h"
 #include "remoting/host/single_window_desktop_environment.h"
 #include "remoting/host/token_validator_factory_impl.h"
 #include "remoting/host/usage_stats_consent.h"
 #include "remoting/host/username.h"
 #include "remoting/host/video_frame_recorder_host_extension.h"
 #include "remoting/protocol/me2me_host_authenticator_factory.h"
 #include "remoting/protocol/network_settings.h"
 #include "remoting/protocol/pairing_registry.h"
 #include "remoting/protocol/token_validator.h"
@@ skipping 235 matching lines @@
   bool allow_pairing_;
 
   bool curtain_required_;
   ThirdPartyAuthConfig third_party_auth_config_;
   bool enable_gnubby_auth_;
 
   // Boolean to change flow, where ncessary, if we're
   // capturing a window instead of the entire desktop.
   bool enable_window_capture_;
 
+  // Destroying the watchdog will disarm it, so it should be destroyed last.

[Jamie, 2014/12/10 03:02:23]

It should be the first member variable in that case.

[Łukasz Anforowicz, 2014/12/10 17:15:31]

It is possible that HostProcess is destroyed, but the process is not -
similarily to how MinimuHeartbeatSender extends the lifetime of
network_task_runner and ui_task_runner, something else could also extend these
lifetimes.  In this case the ShutdownWatchdog would be destroyed together with
HostProcess and would not enforce process shutdown, right?

+  ShutdownWatchdog shutdown_watchdog_;
+
   // Used to specify which window to stream, if enabled.
   webrtc::WindowId window_id_;
 
   scoped_ptr<OAuthTokenGetter> oauth_token_getter_;
   scoped_ptr<XmppSignalStrategy> signal_strategy_;
   scoped_ptr<SignalingConnector> signaling_connector_;
   scoped_ptr<HeartbeatSender> heartbeat_sender_;
   scoped_ptr<HostChangeNotificationListener> host_change_notification_listener_;
   scoped_ptr<HostStatusLogger> host_status_logger_;
   scoped_ptr<HostEventLogger> host_event_logger_;
@@ skipping 22 matching lines @@
       frame_recorder_buffer_size_(0),
       host_username_match_required_(false),
       allow_nat_traversal_(true),
       allow_relay_(true),
       min_udp_port_(0),
       max_udp_port_(0),
       allow_pairing_(true),
       curtain_required_(false),
       enable_gnubby_auth_(false),
       enable_window_capture_(false),
+      shutdown_watchdog_(base::TimeDelta::FromSeconds(15)),

[Łukasz Anforowicz, 2014/12/10 17:15:31]

Looks good.  I wonder if we should pull all the process shutdown timeouts closer
together into either constants or macro definitions.  I guess the same comment
applies to my changelist :->

       window_id_(0),
 #if defined(REMOTING_MULTI_PROCESS)
       desktop_session_connector_(NULL),
 #endif  // defined(REMOTING_MULTI_PROCESS)
       self_(this),
       exit_code_out_(exit_code_out),
       signal_parent_(false) {
   StartOnUiThread();
 }
 
@@ skipping 1038 matching lines @@
   signaling_connector_.reset();
   oauth_token_getter_.reset();
   signal_strategy_.reset();
   network_change_notifier_.reset();
 
   if (state_ == HOST_STOPPING_TO_RESTART) {
     StartHost();
   } else if (state_ == HOST_STOPPING) {
     state_ = HOST_STOPPED;
 
+    shutdown_watchdog_.SetExitCode(*exit_code_out_);
+    shutdown_watchdog_.Arm();
+
     if (policy_watcher_.get()) {
       policy_watcher_->StopWatching(
           base::Bind(&HostProcess::OnPolicyWatcherShutdown, this));
     } else {
       OnPolicyWatcherShutdown();
     }
   } else {
     // This method is only called in STOPPING_TO_RESTART and STOPPING states.
     NOTREACHED();
   }
@@ skipping 52 matching lines @@
   int exit_code = kSuccessExitCode;
   new HostProcess(context.Pass(), &exit_code);
 
   // Run the main (also UI) message loop until the host no longer needs it.
   message_loop.Run();
 
   return exit_code;
 }
 
 }  // namespace remoting