Speculative back-out of r883, which may have broken Windows crash reporting.

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

src/client/windows/common/ipc_protocol.h

@@ -110,7 +110,6 @@ struct ProtocolMessage {
         custom_client_info(),
         dump_request_handle(NULL),
         dump_generated_handle(NULL),
-        parent_dump_request_handle(NULL),
         server_alive_handle(NULL) {
   }
 
@@ -123,7 +122,6 @@ struct ProtocolMessage {
                   MDRawAssertionInfo* arg_assert_info,
                   const CustomClientInfo& custom_info,
                   HANDLE arg_dump_request_handle,
-                  HANDLE arg_parent_dump_request_handle,
                   HANDLE arg_dump_generated_handle,
                   HANDLE arg_server_alive)
     : tag(arg_tag),
@@ -134,7 +132,6 @@ struct ProtocolMessage {
       assert_info(arg_assert_info),
       custom_client_info(custom_info),
       dump_request_handle(arg_dump_request_handle),
-      parent_dump_request_handle(arg_parent_dump_request_handle),
       dump_generated_handle(arg_dump_generated_handle),
       server_alive_handle(arg_server_alive) {
   }
@@ -164,9 +161,6 @@ struct ProtocolMessage {
   // Handle to signal the crash event.
   HANDLE dump_request_handle;
 
-  // Handle to signal a request for a server side minidump.
-  HANDLE parent_dump_request_handle;
-
   // Handle to check if server is done generating crash.
   HANDLE dump_generated_handle;
 

src/client/windows/crash_generation/client_info.cc

@@ -50,8 +50,6 @@ ClientInfo::ClientInfo(CrashGenerationServer* crash_server,
       thread_id_(thread_id),
       process_handle_(NULL),
       dump_requested_handle_(NULL),
-      parent_dump_requested_handle_(NULL),
-      parent_dump_request_wait_handle_(NULL),
       dump_generated_handle_(NULL),
       dump_request_wait_handle_(NULL),
       process_exit_wait_handle_(NULL) {
@@ -72,14 +70,6 @@ bool ClientInfo::Initialize() {
     return false;
   }
 
-  parent_dump_requested_handle_ = CreateEvent(NULL,    // Security attributes.
-                                              TRUE,    // Manual reset.
-                                              FALSE,   // Initial state.
-                                              NULL);   // Name.
-  if (!parent_dump_requested_handle_) {
-    return false;
-  }
-
   dump_generated_handle_ = CreateEvent(NULL,    // Security attributes.
                                        TRUE,    // Manual reset.
                                        FALSE,   // Initial state.
@@ -98,10 +88,6 @@ ClientInfo::~ClientInfo() {
     UnregisterWaitEx(process_exit_wait_handle_, INVALID_HANDLE_VALUE);
   }
 
-  if (parent_dump_request_wait_handle_) {
-    UnregisterWaitEx(parent_dump_request_wait_handle_, INVALID_HANDLE_VALUE);
-  }
-
   if (process_handle_) {
     CloseHandle(process_handle_);
   }
@@ -110,10 +96,6 @@ ClientInfo::~ClientInfo() {
     CloseHandle(dump_requested_handle_);
   }
 
-  if (parent_dump_requested_handle_) {
-    CloseHandle(parent_dump_requested_handle_);
-  }
-
   if (dump_generated_handle_) {
     CloseHandle(dump_generated_handle_);
   }
@@ -125,11 +107,6 @@ void ClientInfo::UnregisterWaits() {
     dump_request_wait_handle_ = NULL;
   }
 
-  if (parent_dump_request_wait_handle_) {
-    UnregisterWait(parent_dump_request_wait_handle_);
-    parent_dump_request_wait_handle_ = NULL;
-  }
-
   if (process_exit_wait_handle_) {
     UnregisterWait(process_exit_wait_handle_);
     process_exit_wait_handle_ = NULL;

src/client/windows/crash_generation/client_info.h

@@ -65,7 +65,6 @@ class ClientInfo {
   HANDLE process_handle() const { return process_handle_; }
   HANDLE dump_requested_handle() const { return dump_requested_handle_; }
   HANDLE dump_generated_handle() const { return dump_generated_handle_; }
-  HANDLE parent_dump_requested_handle() const { return parent_dump_requested_handle_; }
 
   HANDLE dump_request_wait_handle() const {
     return dump_request_wait_handle_;
@@ -83,14 +82,6 @@ class ClientInfo {
     process_exit_wait_handle_ = value;
   }
 
-  HANDLE parent_dump_request_wait_handle() const {
-    return parent_dump_request_wait_handle_;
-  }
-
-  void set_parent_dump_request_wait_handle(HANDLE value) {
-    parent_dump_request_wait_handle_ = value;
-  }
-
   // Unregister all waits for the client.
   void UnregisterWaits();
 
@@ -159,18 +150,12 @@ class ClientInfo {
   // Dump generated event handle.
   HANDLE dump_generated_handle_;
 
-  // Parent dump request event handle.
-  HANDLE parent_dump_requested_handle_;
-
   // Wait handle for dump request event.
   HANDLE dump_request_wait_handle_;
 
   // Wait handle for process exit event.
   HANDLE process_exit_wait_handle_;
 
-  // Wait handle for parent dump request event.
-  HANDLE parent_dump_request_wait_handle_;
-
   // Time when the client process started. It is used to determine the uptime
   // for the client process when it signals a crash.
   FILETIME start_time_;

src/client/windows/crash_generation/crash_generation_client.cc

@@ -194,7 +194,6 @@ bool CrashGenerationClient::RegisterClient(HANDLE pipe) {
                       custom_info_,
                       NULL,
                       NULL,
-                      NULL,
                       NULL);
   ProtocolMessage reply;
   DWORD bytes_count = 0;
@@ -221,10 +220,8 @@ bool CrashGenerationClient::RegisterClient(HANDLE pipe) {
   if (!WriteFile(pipe, &ack_msg, sizeof(ack_msg), &bytes_count, NULL)) {
     return false;
   }
-
   crash_event_ = reply.dump_request_handle;
   crash_generated_ = reply.dump_generated_handle;
-  parent_dump_request_event_ = reply.parent_dump_request_handle;
   server_alive_ = reply.server_alive_handle;
   server_process_id_ = reply.pid;
 
@@ -274,39 +271,6 @@ bool CrashGenerationClient::IsRegistered() const {
   return crash_event_ != NULL;
 }
 
-bool CrashGenerationClient::RequestParentDump() {
-  if (!IsRegistered()) {
-    return false;
-  }
-
-  assert(parent_dump_request_event_);
-  assert(server_alive_);
-
-  // Reset the dump generated event before signaling the crash
-  // event so that the server can set the dump generated event
-  // once it is done generating the event.
-  if (!ResetEvent(crash_generated_)) {
-    return false;
-  }
-
-  // Signal we want a server side crash dump generated
-  if (!SetEvent(parent_dump_request_event_)) {
-    return false;
-  }
-
-  // Wait for the crash dump process to complete
-  HANDLE wait_handles[kWaitEventCount] = {crash_generated_, server_alive_};
-
-  DWORD result = WaitForMultipleObjects(kWaitEventCount,
-                                        wait_handles,
-                                        FALSE,
-                                        kWaitForServerTimeoutMs);
-
-  // Crash dump was successfully generated only if the server
-  // signaled the crash generated event.
-  return result == WAIT_OBJECT_0;
-}
-
 bool CrashGenerationClient::RequestDump(EXCEPTION_POINTERS* ex_info,
                                         MDRawAssertionInfo* assert_info) {
   if (!IsRegistered()) {

src/client/windows/crash_generation/crash_generation_client.h

@@ -92,15 +92,6 @@ class CrashGenerationClient {
   // false will be returned.
   bool RequestDump(MDRawAssertionInfo* assert_info);
 
-  // Requests the crash server generate a minidump on the parent side
-  // of the connection. Blocks until the dump has been completed or
-  // the request times out.
-  //
-  // Returns true if the dump was successful; false otherwise. Note that
-  // if the registration step was not performed or it was not successful,
-  // false will be returned.
-  bool RequestParentDump();
-
  private:
   // Connects to the appropriate pipe and sets the pipe handle state.
   //
@@ -141,10 +132,6 @@ class CrashGenerationClient {
   // Event to signal in case of a crash.
   HANDLE crash_event_;
 
-  // Event signaling a server side minidump is being requested
-  // by the client.
-  HANDLE parent_dump_request_event_;
-
   // Handle to wait on after signaling a crash for the server
   // to finish generating crash dump.
   HANDLE crash_generated_;

src/client/windows/crash_generation/crash_generation_server.cc

@@ -119,8 +119,7 @@ CrashGenerationServer::CrashGenerationServer(
       shutting_down_(false),
       overlapped_(),
       client_info_(NULL),
-      cleanup_item_count_(0),
+      cleanup_item_count_(0) {
-      preferred_parent_thread_id_(0) {
   InitializeCriticalSection(&clients_sync_);
 
   if (dump_path) {
@@ -601,9 +600,6 @@ bool CrashGenerationServer::PrepareReply(const ClientInfo& client_info,
     CloseHandle(reply->server_alive_handle);
   }
 
-  if (reply->parent_dump_request_handle) {
-    CloseHandle(reply->parent_dump_request_handle);
-  }
   return false;
 }
 
@@ -630,16 +626,6 @@ bool CrashGenerationServer::CreateClientHandles(const ClientInfo& client_info,
     return false;
   }
 
-  if (!DuplicateHandle(current_process,
-                       client_info.parent_dump_requested_handle(),
-                       client_info.process_handle(),
-                       &reply->parent_dump_request_handle,
-                       kDumpRequestEventAccess,
-                       FALSE,
-                       0)) {
-    return false;
-  }
-
   if (!DuplicateHandle(current_process,
                        server_alive_handle_,
                        client_info.process_handle(),
@@ -768,20 +754,6 @@ bool CrashGenerationServer::AddClient(ClientInfo* client_info) {
 
   client_info->set_process_exit_wait_handle(process_wait_handle);
 
-  // OnParentDumpRequest will be called if the client requests
-  // a server side minidump be generated.
-  HANDLE parent_request_wait_handle = NULL;
-  if (!RegisterWaitForSingleObject(&parent_request_wait_handle,
-                                   client_info->parent_dump_requested_handle(),
-                                   OnParentDumpRequest,
-                                   client_info,
-                                   INFINITE,
-                                   kDumpRequestThreadFlags)) {
-    return false;
-  }
-
-  client_info->set_parent_dump_request_wait_handle(parent_request_wait_handle);
-
   // New scope to hold the lock for the shortest time.
   {
     AutoCriticalSection lock(&clients_sync_);
@@ -813,20 +785,6 @@ void CALLBACK CrashGenerationServer::OnDumpRequest(void* context, BOOLEAN) {
   ResetEvent(client_info->dump_requested_handle());
 }
 
-// static
-void CALLBACK CrashGenerationServer::OnParentDumpRequest(void* context, BOOLEAN) {
-  assert(context);
-  ClientInfo* client_info = reinterpret_cast<ClientInfo*>(context);
-  client_info->PopulateCustomInfo();
-
-  CrashGenerationServer* crash_server = client_info->crash_server();
-  assert(crash_server);
-
-  crash_server->HandleParentDumpRequest(*client_info);
-
-  ResetEvent(client_info->parent_dump_requested_handle());
-}
-
 // static
 void CALLBACK CrashGenerationServer::OnClientEnd(void* context, BOOLEAN) {
   assert(context);
@@ -885,34 +843,7 @@ void CrashGenerationServer::HandleDumpRequest(const ClientInfo& client_info) {
 
   if (dump_callback_) {
     std::wstring* ptr_dump_path = (dump_path == L"") ? NULL : &dump_path;
-    dump_callback_(dump_context_, &client_info, DUMP_REQ_CHILD, ptr_dump_path);
+    dump_callback_(dump_context_, &client_info, ptr_dump_path);
-  }
-
-  SetEvent(client_info.dump_generated_handle());
-}
-
-void CrashGenerationServer::HandleParentDumpRequest(const ClientInfo& client_info) {
-  std::wstring dump_path;
-  if (generate_dumps_) {
-    DWORD preferred_thread_id = !preferred_parent_thread_id_ ?
-      GetCurrentThreadId() : preferred_parent_thread_id_;
-    bool success =
-      dump_generator_->WriteMinidump(GetCurrentProcess(),
-                                     GetCurrentProcessId(),
-                                     preferred_thread_id,
-                                     GetCurrentThreadId(),
-                                     NULL, // no exception
-                                     NULL, // no assert info
-                                     MiniDumpNormal,
-                                     true,
-                                     &dump_path);
-    if (!success)
-      return;
-  }
-
-  if (dump_callback_) {
-    std::wstring* ptr_dump_path = (dump_path == L"") ? NULL : &dump_path;
-    dump_callback_(dump_context_, &client_info, DUMP_REQ_PARENT, ptr_dump_path);
   }
 
   SetEvent(client_info.dump_generated_handle());

src/client/windows/crash_generation/crash_generation_server.h

@@ -49,19 +49,11 @@ class ClientInfo;
 // generation in this way, the server generates Windows minidump files.
 class CrashGenerationServer {
  public:
-  // For client minidump request callbacks indicates the type of minidump
-  // the request generated.
-  enum ClientDumpRequestType {
-    DUMP_REQ_PARENT,
-    DUMP_REQ_CHILD,
-  };
-
   typedef void (*OnClientConnectedCallback)(void* context,
                                             const ClientInfo* client_info);
 
   typedef void (*OnClientDumpRequestCallback)(void* context,
                                               const ClientInfo* client_info,
-                                              const ClientDumpRequestType request_type,
                                               const std::wstring* file_path);
 
   typedef void (*OnClientExitedCallback)(void* context,
@@ -105,11 +97,6 @@ class CrashGenerationServer {
   // Returns true if initialization is successful; false otherwise.
   bool Start();
 
-  // Sets the preferred parent side minidump thread id passed
-  // to breakpad when a server side minidump is requested by
-  // a client.
-  void SetParentDumpThreadId(DWORD thread_id) { preferred_parent_thread_id_ = thread_id; }
-
  private:
   // Various states the client can be in during the handshake with
   // the server.
@@ -188,18 +175,12 @@ class CrashGenerationServer {
   // Handles a dump request from the client.
   void HandleDumpRequest(const ClientInfo& client_info);
 
-  // Handles a server side minidump request from the client.
-  void HandleParentDumpRequest(const ClientInfo& client_info);
-
   // Callback for pipe connected event.
   static void CALLBACK OnPipeConnected(void* context, BOOLEAN timer_or_wait);
 
   // Callback for a dump request.
   static void CALLBACK OnDumpRequest(void* context, BOOLEAN timer_or_wait);
 
-  // Callback for a server minidump request.
-  static void CALLBACK OnParentDumpRequest(void* context, BOOLEAN timer_or_wait);
-
   // Callback for client process exit event.
   static void CALLBACK OnClientEnd(void* context, BOOLEAN timer_or_wait);
 
@@ -297,10 +278,6 @@ class CrashGenerationServer {
   // already queued to run.
   volatile LONG cleanup_item_count_;
 
-  // For client requested server side minidumps, the preferred
-  // minidump thread id passed to breakpad.
-  DWORD preferred_parent_thread_id_;
-
   // Disable copy ctor and operator=.
   CrashGenerationServer(const CrashGenerationServer& crash_server);
   CrashGenerationServer& operator=(const CrashGenerationServer& crash_server);

src/client/windows/handler/exception_handler.cc

@@ -704,12 +704,6 @@ bool ExceptionHandler::WriteMinidump(const wstring &dump_path,
   return handler.WriteMinidump();
 }
 
-bool ExceptionHandler::RequestMinidumpForParent() {
-  if (!IsOutOfProcess())
-    return false;
-  return crash_generation_client_->RequestParentDump();
-}
-
 bool ExceptionHandler::WriteMinidumpWithException(
     DWORD requesting_thread_id,
     EXCEPTION_POINTERS* exinfo,

src/client/windows/handler/exception_handler.h

@@ -190,12 +190,6 @@ class ExceptionHandler {
   static bool WriteMinidump(const wstring &dump_path,
                             MinidumpCallback callback, void* callback_context);
 
-  // Requests a minidump of the server be generated and waits for completion.
-  // This can be used to capture the execution state of parent process
-  // independent of a crash on either side of the connection. Only valid for
-  // out-of-process clients.
-  bool RequestMinidumpForParent();
-
   // Get the thread ID of the thread requesting the dump (either the exception
   // thread or any other thread that called WriteMinidump directly).  This
   // may be useful if you want to include additional thread state in your

src/client/windows/tests/crash_generation_app/crash_generation_app.cc

@@ -216,7 +216,6 @@ static void _cdecl ShowClientConnected(void* context,
 
 static void _cdecl ShowClientCrashed(void* context,
                                      const ClientInfo* client_info,
-                                     const CrashGenerationServer::ClientDumpRequestType request_type,
                                      const wstring* dump_path) {
   TCHAR* line = new TCHAR[kMaximumLineLength];
   line[0] = _T('\0');

src/client/windows/unittests/crash_generation_server_test.cc

@@ -77,9 +77,8 @@ class CrashGenerationServerTest : public ::testing::Test {
    public:
     MOCK_METHOD1(OnClientConnected,
                  void(const google_breakpad::ClientInfo* client_info));
-    MOCK_METHOD3(OnClientDumpRequested,
+    MOCK_METHOD2(OnClientDumpRequested,
                  void(const google_breakpad::ClientInfo* client_info,
-                      const google_breakpad::CrashGenerationServer::ClientDumpRequestType request_type,
                       const std::wstring* file_path));
     MOCK_METHOD1(OnClientExited,
                  void(const google_breakpad::ClientInfo* client_info));
@@ -177,7 +176,6 @@ class CrashGenerationServerTest : public ::testing::Test {
       custom_info,
       NULL,
       NULL,
-      NULL,
       NULL);
 
     DWORD bytes_count = 0;
@@ -239,10 +237,9 @@ class CrashGenerationServerTest : public ::testing::Test {
   static void CallOnClientDumpRequested(
     void* context,
     const google_breakpad::ClientInfo* client_info,
-    const google_breakpad::CrashGenerationServer::ClientDumpRequestType request_type,
     const std::wstring* file_path) {
     static_cast<MockCrashGenerationServerCallbacks*>(context)->
-      OnClientDumpRequested(client_info, request_type, file_path);
+      OnClientDumpRequested(client_info, file_path);
   }
 
   static void CallOnClientExited(

src/client/windows/unittests/exception_handler_death_test.cc

@@ -131,7 +131,6 @@ static bool gDumpCallbackCalled = false;
 
 void clientDumpCallback(void *dump_context,
                         const google_breakpad::ClientInfo *client_info,
-                        const google_breakpad::CrashGenerationServer::ClientDumpRequestType request_type,
                         const std::wstring *dump_path) {
   gDumpCallbackCalled = true;
 }

src/client/windows/unittests/exception_handler_test.cc

@@ -78,7 +78,6 @@ class ExceptionHandlerTest : public ::testing::Test {
   static void ClientDumpCallback(
       void *dump_context,
       const google_breakpad::ClientInfo *client_info,
-      const google_breakpad::CrashGenerationServer::ClientDumpRequestType request_type,
       const std::wstring *dump_path);
 
   static bool DumpCallback(const wchar_t* dump_path,
@@ -139,7 +138,6 @@ BOOL ExceptionHandlerTest::DoesPathExist(const TCHAR *path_name) {
 void ExceptionHandlerTest::ClientDumpCallback(
     void *dump_context,
     const google_breakpad::ClientInfo *client_info,
-    const google_breakpad::CrashGenerationServer::ClientDumpRequestType request_type,
     const wstring *dump_path) {
   dump_file = *dump_path;
   // Create the full dump file name from the dump path.