Fix CrashGenerationServer to recover from protocol errors and a test for same.

R=siggi at http://breakpad.appspot.com/196001/show


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@695 4c0a9323-5329-0410-9bdc-e9ce6186880e

6 files changed, 442 insertions, 77 deletions

diff --git a/src/client/windows/breakpad_client.gyp b/src/client/windows/breakpad_client.gyp
index 08267dca..9fbb88c3 100755
--- a/src/client/windows/breakpad_client.gyp
+++ b/src/client/windows/breakpad_client.gyp
@@ -40,7 +40,7 @@
         './handler/exception_handler.gyp:*',
         './sender/crash_report_sender.gyp:*',
         './unittests/client_tests.gyp:*',
-        './unittests/gtest.gyp:*',
+        './unittests/testing.gyp:*',
       ]
     },
     {
diff --git a/src/client/windows/crash_generation/crash_generation_server.cc b/src/client/windows/crash_generation/crash_generation_server.cc
index dafa5c2a..61af1b2d 100644
--- a/src/client/windows/crash_generation/crash_generation_server.cc
+++ b/src/client/windows/crash_generation/crash_generation_server.cc
@@ -34,6 +34,8 @@
 #include "client/windows/common/auto_critical_section.h"
 #include "processor/scoped_ptr.h"
 
+#include "client/windows/crash_generation/client_info.h"
+
 namespace google_breakpad {
 
 // Output buffer size.
@@ -113,7 +115,7 @@ CrashGenerationServer::CrashGenerationServer(
       exit_context_(exit_context),
       generate_dumps_(generate_dumps),
       dump_generator_(NULL),
-      server_state_(IPC_SERVER_STATE_INITIAL),
+      server_state_(IPC_SERVER_STATE_UNINITIALIZED),
       shutting_down_(false),
       overlapped_(),
       client_info_(NULL),
@@ -197,10 +199,18 @@ CrashGenerationServer::~CrashGenerationServer() {
     CloseHandle(server_alive_handle_);
   }
 
+  if (overlapped_.hEvent) {
+    CloseHandle(overlapped_.hEvent);
+  }
+
   DeleteCriticalSection(&clients_sync_);
 }
 
 bool CrashGenerationServer::Start() {
+  if (server_state_ != IPC_SERVER_STATE_UNINITIALIZED) {
+    return false;
+  }
+
   server_state_ = IPC_SERVER_STATE_INITIAL;
 
   server_alive_handle_ = CreateMutex(NULL, TRUE, NULL);
@@ -239,9 +249,12 @@ bool CrashGenerationServer::Start() {
     return false;
   }
 
-  // Signal the event to start a separate thread to handle
-  // client connections.
-  return SetEvent(overlapped_.hEvent) != FALSE;
+  // Kick-start the state machine. This will initiate an asynchronous wait
+  // for client connections.
+  HandleInitialState();
+
+  // If we are in error state, it's because we failed to start listening.
+  return server_state_ != IPC_SERVER_STATE_ERROR;
 }
 
 // If the server thread serving clients ever gets into the
@@ -283,33 +296,29 @@ void CrashGenerationServer::HandleInitialState() {
   assert(server_state_ == IPC_SERVER_STATE_INITIAL);
 
   if (!ResetEvent(overlapped_.hEvent)) {
-    server_state_ = IPC_SERVER_STATE_ERROR;
+    EnterErrorState();
     return;
   }
 
   bool success = ConnectNamedPipe(pipe_, &overlapped_) != FALSE;
+  DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
 
   // From MSDN, it is not clear that when ConnectNamedPipe is used
   // in an overlapped mode, will it ever return non-zero value, and
   // if so, in what cases.
   assert(!success);
 
-  DWORD error_code = GetLastError();
   switch (error_code) {
     case ERROR_IO_PENDING:
-      server_state_ = IPC_SERVER_STATE_CONNECTING;
+      EnterStateWhenSignaled(IPC_SERVER_STATE_CONNECTING);
       break;
 
     case ERROR_PIPE_CONNECTED:
-      if (SetEvent(overlapped_.hEvent)) {
-        server_state_ = IPC_SERVER_STATE_CONNECTED;
-      } else {
-        server_state_ = IPC_SERVER_STATE_ERROR;
-      }
+      EnterStateImmediately(IPC_SERVER_STATE_CONNECTED);
       break;
 
     default:
-      server_state_ = IPC_SERVER_STATE_ERROR;
+      EnterErrorState();
       break;
   }
 }
@@ -328,14 +337,14 @@ void CrashGenerationServer::HandleConnectingState() {
                                      &overlapped_,
                                      &bytes_count,
                                      FALSE) != FALSE;
+  DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
 
   if (success) {
-    server_state_ = IPC_SERVER_STATE_CONNECTED;
-    return;
-  }
-
-  if (GetLastError() != ERROR_IO_INCOMPLETE) {
-    server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+    EnterStateImmediately(IPC_SERVER_STATE_CONNECTED);
+  } else if (error_code != ERROR_IO_INCOMPLETE) {
+    EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
+  } else {
+    // remain in CONNECTING state
   }
 }
 
@@ -353,16 +362,17 @@ void CrashGenerationServer::HandleConnectedState() {
                           sizeof(msg_),
                           &bytes_count,
                           &overlapped_) != FALSE;
+  DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
 
   // Note that the asynchronous read issued above can finish before the
   // code below executes. But, it is okay to change state after issuing
   // the asynchronous read. This is because even if the asynchronous read
   // is done, the callback for it would not be executed until the current
   // thread finishes its execution.
-  if (success || GetLastError() == ERROR_IO_PENDING) {
-    server_state_ = IPC_SERVER_STATE_READING;
+  if (success || error_code == ERROR_IO_PENDING) {
+    EnterStateWhenSignaled(IPC_SERVER_STATE_READING);
   } else {
-    server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+    EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
   }
 }
 
@@ -378,21 +388,18 @@ void CrashGenerationServer::HandleReadingState() {
                                      &overlapped_,
                                      &bytes_count,
                                      FALSE) != FALSE;
+  DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
 
   if (success && bytes_count == sizeof(ProtocolMessage)) {
-    server_state_ = IPC_SERVER_STATE_READ_DONE;
-    return;
-  }
-
-  DWORD error_code;
-  error_code = GetLastError();
-
-  // We should never get an I/O incomplete since we should not execute this
-  // unless the Read has finished and the overlapped event is signaled. If
-  // we do get INCOMPLETE, we have a bug in our code.
-  assert(error_code != ERROR_IO_INCOMPLETE);
+    EnterStateImmediately(IPC_SERVER_STATE_READ_DONE);
+  } else {
+    // We should never get an I/O incomplete since we should not execute this
+    // unless the Read has finished and the overlapped event is signaled. If
+    // we do get INCOMPLETE, we have a bug in our code.
+    assert(error_code != ERROR_IO_INCOMPLETE);
 
-  server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+    EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
+  }
 }
 
 // When the server thread serving the client is in the READ_DONE state,
@@ -405,7 +412,7 @@ void CrashGenerationServer::HandleReadDoneState() {
   assert(server_state_ == IPC_SERVER_STATE_READ_DONE);
 
   if (!IsClientRequestValid(msg_)) {
-    server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+    EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
     return;
   }
 
@@ -419,22 +426,26 @@ void CrashGenerationServer::HandleReadDoneState() {
                      msg_.custom_client_info));
 
   if (!client_info->Initialize()) {
-    server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+    EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
     return;
   }
 
+  // Issues an asynchronous WriteFile call if successful.
+  // Iff successful, assigns ownership of the client_info pointer to the server
+  // instance, in which case we must be sure not to free it in this function.
   if (!RespondToClient(client_info.get())) {
-    server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+    EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
     return;
   }
 
+  client_info_ = client_info.release();
+
   // Note that the asynchronous write issued by RespondToClient function
   // can finish before  the code below executes. But it is okay to change
   // state after issuing the asynchronous write. This is because even if
   // the asynchronous write is done, the callback for it would not be
   // executed until the current thread finishes its execution.
-  server_state_ = IPC_SERVER_STATE_WRITING;
-  client_info_ = client_info.release();
+  EnterStateWhenSignaled(IPC_SERVER_STATE_WRITING);
 }
 
 // When the server thread serving the clients is in the WRITING state,
@@ -449,21 +460,19 @@ void CrashGenerationServer::HandleWritingState() {
                                      &overlapped_,
                                      &bytes_count,
                                      FALSE) != FALSE;
+  DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
 
   if (success) {
-    server_state_ = IPC_SERVER_STATE_WRITE_DONE;
+    EnterStateImmediately(IPC_SERVER_STATE_WRITE_DONE);
     return;
   }
 
-  DWORD error_code;
-  error_code = GetLastError();
-
   // We should never get an I/O incomplete since we should not execute this
   // unless the Write has finished and the overlapped event is signaled. If
   // we do get INCOMPLETE, we have a bug in our code.
   assert(error_code != ERROR_IO_INCOMPLETE);
 
-  server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+  EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
 }
 
 // When the server thread serving the clients is in the WRITE_DONE state,
@@ -473,23 +482,20 @@ void CrashGenerationServer::HandleWritingState() {
 void CrashGenerationServer::HandleWriteDoneState() {
   assert(server_state_ == IPC_SERVER_STATE_WRITE_DONE);
 
-  server_state_ = IPC_SERVER_STATE_READING_ACK;
-
   DWORD bytes_count = 0;
   bool success = ReadFile(pipe_,
-                          &msg_,
-                          sizeof(msg_),
-                          &bytes_count,
-                          &overlapped_) != FALSE;
+                           &msg_,
+                           sizeof(msg_),
+                           &bytes_count,
+                           &overlapped_) != FALSE;
+  DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
 
   if (success) {
-    return;
-  }
-
-  DWORD error_code = GetLastError();
-
-  if (error_code != ERROR_IO_PENDING) {
-    server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+    EnterStateImmediately(IPC_SERVER_STATE_READING_ACK);
+  } else if (error_code == ERROR_IO_PENDING) {
+    EnterStateWhenSignaled(IPC_SERVER_STATE_READING_ACK);
+  } else {
+    EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
   }
 }
 
@@ -503,6 +509,7 @@ void CrashGenerationServer::HandleReadingAckState() {
                                      &overlapped_,
                                      &bytes_count,
                                      FALSE) != FALSE;
+  DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
 
   if (success) {
     // The connection handshake with the client is now complete; perform
@@ -511,15 +518,13 @@ void CrashGenerationServer::HandleReadingAckState() {
       connect_callback_(connect_context_, client_info_);
     }
   } else {
-    DWORD error_code = GetLastError();
-
     // We should never get an I/O incomplete since we should not execute this
     // unless the Read has finished and the overlapped event is signaled. If
     // we do get INCOMPLETE, we have a bug in our code.
     assert(error_code != ERROR_IO_INCOMPLETE);
   }
 
-  server_state_ = IPC_SERVER_STATE_DISCONNECTING;
+  EnterStateImmediately(IPC_SERVER_STATE_DISCONNECTING);
 }
 
 // When the server thread serving the client is in the DISCONNECTING state,
@@ -539,12 +544,12 @@ void CrashGenerationServer::HandleDisconnectingState() {
   overlapped_.Pointer = NULL;
 
   if (!ResetEvent(overlapped_.hEvent)) {
-    server_state_ = IPC_SERVER_STATE_ERROR;
+    EnterErrorState();
     return;
   }
 
   if (!DisconnectNamedPipe(pipe_)) {
-    server_state_ = IPC_SERVER_STATE_ERROR;
+    EnterErrorState();
     return;
   }
 
@@ -554,7 +559,21 @@ void CrashGenerationServer::HandleDisconnectingState() {
     return;
   }
 
-  server_state_ = IPC_SERVER_STATE_INITIAL;
+  EnterStateImmediately(IPC_SERVER_STATE_INITIAL);
+}
+
+void CrashGenerationServer::EnterErrorState() {
+  SetEvent(overlapped_.hEvent);
+  server_state_ = IPC_SERVER_STATE_ERROR;
+}
+
+void CrashGenerationServer::EnterStateWhenSignaled(IPCServerState state) {
+  server_state_ = state;
+}
+
+void CrashGenerationServer::EnterStateImmediately(IPCServerState state) {
+  server_state_ = state;
+
   if (!SetEvent(overlapped_.hEvent)) {
     server_state_ = IPC_SERVER_STATE_ERROR;
   }
@@ -626,18 +645,25 @@ bool CrashGenerationServer::RespondToClient(ClientInfo* client_info) {
     return false;
   }
 
-  if (!AddClient(client_info)) {
+  DWORD bytes_count = 0;
+  bool success = WriteFile(pipe_,
+                            &reply,
+                            sizeof(reply),
+                            &bytes_count,
+                            &overlapped_) != FALSE;
+  DWORD error_code = success ? ERROR_SUCCESS : GetLastError();
+
+  if (!success && error_code != ERROR_IO_PENDING) {
     return false;
   }
 
-  DWORD bytes_count = 0;
-  bool success = WriteFile(pipe_,
-                           &reply,
-                           sizeof(reply),
-                           &bytes_count,
-                           &overlapped_) != FALSE;
+  // Takes over ownership of client_info. We MUST return true if AddClient
+  // succeeds.
+  if (!AddClient(client_info)) {
+    return false;
+  }
 
-  return success || GetLastError() == ERROR_IO_PENDING;
+  return true;
 }
 
 // The server thread servicing the clients runs this method. The method
@@ -739,7 +765,7 @@ bool CrashGenerationServer::AddClient(ClientInfo* client_info) {
 
 // static
 void CALLBACK CrashGenerationServer::OnPipeConnected(void* context, BOOLEAN) {
-  assert (context);
+  assert(context);
 
   CrashGenerationServer* obj =
       reinterpret_cast<CrashGenerationServer*>(context);
diff --git a/src/client/windows/crash_generation/crash_generation_server.h b/src/client/windows/crash_generation/crash_generation_server.h
index cacb639a..31a353bf 100644
--- a/src/client/windows/crash_generation/crash_generation_server.h
+++ b/src/client/windows/crash_generation/crash_generation_server.h
@@ -33,11 +33,11 @@
 #include <list>
 #include <string>
 #include "client/windows/common/ipc_protocol.h"
-#include "client/windows/crash_generation/client_info.h"
 #include "client/windows/crash_generation/minidump_generator.h"
 #include "processor/scoped_ptr.h"
 
 namespace google_breakpad {
+class ClientInfo;
 
 // Abstraction for server side implementation of out-of-process crash
 // generation protocol for Windows platform only. It generates Windows
@@ -91,7 +91,8 @@ class CrashGenerationServer {
 
   ~CrashGenerationServer();
 
-  // Performs initialization steps needed to start listening to clients.
+  // Performs initialization steps needed to start listening to clients. Upon
+  // successful return clients may connect to this server's pipe.
   //
   // Returns true if initialization is successful; false otherwise.
   bool Start();
@@ -100,6 +101,9 @@ class CrashGenerationServer {
   // Various states the client can be in during the handshake with
   // the server.
   enum IPCServerState {
+    // Server starts in this state.
+    IPC_SERVER_STATE_UNINITIALIZED,
+
     // Server is in error state and it cannot serve any clients.
     IPC_SERVER_STATE_ERROR,
 
@@ -192,6 +196,21 @@ class CrashGenerationServer {
   // Generates dump for the given client.
   bool GenerateDump(const ClientInfo& client, std::wstring* dump_path);
 
+  // Puts the server in a permanent error state and sets a signal such that
+  // the state will be immediately entered after the current state transition
+  // is complete.
+  void EnterErrorState();
+
+  // Puts the server in the specified state and sets a signal such that the
+  // state is immediately entered after the current state transition is
+  // complete.
+  void EnterStateImmediately(IPCServerState state);
+
+  // Puts the server in the specified state. No signal will be set, so the state
+  // transition will only occur when signaled manually or by completion of an
+  // asynchronous IO operation.
+  void EnterStateWhenSignaled(IPCServerState state);
+
   // Sync object for thread-safe access to the shared list of clients.
   CRITICAL_SECTION clients_sync_;
 
diff --git a/src/client/windows/unittests/client_tests.gyp b/src/client/windows/unittests/client_tests.gyp
index 80c03ad4..5c5ef479 100755
--- a/src/client/windows/unittests/client_tests.gyp
+++ b/src/client/windows/unittests/client_tests.gyp
@@ -41,9 +41,11 @@
         'minidump_test.cc',
         'dump_analysis.cc',
         'dump_analysis.h',
+        'crash_generation_server_test.cc'
       ],
       'dependencies': [
-        'gtest.gyp:gtest',
+        'testing.gyp:gtest',
+        'testing.gyp:gmock',
         '../breakpad_client.gyp:common',
         '../crash_generation/crash_generation.gyp:crash_generation_server',
         '../crash_generation/crash_generation.gyp:crash_generation_client',
diff --git a/src/client/windows/unittests/crash_generation_server_test.cc b/src/client/windows/unittests/crash_generation_server_test.cc
new file mode 100644
index 00000000..75d0be97
--- /dev/null
+++ b/src/client/windows/unittests/crash_generation_server_test.cc
@@ -0,0 +1,298 @@
+// Copyright 2010, Google Inc.

+// All rights reserved.

+//

+// Redistribution and use in source and binary forms, with or without

+// modification, are permitted provided that the following conditions are

+// met:

+//

+//     * Redistributions of source code must retain the above copyright

+// notice, this list of conditions and the following disclaimer.

+//     * Redistributions in binary form must reproduce the above

+// copyright notice, this list of conditions and the following disclaimer

+// in the documentation and/or other materials provided with the

+// distribution.

+//     * Neither the name of Google Inc. nor the names of its

+// contributors may be used to endorse or promote products derived from

+// this software without specific prior written permission.

+//

+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS

+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT

+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR

+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT

+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,

+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT

+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,

+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY

+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT

+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE

+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

+

+

+#include "testing/gtest/include/gtest/gtest.h"

+#include "testing/include/gmock/gmock.h"

+

+#include "client/windows/crash_generation/crash_generation_server.h"

+#include "client/windows/common/ipc_protocol.h"

+

+using testing::_;

+

+namespace {

+

+const wchar_t kPipeName[] =

+  L"\\\\.\\pipe\\CrashGenerationServerTest\\TestCaseServer";

+

+const DWORD kPipeDesiredAccess = FILE_READ_DATA |

+                                 FILE_WRITE_DATA |

+                                 FILE_WRITE_ATTRIBUTES;

+

+const DWORD kPipeFlagsAndAttributes = SECURITY_IDENTIFICATION |

+                                      SECURITY_SQOS_PRESENT;

+

+const DWORD kPipeMode = PIPE_READMODE_MESSAGE;

+

+int kCustomInfoCount = 2;

+

+google_breakpad::CustomInfoEntry kCustomInfoEntries[] = {

+    google_breakpad::CustomInfoEntry(L"prod", L"CrashGenerationServerTest"),

+    google_breakpad::CustomInfoEntry(L"ver", L"1.0"),

+};

+

+class CrashGenerationServerTest : public ::testing::Test {

+ public:

+  CrashGenerationServerTest()

+      : crash_generation_server_(kPipeName,

+                                 NULL,

+                                 CallOnClientConnected, &mock_callbacks_,

+                                 CallOnClientDumpRequested, &mock_callbacks_,

+                                 CallOnClientExited, &mock_callbacks_,

+                                 false,

+                                 NULL),

+        thread_id_(0),

+        exception_pointers_(NULL) {

+    memset(&assert_info_, 0, sizeof(assert_info_));

+  }

+

+ protected:

+  class MockCrashGenerationServerCallbacks {

+   public:

+    MOCK_METHOD1(OnClientConnected,

+                 void(const google_breakpad::ClientInfo* client_info));

+    MOCK_METHOD2(OnClientDumpRequested,

+                 void(const google_breakpad::ClientInfo* client_info,

+                      const std::wstring* file_path));

+    MOCK_METHOD1(OnClientExited,

+                 void(const google_breakpad::ClientInfo* client_info));

+  };

+

+  enum ClientFault {

+    NO_FAULT,

+    CLOSE_AFTER_CONNECT,

+    SEND_INVALID_REGISTRATION,

+    TRUNCATE_REGISTRATION,

+    CLOSE_AFTER_REGISTRATION,

+    RESPONSE_BUFFER_TOO_SMALL,

+    CLOSE_AFTER_RESPONSE,

+    SEND_INVALID_ACK

+  };

+

+  void SetUp() {

+    ASSERT_TRUE(crash_generation_server_.Start());

+  }

+

+  void FaultyClient(ClientFault fault_type) {

+    HANDLE pipe = CreateFile(kPipeName,

+                             kPipeDesiredAccess,

+                             0,

+                             NULL,

+                             OPEN_EXISTING,

+                             kPipeFlagsAndAttributes,

+                             NULL);

+

+    if (pipe == INVALID_HANDLE_VALUE) {

+      ASSERT_EQ(ERROR_PIPE_BUSY, GetLastError());

+

+      // Cannot continue retrying if wait on pipe fails.

+      ASSERT_TRUE(WaitNamedPipe(kPipeName, 500));

+

+      pipe = CreateFile(kPipeName,

+                        kPipeDesiredAccess,

+                        0,

+                        NULL,

+                        OPEN_EXISTING,

+                        kPipeFlagsAndAttributes,

+                        NULL);

+    }

+

+    ASSERT_NE(pipe, INVALID_HANDLE_VALUE);

+

+    DWORD mode = kPipeMode;

+    ASSERT_TRUE(SetNamedPipeHandleState(pipe, &mode, NULL, NULL));

+

+    DoFaultyClient(fault_type, pipe);

+

+    CloseHandle(pipe);

+  }

+

+  void DoTestFault(ClientFault fault) {

+    EXPECT_CALL(mock_callbacks_, OnClientConnected(_)).Times(0);

+    ASSERT_NO_FATAL_FAILURE(FaultyClient(fault));

+    ASSERT_NO_FATAL_FAILURE(FaultyClient(fault));

+    ASSERT_NO_FATAL_FAILURE(FaultyClient(fault));

+

+    EXPECT_CALL(mock_callbacks_, OnClientConnected(_));

+

+    ASSERT_NO_FATAL_FAILURE(FaultyClient(NO_FAULT));

+

+    // Slight hack. The OnClientConnected is only invoked after the ack is

+    // received by the server. At that point, the FaultyClient call has already

+    // returned. The best way to wait until the server is done handling that is

+    // to send one more ping, whose processing will be blocked by delivery of

+    // the OnClientConnected message.

+    ASSERT_NO_FATAL_FAILURE(FaultyClient(CLOSE_AFTER_CONNECT));

+  }

+

+  MockCrashGenerationServerCallbacks mock_callbacks_;

+

+ private:

+  // Depends on the caller to successfully open the pipe before invocation and

+  // to close it immediately afterwards.

+  void DoFaultyClient(ClientFault fault_type, HANDLE pipe) {

+    if (fault_type == CLOSE_AFTER_CONNECT) {

+      return;

+    }

+

+    google_breakpad::CustomClientInfo custom_info = {kCustomInfoEntries,

+                                                     kCustomInfoCount};

+

+    google_breakpad::ProtocolMessage msg(

+      fault_type == SEND_INVALID_REGISTRATION ?

+        google_breakpad::MESSAGE_TAG_NONE :

+        google_breakpad::MESSAGE_TAG_REGISTRATION_REQUEST,

+      GetCurrentProcessId(),

+      MiniDumpNormal,

+      &thread_id_,

+      &exception_pointers_,

+      &assert_info_,

+      custom_info,

+      NULL,

+      NULL,

+      NULL);

+

+    DWORD bytes_count = 0;

+

+    ASSERT_TRUE(WriteFile(pipe,

+                          &msg,

+                          fault_type == TRUNCATE_REGISTRATION ?

+                            sizeof(msg) / 2 : sizeof(msg),

+                          &bytes_count,

+                          NULL));

+

+    if (fault_type == CLOSE_AFTER_REGISTRATION) {

+      return;

+    }

+

+    google_breakpad::ProtocolMessage reply;

+

+    if (!ReadFile(pipe,

+                  &reply,

+                  fault_type == RESPONSE_BUFFER_TOO_SMALL ?

+                    sizeof(google_breakpad::ProtocolMessage) / 2 :

+                    sizeof(google_breakpad::ProtocolMessage),

+                  &bytes_count,

+                  NULL)) {

+      switch (fault_type) {

+        case TRUNCATE_REGISTRATION:

+        case RESPONSE_BUFFER_TOO_SMALL:

+        case SEND_INVALID_REGISTRATION:

+          return;

+

+        default:

+          FAIL() << "Unexpectedly failed to register.";

+      }

+    }

+

+    if (fault_type == CLOSE_AFTER_RESPONSE) {

+      return;

+    }

+

+    google_breakpad::ProtocolMessage ack_msg;

+    ack_msg.tag = google_breakpad::MESSAGE_TAG_REGISTRATION_ACK;

+

+    ASSERT_TRUE(WriteFile(pipe,

+                          &ack_msg,

+                          SEND_INVALID_ACK ?

+                            sizeof(ack_msg) : sizeof(ack_msg) / 2,

+                          &bytes_count,

+                          NULL));

+

+    return;

+  }

+

+  static void CallOnClientConnected(

+    void* context, const google_breakpad::ClientInfo* client_info) {

+    static_cast<MockCrashGenerationServerCallbacks*>(context)->

+      OnClientConnected(client_info);

+  }

+

+  static void CallOnClientDumpRequested(

+    void* context,

+    const google_breakpad::ClientInfo* client_info,

+    const std::wstring* file_path) {

+    static_cast<MockCrashGenerationServerCallbacks*>(context)->

+      OnClientDumpRequested(client_info, file_path);

+  }

+

+  static void CallOnClientExited(

+    void* context, const google_breakpad::ClientInfo* client_info) {

+    static_cast<MockCrashGenerationServerCallbacks*>(context)->

+      OnClientExited(client_info);

+  }

+

+  DWORD thread_id_;

+  EXCEPTION_POINTERS* exception_pointers_;

+  MDRawAssertionInfo assert_info_;

+

+  google_breakpad::CrashGenerationServer crash_generation_server_;

+};

+

+TEST_F(CrashGenerationServerTest, PingServerTest) {

+  DoTestFault(CLOSE_AFTER_CONNECT);

+}

+

+TEST_F(CrashGenerationServerTest, InvalidRegistration) {

+  DoTestFault(SEND_INVALID_REGISTRATION);

+}

+

+TEST_F(CrashGenerationServerTest, TruncateRegistration) {

+  DoTestFault(TRUNCATE_REGISTRATION);

+}

+

+TEST_F(CrashGenerationServerTest, CloseAfterRegistration) {

+  DoTestFault(CLOSE_AFTER_REGISTRATION);

+}

+

+TEST_F(CrashGenerationServerTest, ResponseBufferTooSmall) {

+  DoTestFault(RESPONSE_BUFFER_TOO_SMALL);

+}

+

+TEST_F(CrashGenerationServerTest, CloseAfterResponse) {

+  DoTestFault(CLOSE_AFTER_RESPONSE);

+}

+

+// It turns out that, as long as you send one byte, the ACK is accepted and

+// registration succeeds.

+TEST_F(CrashGenerationServerTest, SendInvalidAck) {

+  EXPECT_CALL(mock_callbacks_, OnClientConnected(_));

+  ASSERT_NO_FATAL_FAILURE(FaultyClient(SEND_INVALID_ACK));

+

+  // See DoTestFault for an explanation of this line

+  ASSERT_NO_FATAL_FAILURE(FaultyClient(CLOSE_AFTER_CONNECT));

+

+  EXPECT_CALL(mock_callbacks_, OnClientConnected(_));

+  ASSERT_NO_FATAL_FAILURE(FaultyClient(NO_FAULT));

+

+  // See DoTestFault for an explanation of this line

+  ASSERT_NO_FATAL_FAILURE(FaultyClient(CLOSE_AFTER_CONNECT));

+}

+

+}  // anonymous namespace

diff --git a/src/client/windows/unittests/gtest.gyp b/src/client/windows/unittests/testing.gyp
index 1205a40c..85854288 100755..100644
--- a/src/client/windows/unittests/gtest.gyp
+++ b/src/client/windows/unittests/testing.gyp
@@ -44,7 +44,6 @@
       ],
       'sources': [
         '<(DEPTH)/testing/gtest/src/gtest-all.cc',
-        '<(DEPTH)/testing/gtest/src/gtest_main.cc',
       ],
       'direct_dependent_settings': {
         'include_dirs': [
@@ -53,5 +52,26 @@
         ]
       },
     },
+    {
+      'target_name': 'gmock',
+      'type': '<(library)',
+      'include_dirs': [
+        '<(DEPTH)/testing/include',
+        '<(DEPTH)/testing/',
+        '<(DEPTH)/testing/gtest',
+        '<(DEPTH)/testing/gtest/include',
+      ],
+      'sources': [
+        '<(DEPTH)/testing/src/gmock-all.cc',
+        '<(DEPTH)/testing/src/gmock_main.cc',
+      ],
+      'direct_dependent_settings': {
+        'include_dirs': [
+          '<(DEPTH)/testing/include',
+          '<(DEPTH)/testing/gtest/include',
+        ]
+      },
+    },
+
   ],
 }