Speculatively back out r984. See http://codereview.chromium.org/10805065/

and
http://build.chromium.org/p/chromium/builders/NACL%20Tests%20%28x64%29/builds/34563

chrome src/native_client/tests/inbrowser_crash_test/crash_dump_tester.py says
that the observed failures are a symptom of crash_service.exe itself crashing.


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

7 files changed, 11 insertions, 103 deletions

diff --git a/src/client/windows/common/ipc_protocol.h b/src/client/windows/common/ipc_protocol.h
index 42ea8455..b03c032b 100644
--- a/src/client/windows/common/ipc_protocol.h
+++ b/src/client/windows/common/ipc_protocol.h
@@ -82,11 +82,6 @@ struct CustomInfoEntry {
   wchar_t value[kValueMaxLength];
 };
 
-struct CustomDataStream {
-  size_t size;
-  u_int8_t stream[1];
-};
-
 // Constants for the protocol between client and the server.
 
 // Tags sent with each message indicating the purpose of
diff --git a/src/client/windows/crash_generation/client_info.cc b/src/client/windows/crash_generation/client_info.cc
index 294096f3..60bbac82 100644
--- a/src/client/windows/crash_generation/client_info.cc
+++ b/src/client/windows/crash_generation/client_info.cc
@@ -31,8 +31,6 @@
 #include "client/windows/common/ipc_protocol.h"
 
 static const wchar_t kCustomInfoProcessUptimeName[] = L"ptime";
-static const wchar_t kCustomDataStreamCustomFieldName[] = L"custom-data-stream";
-static const size_t kMaxCustomDataStreamSize = 100 * 1024 * 1024;
 static const size_t kMaxCustomInfoEntries = 4096;
 
 namespace google_breakpad {
@@ -50,7 +48,6 @@ ClientInfo::ClientInfo(CrashGenerationServer* crash_server,
       ex_info_(ex_info),
       assert_info_(assert_info),
       custom_client_info_(custom_client_info),
-      custom_data_stream_(NULL),
       thread_id_(thread_id),
       process_handle_(NULL),
       dump_requested_handle_(NULL),
@@ -89,11 +86,6 @@ bool ClientInfo::Initialize() {
 }
 
 ClientInfo::~ClientInfo() {
-  if (custom_data_stream_) {
-    delete custom_data_stream_;
-    custom_data_stream_ = NULL;
-  }
-
   if (dump_request_wait_handle_) {
     // Wait for callbacks that might already be running to finish.
     UnregisterWaitEx(dump_request_wait_handle_, INVALID_HANDLE_VALUE);
@@ -207,43 +199,6 @@ bool ClientInfo::PopulateCustomInfo() {
   return (bytes_count != read_count);
 }
 
-bool ClientInfo::PopulateCustomDataStream() {
-  for (SIZE_T i = 0; i < custom_client_info_.count; ++i) {
-    if (_wcsicmp(kCustomDataStreamCustomFieldName,
-                 custom_client_info_.entries[i].name) != 0) {
-      continue;
-    }
-    wchar_t address_str[CustomInfoEntry::kValueMaxLength];
-    memcpy(address_str, custom_client_info_.entries[i].value,
-           CustomInfoEntry::kValueMaxLength);
-    wchar_t* size_str = wcschr(address_str, ':');
-    if (!size_str)
-      return false;
-
-    size_str[0] = 0;
-    ++size_str;
-    void* address = reinterpret_cast<void*>(_wcstoi64(address_str, NULL, 16));
-    long size = wcstol(size_str, NULL, 16);
-    if (size <= 0 || size > kMaxCustomDataStreamSize)
-      return false;
-
-    custom_data_stream_ = reinterpret_cast<CustomDataStream*>(
-        new u_int8_t[sizeof(CustomDataStream) + size - 1]);
-
-    SIZE_T bytes_count = 0;
-    if (!ReadProcessMemory(process_handle_, address,
-                           custom_data_stream_->stream, size, &bytes_count)) {
-      delete custom_data_stream_;
-      custom_data_stream_ = NULL;
-      return false;
-    }
-
-    return true;
-  }
-
-  return false;
-}
-
 CustomClientInfo ClientInfo::GetCustomInfo() const {
   CustomClientInfo custom_info;
   custom_info.entries = custom_info_entries_.get();
diff --git a/src/client/windows/crash_generation/client_info.h b/src/client/windows/crash_generation/client_info.h
index 3d88ffd4..999e6678 100644
--- a/src/client/windows/crash_generation/client_info.h
+++ b/src/client/windows/crash_generation/client_info.h
@@ -61,7 +61,6 @@ class ClientInfo {
   MINIDUMP_TYPE dump_type() const { return dump_type_; }
   EXCEPTION_POINTERS** ex_info() const { return ex_info_; }
   MDRawAssertionInfo* assert_info() const { return assert_info_; }
-  CustomDataStream* custom_data_stream() const { return custom_data_stream_; }
   DWORD* thread_id() const { return thread_id_; }
   HANDLE process_handle() const { return process_handle_; }
   HANDLE dump_requested_handle() const { return dump_requested_handle_; }
@@ -91,10 +90,6 @@ class ClientInfo {
   bool GetClientExceptionInfo(EXCEPTION_POINTERS** ex_info) const;
   bool GetClientThreadId(DWORD* thread_id) const;
 
-  // Reads the custom data stream (if supplied) from the client process
-  // address space.
-  bool PopulateCustomDataStream();
-
   // Reads the custom information from the client process address space.
   bool PopulateCustomInfo();
 
@@ -135,9 +130,6 @@ class ClientInfo {
   // Custom information about the client.
   CustomClientInfo custom_client_info_;
 
-  // Custom data stream supplied by the client.
-  CustomDataStream* custom_data_stream_;
-
   // Contains the custom client info entries read from the client process
   // memory. This will be populated only if the method GetClientCustomInfo
   // is called.
diff --git a/src/client/windows/crash_generation/crash_generation_server.cc b/src/client/windows/crash_generation/crash_generation_server.cc
index de3d3fce..477973c1 100644
--- a/src/client/windows/crash_generation/crash_generation_server.cc
+++ b/src/client/windows/crash_generation/crash_generation_server.cc
@@ -796,7 +796,6 @@ void CALLBACK CrashGenerationServer::OnDumpRequest(void* context, BOOLEAN) {
   assert(context);
   ClientInfo* client_info = reinterpret_cast<ClientInfo*>(context);
   client_info->PopulateCustomInfo();
-  client_info->PopulateCustomDataStream();
 
   CrashGenerationServer* crash_server = client_info->crash_server();
   assert(crash_server);
@@ -893,7 +892,6 @@ bool CrashGenerationServer::GenerateDump(const ClientInfo& client,
                                         GetCurrentThreadId(),
                                         client_ex_info,
                                         client.assert_info(),
-                                        client.custom_data_stream(),
                                         client.dump_type(),
                                         true,
                                         dump_path);
diff --git a/src/client/windows/crash_generation/minidump_generator.cc b/src/client/windows/crash_generation/minidump_generator.cc
index f407ef8c..fe4937aa 100644
--- a/src/client/windows/crash_generation/minidump_generator.cc
+++ b/src/client/windows/crash_generation/minidump_generator.cc
@@ -271,15 +271,14 @@ bool MinidumpGenerator::WriteMinidump(HANDLE process_handle,
                                       DWORD requesting_thread_id,
                                       EXCEPTION_POINTERS* exception_pointers,
                                       MDRawAssertionInfo* assert_info,
-                                      CustomDataStream* custom_data_stream,
                                       MINIDUMP_TYPE dump_type,
                                       bool is_client_pointers,
                                       wstring* dump_path) {
   // Just call the full WriteMinidump with NULL as the full_dump_path.
   return this->WriteMinidump(process_handle, process_id, thread_id,
                              requesting_thread_id, exception_pointers,
-                             assert_info, custom_data_stream, dump_type,
-                             is_client_pointers, dump_path, NULL);
+                             assert_info, dump_type, is_client_pointers,
+                             dump_path, NULL);
 }
 
 bool MinidumpGenerator::WriteMinidump(HANDLE process_handle,
@@ -288,7 +287,6 @@ bool MinidumpGenerator::WriteMinidump(HANDLE process_handle,
                                       DWORD requesting_thread_id,
                                       EXCEPTION_POINTERS* exception_pointers,
                                       MDRawAssertionInfo* assert_info,
-                                      CustomDataStream* custom_data_stream,
                                       MINIDUMP_TYPE dump_type,
                                       bool is_client_pointers,
                                       wstring* dump_path,
@@ -370,9 +368,9 @@ bool MinidumpGenerator::WriteMinidump(HANDLE process_handle,
     breakpad_info.requesting_thread_id = requesting_thread_id;
   }
 
-  // Leave room in user_stream_array for possible assertion info, handle
-  // operations, and custom data streams.
-  MINIDUMP_USER_STREAM user_stream_array[4];
+  // Leave room in user_stream_array for possible assertion info and handle
+  // operations streams.
+  MINIDUMP_USER_STREAM user_stream_array[3];
   user_stream_array[0].Type = MD_BREAKPAD_INFO_STREAM;
   user_stream_array[0].BufferSize = sizeof(breakpad_info);
   user_stream_array[0].Buffer = &breakpad_info;
@@ -416,16 +414,6 @@ bool MinidumpGenerator::WriteMinidump(HANDLE process_handle,
     ++user_streams.UserStreamCount;
   }
 
-  if (custom_data_stream) {
-    user_stream_array[user_streams.UserStreamCount].Type =
-        MD_CUSTOM_DATA_STREAM;
-    user_stream_array[user_streams.UserStreamCount].BufferSize =
-        static_cast<ULONG>(custom_data_stream->size);
-    user_stream_array[user_streams.UserStreamCount].Buffer =
-        custom_data_stream->stream;
-    ++user_streams.UserStreamCount;
-  }
-
   // If the process is terminated by STATUS_INVALID_HANDLE exception store
   // the trace of operatios for the offending handle value. Do nothing special
   // if the client already requested the handle trace to be stored in the dump.
diff --git a/src/client/windows/crash_generation/minidump_generator.h b/src/client/windows/crash_generation/minidump_generator.h
index b2e973e6..5f9e4b54 100644
--- a/src/client/windows/crash_generation/minidump_generator.h
+++ b/src/client/windows/crash_generation/minidump_generator.h
@@ -33,7 +33,6 @@
 #include <windows.h>
 #include <dbghelp.h>
 #include <list>
-#include "client/windows/common/ipc_protocol.h"
 #include "google_breakpad/common/minidump_format.h"
 
 namespace google_breakpad {
@@ -58,7 +57,6 @@ class MinidumpGenerator {
                      DWORD requesting_thread_id,
                      EXCEPTION_POINTERS* exception_pointers,
                      MDRawAssertionInfo* assert_info,
-                     CustomDataStream* custom_data_stream,
                      MINIDUMP_TYPE dump_type,
                      bool is_client_pointers,
                      std::wstring* dump_path);
@@ -72,7 +70,6 @@ class MinidumpGenerator {
                      DWORD requesting_thread_id,
                      EXCEPTION_POINTERS* exception_pointers,
                      MDRawAssertionInfo* assert_info,
-                     CustomDataStream* custom_data_stream,
                      MINIDUMP_TYPE dump_type,
                      bool is_client_pointers,
                      std::wstring* dump_path,
diff --git a/src/client/windows/unittests/minidump_test.cc b/src/client/windows/unittests/minidump_test.cc
index f47ee598..ab7ae3b7 100644
--- a/src/client/windows/unittests/minidump_test.cc
+++ b/src/client/windows/unittests/minidump_test.cc
@@ -31,7 +31,6 @@
 #include <objbase.h>
 #include <dbghelp.h>
 
-#include "../common/ipc_protocol.h"
 #include "../crash_generation/minidump_generator.h"
 #include "dump_analysis.h"  // NOLINT
 
@@ -87,8 +86,7 @@ class MinidumpTest: public testing::Test {
     }
   }
 
-  bool WriteDump(ULONG flags, MDRawAssertionInfo* assert,
-                 google_breakpad::CustomDataStream* custom_data) {
+  bool WriteDump(ULONG flags) {
     using google_breakpad::MinidumpGenerator;
 
     // Fake exception is access violation on write to this.
@@ -114,8 +112,7 @@ class MinidumpTest: public testing::Test {
                                           ::GetCurrentThreadId(),
                                           ::GetCurrentThreadId(),
                                           &ex_ptrs,
-                                          assert,
-                                          custom_data,
+                                          NULL,
                                           static_cast<MINIDUMP_TYPE>(flags),
                                           TRUE,
                                           &dump_file_,
@@ -180,7 +177,7 @@ TEST_F(MinidumpTest, Version) {
 }
 
 TEST_F(MinidumpTest, Normal) {
-  EXPECT_TRUE(WriteDump(MiniDumpNormal, NULL, NULL));
+  EXPECT_TRUE(WriteDump(MiniDumpNormal));
   DumpAnalysis mini(dump_file_);
 
   // We expect threads, modules and some memory.
@@ -209,13 +206,10 @@ TEST_F(MinidumpTest, Normal) {
 
   // We expect no off-stack memory in this dump.
   EXPECT_FALSE(mini.HasMemory(this));
-
-  // We do not expect a custom data stream.
-  EXPECT_FALSE(mini.HasStream(MD_CUSTOM_DATA_STREAM));
 }
 
 TEST_F(MinidumpTest, SmallDump) {
-  ASSERT_TRUE(WriteDump(kSmallDumpType, NULL, NULL));
+  ASSERT_TRUE(WriteDump(kSmallDumpType));
   DumpAnalysis mini(dump_file_);
 
   EXPECT_TRUE(mini.HasStream(ThreadListStream));
@@ -246,7 +240,7 @@ TEST_F(MinidumpTest, SmallDump) {
 }
 
 TEST_F(MinidumpTest, LargerDump) {
-  ASSERT_TRUE(WriteDump(kLargerDumpType, NULL, NULL));
+  ASSERT_TRUE(WriteDump(kLargerDumpType));
   DumpAnalysis mini(dump_file_);
 
   // The dump should have all of these streams.
@@ -278,7 +272,7 @@ TEST_F(MinidumpTest, LargerDump) {
 }
 
 TEST_F(MinidumpTest, FullDump) {
-  ASSERT_TRUE(WriteDump(kFullDumpType, NULL, NULL));
+  ASSERT_TRUE(WriteDump(kFullDumpType));
   ASSERT_TRUE(dump_file_ != L"");
   ASSERT_TRUE(full_dump_file_ != L"");
   DumpAnalysis mini(dump_file_);
@@ -335,15 +329,4 @@ TEST_F(MinidumpTest, FullDump) {
   EXPECT_FALSE(full.HasStream(TokenStream));
 }
 
-TEST_F(MinidumpTest, CustomData) {
-  google_breakpad::CustomDataStream custom_data;
-  custom_data.size = 1;
-  custom_data.stream[0] = 'A';
-  EXPECT_TRUE(WriteDump(MiniDumpNormal, NULL, &custom_data));
-  DumpAnalysis mini(dump_file_);
-
-  // We expect a custom data stream.
-  EXPECT_TRUE(mini.HasStream(MD_CUSTOM_DATA_STREAM));
-}
-
 }  // namespace