Crash the network process when a fatal daemon-to-network protocol error encountered.

Advantages of this approach:
- We will be getting crash reports from the field when fatal mistakes happen.
- The exponential backoff counter will not be reset.

BUG=134694


Review URL: https://chromiumcodereview.appspot.com/11234034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163702 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 63 insertions, 42 deletions

diff --git a/remoting/host/chromoting_messages.h b/remoting/host/chromoting_messages.h
index ad85120..17fdcec 100644
--- a/remoting/host/chromoting_messages.h
+++ b/remoting/host/chromoting_messages.h
@@ -12,6 +12,15 @@
 //-----------------------------------------------------------------------------
 // Chromoting messages sent from the daemon to the network process.
 
+// Requests the network process to crash producing a crash dump. The daemon
+// sends this message when a fatal error has been detected indicating that
+// the network process misbehaves. The daemon passes the location of the code
+// that detected the error.
+IPC_MESSAGE_CONTROL3(ChromotingDaemonNetworkMsg_Crash,
+                     std::string /* function_name */,
+                     std::string /* file_name */,
+                     int /* line_number */)
+
 // Delivers the host configuration (and updates) to the network process.
 IPC_MESSAGE_CONTROL1(ChromotingDaemonNetworkMsg_Configuration, std::string)
 
diff --git a/remoting/host/daemon_process.cc b/remoting/host/daemon_process.cc
index 2e69334..54236f9 100644
--- a/remoting/host/daemon_process.cc
+++ b/remoting/host/daemon_process.cc
@@ -78,7 +78,7 @@ void DaemonProcess::CloseDesktopSession(int terminal_id) {
   // a protocol error and the network process will be restarted.
   if (!IsTerminalIdKnown(terminal_id)) {
     LOG(ERROR) << "An invalid terminal ID. terminal_id=" << terminal_id;
-    RestartNetworkProcess();
+    CrashNetworkProcess(FROM_HERE);
     DeleteAllDesktopSessions();
     return;
   }
@@ -115,27 +115,6 @@ DaemonProcess::DaemonProcess(
   DCHECK(caller_task_runner->BelongsToCurrentThread());
 }
 
-void DaemonProcess::Initialize() {
-  DCHECK(caller_task_runner()->BelongsToCurrentThread());
-
-  // Get the name of the host configuration file.
-  FilePath default_config_dir = remoting::GetConfigDir();
-  FilePath config_path = default_config_dir.Append(kDefaultHostConfigFile);
-  const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(kHostConfigSwitchName)) {
-    config_path = command_line->GetSwitchValuePath(kHostConfigSwitchName);
-  }
-
-  // Start watching the host configuration file.
-  config_watcher_.reset(new ConfigFileWatcher(caller_task_runner(),
-                                              io_task_runner(),
-                                              this));
-  config_watcher_->Watch(config_path);
-
-  // Launch the process.
-  LaunchNetworkProcess();
-}
-
 bool DaemonProcess::IsTerminalIdKnown(int terminal_id) {
   return terminal_id < next_terminal_id_;
 }
@@ -148,7 +127,7 @@ void DaemonProcess::CreateDesktopSession(int terminal_id) {
   // a protocol error and the network process will be restarted.
   if (IsTerminalIdKnown(terminal_id)) {
     LOG(ERROR) << "An invalid terminal ID. terminal_id=" << terminal_id;
-    RestartNetworkProcess();
+    CrashNetworkProcess(FROM_HERE);
     DeleteAllDesktopSessions();
     return;
   }
@@ -159,6 +138,33 @@ void DaemonProcess::CreateDesktopSession(int terminal_id) {
   next_terminal_id_ = std::max(next_terminal_id_, terminal_id + 1);
 }
 
+void DaemonProcess::CrashNetworkProcess(
+    const tracked_objects::Location& location) {
+  SendToNetwork(new ChromotingDaemonNetworkMsg_Crash(
+      location.function_name(), location.file_name(), location.line_number()));
+}
+
+void DaemonProcess::Initialize() {
+  DCHECK(caller_task_runner()->BelongsToCurrentThread());
+
+  // Get the name of the host configuration file.
+  FilePath default_config_dir = remoting::GetConfigDir();
+  FilePath config_path = default_config_dir.Append(kDefaultHostConfigFile);
+  const CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(kHostConfigSwitchName)) {
+    config_path = command_line->GetSwitchValuePath(kHostConfigSwitchName);
+  }
+
+  // Start watching the host configuration file.
+  config_watcher_.reset(new ConfigFileWatcher(caller_task_runner(),
+                                              io_task_runner(),
+                                              this));
+  config_watcher_->Watch(config_path);
+
+  // Launch the process.
+  LaunchNetworkProcess();
+}
+
 void DaemonProcess::DoStop() {
   DCHECK(caller_task_runner()->BelongsToCurrentThread());
 
diff --git a/remoting/host/daemon_process.h b/remoting/host/daemon_process.h
index 8d973f5..897d880 100644
--- a/remoting/host/daemon_process.h
+++ b/remoting/host/daemon_process.h
@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "ipc/ipc_channel.h"
@@ -69,12 +70,15 @@ class DaemonProcess
                 scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
                 const base::Closure& stopped_callback);
 
-  // Reads the host configuration and launches the network process.
-  void Initialize();
-
   // Creates a desktop session and assigns a unique ID to it.
   void CreateDesktopSession(int terminal_id);
 
+  // Requests the network process to crash.
+  void CrashNetworkProcess(const tracked_objects::Location& location);
+
+  // Reads the host configuration and launches the network process.
+  void Initialize();
+
   // Returns true if |terminal_id| is considered to be known. I.e. it is
   // less or equal to the highest ID we have seen so far.
   bool IsTerminalIdKnown(int terminal_id);
@@ -89,9 +93,6 @@ class DaemonProcess
   // Launches the network process and establishes an IPC channel with it.
   virtual void LaunchNetworkProcess() = 0;
 
-  // Restart the network process.
-  virtual void RestartNetworkProcess() = 0;
-
   scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner() {
     return caller_task_runner_;
   }
diff --git a/remoting/host/daemon_process_unittest.cc b/remoting/host/daemon_process_unittest.cc
index 5ca1285..5a8d045 100644
--- a/remoting/host/daemon_process_unittest.cc
+++ b/remoting/host/daemon_process_unittest.cc
@@ -25,6 +25,7 @@ namespace remoting {
 namespace {
 
 enum Messages {
+  kMessageCrash = ChromotingDaemonNetworkMsg_Crash::ID,
   kMessageConfiguration = ChromotingDaemonNetworkMsg_Configuration::ID,
   kMessageConnectTerminal = ChromotingNetworkHostMsg_ConnectTerminal::ID,
   kMessageDisconnectTerminal = ChromotingNetworkHostMsg_DisconnectTerminal::ID,
@@ -61,7 +62,6 @@ class MockDaemonProcess : public DaemonProcess {
 
   MOCK_METHOD1(DoCreateDesktopSessionPtr, DesktopSession*(int));
   MOCK_METHOD0(LaunchNetworkProcess, void());
-  MOCK_METHOD0(RestartNetworkProcess, void());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockDaemonProcess);
@@ -270,8 +270,9 @@ TEST_F(DaemonProcessTest, InvalidDisconnectTerminal) {
   InSequence s;
   EXPECT_CALL(*daemon_process_, Sent(Message(kMessageConfiguration)));
   EXPECT_CALL(*daemon_process_, Received(Message(kMessageDisconnectTerminal)));
-  EXPECT_CALL(*daemon_process_, RestartNetworkProcess())
-      .WillRepeatedly(Invoke(this, &DaemonProcessTest::LaunchNetworkProcess));
+  EXPECT_CALL(*daemon_process_, Sent(Message(kMessageCrash)))
+      .WillOnce(InvokeWithoutArgs(this,
+                                  &DaemonProcessTest::LaunchNetworkProcess));
   EXPECT_CALL(*daemon_process_, Sent(Message(kMessageConfiguration)));
 
   StartDaemonProcess();
@@ -291,8 +292,9 @@ TEST_F(DaemonProcessTest, InvalidConnectTerminal) {
   EXPECT_CALL(*daemon_process_, Sent(Message(kMessageConfiguration)));
   EXPECT_CALL(*daemon_process_, Received(Message(kMessageConnectTerminal)));
   EXPECT_CALL(*daemon_process_, Received(Message(kMessageConnectTerminal)));
-  EXPECT_CALL(*daemon_process_, RestartNetworkProcess())
-      .WillRepeatedly(Invoke(this, &DaemonProcessTest::LaunchNetworkProcess));
+  EXPECT_CALL(*daemon_process_, Sent(Message(kMessageCrash)))
+      .WillOnce(InvokeWithoutArgs(this,
+                                  &DaemonProcessTest::LaunchNetworkProcess));
   EXPECT_CALL(*daemon_process_, Sent(Message(kMessageConfiguration)));
 
   StartDaemonProcess();
diff --git a/remoting/host/daemon_process_win.cc b/remoting/host/daemon_process_win.cc
index cb81b4a..5b0a57d 100644
--- a/remoting/host/daemon_process_win.cc
+++ b/remoting/host/daemon_process_win.cc
@@ -53,7 +53,6 @@ class DaemonProcessWin : public DaemonProcess {
   virtual scoped_ptr<DesktopSession> DoCreateDesktopSession(
       int terminal_id) OVERRIDE;
   virtual void LaunchNetworkProcess() OVERRIDE;
-  virtual void RestartNetworkProcess() OVERRIDE;
 
  private:
   scoped_ptr<WorkerProcessLauncher> network_launcher_;
@@ -117,13 +116,6 @@ void DaemonProcessWin::LaunchNetworkProcess() {
       caller_task_runner(), delegate.Pass(), this));
 }
 
-void DaemonProcessWin::RestartNetworkProcess() {
-  DCHECK(caller_task_runner()->BelongsToCurrentThread());
-
-  network_launcher_.reset();
-  LaunchNetworkProcess();
-}
-
 scoped_ptr<DaemonProcess> DaemonProcess::Create(
     scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 09ee9dc..420465c 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -170,6 +170,15 @@ class HostProcess
     return true;
   }
 
+  // Crashes the process in response to a daemon's request. The daemon passes
+  // the location of the code that detected the fatal error resulted in this
+  // request.
+  void OnCrash(const std::string& function_name,
+               const std::string& file_name,
+               const int& line_number) {
+    CHECK(false);
+  }
+
 #else  // !defined(REMOTING_MULTI_PROCESS)
 
   bool InitWithCommandLine(const CommandLine* cmd_line) {
@@ -296,6 +305,8 @@ class HostProcess
 #if defined(REMOTING_MULTI_PROCESS)
     bool handled = true;
     IPC_BEGIN_MESSAGE_MAP(HostProcess, message)
+      IPC_MESSAGE_HANDLER(ChromotingDaemonNetworkMsg_Crash,
+                          OnCrash)
       IPC_MESSAGE_HANDLER(ChromotingDaemonNetworkMsg_Configuration,
                           OnConfigUpdated)
       IPC_MESSAGE_FORWARD(ChromotingDaemonNetworkMsg_TerminalDisconnected,