Implement CrashUploadListWin to enable chrome://crashes on Windows.

chrome/browser/crash_upload_list_win.cc

Index: chrome/browser/crash_upload_list_win.cc
diff --git a/chrome/browser/crash_upload_list_win.cc b/chrome/browser/crash_upload_list_win.cc
new file mode 100755
index 0000000000000000000000000000000000000000..cb66f995c33f663bada44127e8d36e3801a09327
--- /dev/null
+++ b/chrome/browser/crash_upload_list_win.cc
@@ -0,0 +1,73 @@
+// Copyright (c) 2011 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.
+
+#include "chrome/browser/crash_upload_list_win.h"
+
+#include "base/stringprintf.h"
+#include "base/string_util.h"
+#include "base/sys_string_conversions.h"
+
+CrashUploadListWin::CrashUploadListWin(Delegate* delegate)
+    : CrashUploadList(delegate) {}
+
+void CrashUploadListWin::LoadCrashList() {
+  std::vector<uint8> buffer(1024);
+  HANDLE event_log = OpenEventLog(NULL, L"Application");
+  if (event_log) {
+    while (true) {
+      DWORD bytes_read;
+      DWORD bytes_needed;
+      BOOL success =
+          ReadEventLog(event_log,
+                       EVENTLOG_SEQUENTIAL_READ | EVENTLOG_BACKWARDS_READ,
+                       0,
+                       &buffer[0],
+                       buffer.size(),
+                       &bytes_read,
+                       &bytes_needed);
+      if (success) {
+        DWORD record_offset = 0;
+        while (record_offset < bytes_read) {
+          EVENTLOGRECORD* record = (EVENTLOGRECORD*)&buffer[record_offset];

[MAD, 2011/03/31 14:07:58]

I'm not sure this works... What if the beginning of the record fits within the
bytes_read, but it gets truncated in the middle?

To make sure this doesn't happen, we should make sure that the size of the read
buffer is a multiple of the size of a record, right?

[Alexei Svitkine (slow), 2011/03/31 14:36:47]

It's not explicitly clarified in the API docs:

http://msdn.microsoft.com/en-us/library/aa363674(v=vs.85).aspx

However, we can't do the sizeof() comparison checks because records are variable
length - they have trailing data past the struct header, and each record's
length is specified by record->Length.

I would assume that the API would not return partial records - but again, it is
not explicitly stated in the docs. If we want to be paranoid, we can check that
the size left is enough to fit sizeof(EVENTLOGRECORD), and if that's the case,
do a second check using record->Length.

What do you think?

[MAD, 2011/03/31 14:42:51]

Good point, I forgot about the dynamic size allowing contained data to be of any
size...

Yes, please add the check that we have at least enough room for the struct, and
then use the size field to confirm we also got all the dymanic data...

Thanks!

+          if (IsPossibleCrashLogRecord(record))
+            ProcessPossibleCrashLogRecord(record);
+          record_offset += record->Length;
+        }
+      } else if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) {
+        // Resize buffer to the required minimum size.
+        buffer.resize(bytes_needed);
+      } else {
+        // Stop on any other error, including the expected case
+        // of ERROR_HANDLE_EOF.

[MAD, 2011/03/31 14:07:58]

I think we should DCHECK that we actually got ERROR_HANDLE_EOF, so that we see
when other errors occur... Or at least log them...

[Alexei Svitkine (slow), 2011/03/31 14:36:47]

Good idea! Done.

+        break;
+      }
+    }
+    CloseEventLog(event_log);
+  }
+}
+
+bool CrashUploadListWin::IsPossibleCrashLogRecord(
+    EVENTLOGRECORD* record) const {
+  LPWSTR provider_name = (LPWSTR)((uint8*)record + sizeof(EVENTLOGRECORD));
+  return !wcscmp(L"Chrome", provider_name) &&
+      record->EventType == EVENTLOG_INFORMATION_TYPE &&
+      record->NumStrings >= 1;
+}
+
+void CrashUploadListWin::ProcessPossibleCrashLogRecord(EVENTLOGRECORD* record) {
+  // Add the crash if the message matches the expected pattern.
+  const std::wstring pattern_prefix(L"Crash uploaded. Id=");
+  const std::wstring pattern_suffix(L".");
+  const unsigned int pattern_length =
+      pattern_prefix.size() + pattern_suffix.size();

[MAD, 2011/03/31 14:07:58]

Are you sure about this? I think you need to find the distance between where
these two are found in the string instead of the sum of their sizes, right?

[Alexei Svitkine (slow), 2011/03/31 14:36:47]

Good catch! I think I got this wrong after refactoring from the earlier version.

Fixed by passing message.size() - pattern_length to substr().

+  std::wstring message((LPWSTR)((uint8*)record + record->StringOffset));
+  if (StartsWith(message, pattern_prefix, false) &&
+      EndsWith(message, pattern_suffix, false)) {
+    std::wstring crash_id =
+        message.substr(pattern_prefix.size(), pattern_length);
+    crashes().push_back(
+        CrashInfo(base::SysWideToUTF8(crash_id),
+                  base::Time::FromDoubleT(record->TimeGenerated)));
+  }
+}