* Breakpad on OSX now works with stock Breakpad framwork.

* We now add all the same Metadata on OS X as we do on Windows.
* Made the code for logging URLs in crash dumps a little more x-platform.
Remove custom Breakpad hacks so we can use an unmodified version of the Framework.

Review URL: http://codereview.chromium.org/55028

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

8 files changed, 262 insertions, 28 deletions

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 446a263..faf8d7c 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -36,6 +36,7 @@
 #include "chrome/renderer/localized_error.h"
 #include "chrome/renderer/media/audio_renderer_impl.h"
 #include "chrome/renderer/render_process.h"
+#include "chrome/renderer/renderer_logging.h"
 #include "chrome/renderer/user_script_slave.h"
 #include "chrome/renderer/visitedlink_slave.h"
 #include "chrome/renderer/webmediaplayer_delegate_impl.h"
@@ -77,18 +78,10 @@
 #include "base/gfx/gdi_util.h"
 #include "base/gfx/native_theme.h"
 #include "chrome/common/gfx/emf.h"
-#include "chrome/renderer/renderer_logging.h"
+#include "chrome/views/controls/message_box_view.h"
 #include "skia/ext/vector_canvas.h"
 #endif
 
-#if defined(OS_WIN)
-// If true, the URL of the active renderer is logged. Logging is done in such
-// way that if the renderer crashes the URL of the active renderer is contained
-// in the dump. Currently mini-dumps are only supported on windows, so this is
-// only enabled on windows.
-#define LOG_RENDERER_URL
-#endif
-
 using base::TimeDelta;
 using webkit_glue::WebAccessibility;
 
@@ -335,11 +328,9 @@ void RenderView::Init(gfx::NativeViewId parent_hwnd,
 }
 
 void RenderView::OnMessageReceived(const IPC::Message& message) {
-#ifdef LOG_RENDERER_URL
   WebFrame* main_frame = webview() ? webview()->GetMainFrame() : NULL;
   renderer_logging::ScopedActiveRenderingURLSetter url_setter(
       main_frame ? main_frame->GetURL() : GURL());
-#endif
 
   // If this is developer tools renderer intercept tools messages first.
   if (devtools_client_.get() && devtools_client_->OnMessageReceived(message))
@@ -773,9 +764,7 @@ void RenderView::OnNavigate(const ViewMsg_Navigate_Params& params) {
   if (!webview())
     return;
 
-#ifdef LOG_RENDERER_URL
   renderer_logging::ScopedActiveRenderingURLSetter url_setter(params.url);
-#endif
 
   AboutHandler::MaybeHandle(params.url);
 
diff --git a/chrome/renderer/renderer.scons b/chrome/renderer/renderer.scons
index 25d309f..a620ad6 100644
--- a/chrome/renderer/renderer.scons
+++ b/chrome/renderer/renderer.scons
@@ -123,6 +123,7 @@ if env.Bit('linux'):
   # Linux-specific.
   input_files.Append(
       'renderer_main_platform_delegate_linux.cc',
+      'renderer_logging_linux.cc',
   )
 
   # TODO(port): Port these to Linux
diff --git a/chrome/renderer/renderer.vcproj b/chrome/renderer/renderer.vcproj
index e9e6a52..38df719 100644
--- a/chrome/renderer/renderer.vcproj
+++ b/chrome/renderer/renderer.vcproj
@@ -342,11 +342,11 @@
 			>
 		</File>
 		<File
-			RelativePath=".\renderer_logging.cc"
+			RelativePath=".\renderer_logging.h"
 			>
 		</File>
 		<File
-			RelativePath=".\renderer_logging.h"
+			RelativePath=".\renderer_logging_win.cc"
 			>
 		</File>
 		<File
diff --git a/chrome/renderer/renderer_logging.h b/chrome/renderer/renderer_logging.h
index 6be45de..d7c91a5 100644
--- a/chrome/renderer/renderer_logging.h
+++ b/chrome/renderer/renderer_logging.h
@@ -6,8 +6,7 @@
 #define CHROME_RENDERER_RENDERER_LOGGING_H_
 
 #include "base/basictypes.h"
-
-class GURL;
+#include "googleurl/src/gurl.h"
 
 namespace renderer_logging {
 
@@ -19,8 +18,13 @@ void SetActiveRendererURL(const GURL& url);
 // and clears the active rendering URL in the destructor.
 class ScopedActiveRenderingURLSetter {
  public:
-  explicit ScopedActiveRenderingURLSetter(const GURL& url);
-  ~ScopedActiveRenderingURLSetter();
+  explicit ScopedActiveRenderingURLSetter(const GURL& url)  {
+    SetActiveRendererURL(url);
+  }
+
+  ~ScopedActiveRenderingURLSetter()  {
+    SetActiveRendererURL(GURL());
+  }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ScopedActiveRenderingURLSetter);
@@ -28,4 +32,24 @@ class ScopedActiveRenderingURLSetter {
 
 }  // namespace renderer_logging
 
+#if defined(OS_MACOSX) && __OBJC__
+// Exported for testing purposes.
+
+@class NSString;
+
+typedef void (*SetCrashKeyValueFuncPtr)(NSString*, NSString*);
+typedef void (*ClearCrashKeyValueFuncPtr)(NSString*);
+
+namespace renderer_logging {
+void SetActiveRendererURLImpl(const GURL& url,
+                              SetCrashKeyValueFuncPtr set_key_func,
+                              ClearCrashKeyValueFuncPtr clear_key_func);
+
+extern const int kMaxNumCrashURLChunks;
+extern const int kMaxNumURLChunkValueLength;
+extern const char *kUrlChunkFormatStr;
+}  // namespace renderer_logging
+
+#endif  // defined(OS_MACOSX) && __OBJC__
+
 #endif  // CHROME_RENDERER_RENDERER_LOGGING_H_
diff --git a/chrome/renderer/renderer_logging_linux.cc b/chrome/renderer/renderer_logging_linux.cc
new file mode 100644
index 0000000..a703137
--- /dev/null
+++ b/chrome/renderer/renderer_logging_linux.cc
@@ -0,0 +1,19 @@
+// Copyright (c) 2009 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/renderer/renderer_logging.h"
+
+#include "base/logging.h"
+#include "googleurl/src/gurl.h"
+
+namespace renderer_logging {
+
+// Sets the URL that is logged if the renderer crashes. Use GURL() to clear
+// the URL.
+void SetActiveRendererURL(const GURL& url) {
+  // TODO(port): Once breakpad is integrated we can turn this on.
+  NOTIMPLEMENTED();
+}
+
+}  // namespace renderer_logging
diff --git a/chrome/renderer/renderer_logging_mac.mm b/chrome/renderer/renderer_logging_mac.mm
new file mode 100644
index 0000000..d108a85
--- /dev/null
+++ b/chrome/renderer/renderer_logging_mac.mm
@@ -0,0 +1,71 @@
+// Copyright (c) 2009 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/renderer/renderer_logging.h"
+
+#import <Foundation/Foundation.h>
+
+#include "base/string_util.h"
+#include "googleurl/src/gurl.h"
+#import "chrome/app/breakpad_mac.h"
+
+namespace renderer_logging {
+
+const int kMaxNumCrashURLChunks = 8;
+const int kMaxNumURLChunkValueLength = 255;
+const char *kUrlChunkFormatStr = "url-chunk-%d";
+
+void SetActiveRendererURLImpl(const GURL& url,
+                              SetCrashKeyValueFuncPtr set_key_func,
+                              ClearCrashKeyValueFuncPtr clear_key_func) {
+
+  NSString *kUrlChunkFormatStr_utf8 = [NSString
+      stringWithUTF8String:kUrlChunkFormatStr];
+
+  // First remove any old url chunks we might have lying around.
+  for (int i = 0; i < kMaxNumCrashURLChunks; i++) {
+    // On Windows the url-chunk items are 1-based, so match that.
+    NSString *key = [NSString stringWithFormat:kUrlChunkFormatStr_utf8, i+1];
+    clear_key_func(key);
+  }
+
+  const std::string& raw_url_utf8 = url.possibly_invalid_spec();
+  NSString *raw_url = [NSString stringWithUTF8String:raw_url_utf8.c_str()];
+  size_t raw_url_length = [raw_url length];
+
+  // Bail on zero-length URLs.
+  if (raw_url_length == 0) {
+    return;
+  }
+
+  // Parcel the URL up into up to 8, 255 byte segments.
+  size_t start_ofs = 0;
+  for (int i = 0;
+       i < kMaxNumCrashURLChunks && start_ofs < raw_url_length;
+       ++i) {
+
+    // On Windows the url-chunk items are 1-based, so match that.
+    NSString *key = [NSString stringWithFormat:kUrlChunkFormatStr_utf8, i+1];
+    NSRange range;
+    range.location = start_ofs;
+    range.length = std::min((size_t)kMaxNumURLChunkValueLength,
+                            raw_url_length - start_ofs);
+    NSString *value = [raw_url substringWithRange:range];
+    set_key_func(key, value);
+
+    // Next chunk.
+    start_ofs += kMaxNumURLChunkValueLength;
+  }
+}
+
+void SetActiveRendererURL(const GURL& url) {
+  // If Breakpad isn't initialized then bail.
+  if (IsCrashReporterEnabled()) {
+    return;
+  }
+
+  SetActiveRendererURLImpl(url, SetCrashKeyValue, ClearCrashKeyValue);
+}
+
+}  // namespace renderer_logging
diff --git a/chrome/renderer/renderer_logging_mac_unittest.mm b/chrome/renderer/renderer_logging_mac_unittest.mm
new file mode 100644
index 0000000..6aece64
--- /dev/null
+++ b/chrome/renderer/renderer_logging_mac_unittest.mm
@@ -0,0 +1,138 @@
+// Copyright (c) 2009 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/renderer/renderer_logging.h"
+
+#import <Foundation/Foundation.h>
+
+#include "base/logging.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+typedef PlatformTest RendererLoggingTest;
+
+namespace {
+
+// Class to mock breakpad's setkeyvalue/clearkeyvalue functions needed for
+// SetActiveRendererURLImpl.
+// The Keys are stored in a static dictionary and methods are provided to
+// verify correctness.
+class MockBreakpadKeyValueStore {
+ public:
+  MockBreakpadKeyValueStore() {
+    // Only one of these objects can be active at once.
+    DCHECK(dict == NULL);
+    dict = [[NSMutableDictionary alloc] init];
+  }
+
+  ~MockBreakpadKeyValueStore() {
+    // Only one of these objects can be active at once.
+    DCHECK(dict != NULL);
+    [dict release];
+    dict = NULL;
+  }
+
+  static void SetKeyValue(NSString* key, NSString* value) {
+    DCHECK(dict != NULL);
+    [dict setObject:value forKey:key];
+  }
+
+  static void ClearKeyValue(NSString *key) {
+    DCHECK(dict != NULL);
+    [dict removeObjectForKey:key];
+  }
+
+  int CountDictionaryEntries() {
+    return [dict count];
+  }
+
+  bool VerifyDictionaryContents(const std::string &url) {
+    using renderer_logging::kMaxNumCrashURLChunks;
+    using renderer_logging::kMaxNumURLChunkValueLength;
+    using renderer_logging::kUrlChunkFormatStr;
+
+    int num_url_chunks = CountDictionaryEntries();
+    EXPECT_TRUE(num_url_chunks <= kMaxNumCrashURLChunks);
+
+    NSString *kUrlChunkFormatStr_utf8 = [NSString
+        stringWithUTF8String:kUrlChunkFormatStr];
+
+    NSString *accumulated_url = @"";
+    for (int i = 0; i < num_url_chunks; ++i) {
+      // URL chunk names are 1-based.
+      NSString *key = [NSString stringWithFormat:kUrlChunkFormatStr_utf8, i+1];
+      EXPECT_TRUE(key != NULL);
+      NSString *value = [dict objectForKey:key];
+      EXPECT_TRUE([value length] > 0);
+      EXPECT_TRUE([value length] <= (unsigned)kMaxNumURLChunkValueLength);
+      accumulated_url = [accumulated_url stringByAppendingString:value];
+    }
+
+    NSString *expected_url = [NSString stringWithUTF8String:url.c_str()];
+    return([accumulated_url isEqualToString:expected_url]);
+  }
+
+ private:
+  static NSMutableDictionary* dict;
+  DISALLOW_COPY_AND_ASSIGN(MockBreakpadKeyValueStore);
+};
+
+}  // namespace
+
+// Call through to SetActiveRendererURLImpl using the functions from
+// MockBreakpadKeyValueStore.
+void SetActiveRendererURLWithMock(const GURL& url) {
+  using renderer_logging::SetActiveRendererURLImpl;
+
+  SetCrashKeyValueFuncPtr setFunc = MockBreakpadKeyValueStore::SetKeyValue;
+  ClearCrashKeyValueFuncPtr clearFunc =
+      MockBreakpadKeyValueStore::ClearKeyValue;
+
+  SetActiveRendererURLImpl(url, setFunc, clearFunc);
+}
+
+TEST_F(RendererLoggingTest, TestUrlSplitting) {
+  using renderer_logging::kMaxNumCrashURLChunks;
+  using renderer_logging::kMaxNumURLChunkValueLength;
+
+  const std::string short_url("http://abc/");
+  std::string long_url("http://");
+  std::string overflow_url("http://");
+
+  long_url += std::string(kMaxNumURLChunkValueLength * 2, 'a');
+  long_url += "/";
+
+  int max_num_chars_stored_in_dump = kMaxNumURLChunkValueLength *
+      kMaxNumCrashURLChunks;
+  overflow_url += std::string(max_num_chars_stored_in_dump + 1, 'a');
+  overflow_url += "/";
+
+  // Check that Clearing NULL URL works.
+  MockBreakpadKeyValueStore mock;
+  SetActiveRendererURLWithMock(GURL());
+  EXPECT_EQ(mock.CountDictionaryEntries(), 0);
+
+  // Check that we can set a URL.
+  SetActiveRendererURLWithMock(GURL(short_url.c_str()));
+  EXPECT_TRUE(mock.VerifyDictionaryContents(short_url));
+  EXPECT_EQ(mock.CountDictionaryEntries(), 1);
+  SetActiveRendererURLWithMock(GURL());
+  EXPECT_EQ(mock.CountDictionaryEntries(), 0);
+
+  // Check that we can replace a long url with a short url.
+  SetActiveRendererURLWithMock(GURL(long_url.c_str()));
+  EXPECT_TRUE(mock.VerifyDictionaryContents(long_url));
+  SetActiveRendererURLWithMock(GURL(short_url.c_str()));
+  EXPECT_TRUE(mock.VerifyDictionaryContents(short_url));
+  SetActiveRendererURLWithMock(GURL());
+  EXPECT_EQ(mock.CountDictionaryEntries(), 0);
+
+
+  // Check that overflow works correctly.
+  SetActiveRendererURLWithMock(GURL(overflow_url.c_str()));
+  EXPECT_TRUE(mock.VerifyDictionaryContents(
+      overflow_url.substr(0, max_num_chars_stored_in_dump)));
+  SetActiveRendererURLWithMock(GURL());
+  EXPECT_EQ(mock.CountDictionaryEntries(), 0);
+}
diff --git a/chrome/renderer/renderer_logging.cc b/chrome/renderer/renderer_logging_win.cc
index 2c708bf..6d1b758 100644
--- a/chrome/renderer/renderer_logging.cc
+++ b/chrome/renderer/renderer_logging_win.cc
@@ -30,12 +30,4 @@ void SetActiveRendererURL(const GURL& url) {
   (set_active_renderer_url)(UTF8ToWide(url.possibly_invalid_spec()).c_str());
 }
 
-ScopedActiveRenderingURLSetter::ScopedActiveRenderingURLSetter(const GURL& url) {
-  SetActiveRendererURL(url);
-}
-
-ScopedActiveRenderingURLSetter::~ScopedActiveRenderingURLSetter() {
-  SetActiveRendererURL(GURL());
-}
-
-}
+}  // namespace renderer_logging