[Mac] Log stack trace for CHECK in bug 97285.

The hypothesis in comment #55 on the bug is that while setting up a
new channel to an already-connected plugin,
NPChannelBase::RemoveRoute() can be called on the earlier channel, so
the renderer and plugin channel mappings get out of sync.  This will
add a Breakpad key encoding the top stack frames when RemoveRoute() is
called within the scoped of WebPluginDelegateProxy::Initialize().

objc_zombies.mm already implemented the stackframe-encoding code, so I
pulled that to a central location.  That code can be left in place
when the other code is removed after some traces are generated.

BUG=97285
TEST=crash server.


Review URL: https://chromiumcodereview.appspot.com/10408004

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

6 files changed, 102 insertions, 16 deletions

diff --git a/base/mac/crash_logging.h b/base/mac/crash_logging.h
index ea28ce5..7e83eae 100644
--- a/base/mac/crash_logging.h
+++ b/base/mac/crash_logging.h
@@ -32,6 +32,13 @@ BASE_EXPORT void SetCrashKeyFunctions(SetCrashKeyValueFuncPtr set_key_func,
 BASE_EXPORT void SetCrashKeyValue(NSString* key, NSString* val);
 BASE_EXPORT void ClearCrashKey(NSString* key);
 
+// Format |count| items from |addresses| using %p, and set the
+// resulting string as value for crash key |key|.  A maximum of 23
+// items will be encoded, since breakpad limits values to 255 bytes.
+BASE_EXPORT void SetCrashKeyFromAddresses(NSString* key,
+                                          const void* const* addresses,
+                                          size_t count);
+
 #if __OBJC__
 
 class BASE_EXPORT ScopedCrashKey {
diff --git a/base/mac/crash_logging.mm b/base/mac/crash_logging.mm
index ce0db39..e18a3fb 100644
--- a/base/mac/crash_logging.mm
+++ b/base/mac/crash_logging.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -6,6 +6,8 @@
 
 #import <Foundation/Foundation.h>
 
+#include "base/logging.h"
+
 namespace base {
 namespace mac {
 
@@ -28,6 +30,30 @@ void ClearCrashKey(NSString* key) {
     g_clear_key_func(key);
 }
 
+void SetCrashKeyFromAddresses(NSString* key,
+                              const void* const* addresses,
+                              size_t count) {
+  NSString* value = @"<null>";
+  if (addresses && count) {
+    const size_t kBreakpadValueMax = 255;
+
+    NSMutableArray* hexBacktrace = [NSMutableArray arrayWithCapacity:count];
+    size_t length = 0;
+    for (size_t i = 0; i < count; ++i) {
+      NSString* s = [NSString stringWithFormat:@"%p", addresses[i]];
+      length += 1 + [s length];
+      if (length > kBreakpadValueMax)
+        break;
+      [hexBacktrace addObject:s];
+    }
+    value = [hexBacktrace componentsJoinedByString:@" "];
+
+    // Warn someone if this exceeds the breakpad limits.
+    DCHECK_LE(strlen([value UTF8String]), kBreakpadValueMax);
+  }
+  base::mac::SetCrashKeyValue(key, value);
+}
+
 ScopedCrashKey::ScopedCrashKey(NSString* key, NSString* value)
     : crash_key_([key retain]) {
   if (g_set_key_func)
diff --git a/chrome/common/mac/objc_zombie.mm b/chrome/common/mac/objc_zombie.mm
index e5d3a82..5ef00a6 100644
--- a/chrome/common/mac/objc_zombie.mm
+++ b/chrome/common/mac/objc_zombie.mm
@@ -290,21 +290,11 @@ void ZombieObjectCrash(id object, SEL aSelector, SEL viaSelector) {
   // Set a value for breakpad to report.
   base::mac::SetCrashKeyValue(@"zombie", aString);
 
-  // Hex-encode the backtrace and tuck it into a breakpad key.
-  NSString* deallocTrace = @"<unknown>";
-  if (found && record.traceDepth) {
-    NSMutableArray* hexBacktrace =
-        [NSMutableArray arrayWithCapacity:record.traceDepth];
-    for (size_t i = 0; i < record.traceDepth; ++i) {
-      NSString* s = [NSString stringWithFormat:@"%p", record.trace[i]];
-      [hexBacktrace addObject:s];
-    }
-    deallocTrace = [hexBacktrace componentsJoinedByString:@" "];
-
-    // Warn someone if this exceeds the breakpad limits.
-    DCHECK_LE(strlen([deallocTrace UTF8String]), 255U);
+  // Encode trace into a breakpad key.
+  if (found) {
+    base::mac::SetCrashKeyFromAddresses(
+        @"zombie_dealloc_bt", record.trace, record.traceDepth);
   }
-  base::mac::SetCrashKeyValue(@"zombie_dealloc_bt", deallocTrace);
 
   // Log -dealloc backtrace in debug builds then crash with a useful
   // stack trace.
diff --git a/content/renderer/plugin_channel_host.cc b/content/renderer/plugin_channel_host.cc
index f9f384e..52c9632 100644
--- a/content/renderer/plugin_channel_host.cc
+++ b/content/renderer/plugin_channel_host.cc
@@ -16,6 +16,35 @@
 
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebBindings.h"
 
+// TODO(shess): Debugging for http://crbug.com/97285
+//
+// The hypothesis at #55 requires that RemoveRoute() be called between
+// sending ViewHostMsg_OpenChannelToPlugin to the browser, and calling
+// GetPluginChannelHost() on the result.  This code detects that case
+// and stores the backtrace of the RemoveRoute() in a breakpad key.
+// The specific RemoveRoute() is not tracked (there could be multiple,
+// and which is the one can't be known until the open completes), but
+// the backtrace from any such nested call should be sufficient to
+// drive a repro.
+#if defined(OS_MACOSX)
+#include "base/debug/stack_trace.h"
+#include "base/mac/crash_logging.h"
+#include "base/sys_string_conversions.h"
+
+namespace {
+
+// Breakpad key for the RemoveRoute() backtrace.
+const char* kRemoveRouteTraceKey = "remove_route_bt";
+
+// GetRemoveTrackingFlag() exposes this so that
+// WebPluginDelegateProxy::Initialize() can do scoped set/reset.  When
+// true, RemoveRoute() knows WBDP::Initialize() is on the stack, and
+// records the backtrace.
+bool remove_tracking = false;
+
+}  // namespace
+#endif
+
 // A simple MessageFilter that will ignore all messages and respond to sync
 // messages with an error when is_listening_ is false.
 class IsListeningFilter : public IPC::ChannelProxy::MessageFilter {
@@ -73,6 +102,14 @@ void PluginChannelHost::SetListening(bool flag) {
   IsListeningFilter::is_listening_ = flag;
 }
 
+#if defined(OS_MACOSX)
+// static
+bool* PluginChannelHost::GetRemoveTrackingFlag() {
+  return &remove_tracking;
+}
+#endif
+
+// static
 PluginChannelHost* PluginChannelHost::GetPluginChannelHost(
     const IPC::ChannelHandle& channel_handle,
     base::MessageLoopProxy* ipc_message_loop) {
@@ -120,6 +157,16 @@ void PluginChannelHost::AddRoute(int route_id,
 }
 
 void PluginChannelHost::RemoveRoute(int route_id) {
+#if defined(OS_MACOSX)
+  if (remove_tracking) {
+    base::debug::StackTrace trace;
+    size_t count = 0;
+    const void* const* addresses = trace.Addresses(&count);
+    base::mac::SetCrashKeyFromAddresses(
+        base::SysUTF8ToNSString(kRemoveRouteTraceKey), addresses, count);
+  }
+#endif
+
   proxies_.erase(route_id);
   NPChannelBase::RemoveRoute(route_id);
 }
diff --git a/content/renderer/plugin_channel_host.h b/content/renderer/plugin_channel_host.h
index 486098d..92001b9 100644
--- a/content/renderer/plugin_channel_host.h
+++ b/content/renderer/plugin_channel_host.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -17,6 +17,11 @@ class NPObjectBase;
 // On the plugin side there's a corresponding PluginChannel.
 class PluginChannelHost : public NPChannelBase {
  public:
+#if defined(OS_MACOSX)
+  // TODO(shess): Debugging for http://crbug.com/97285 .  See comment
+  // in plugin_channel_host.cc.
+  static bool* GetRemoveTrackingFlag();
+#endif
   static PluginChannelHost* GetPluginChannelHost(
       const IPC::ChannelHandle& channel_handle,
       base::MessageLoopProxy* ipc_message_loop);
diff --git a/content/renderer/webplugin_delegate_proxy.cc b/content/renderer/webplugin_delegate_proxy.cc
index edc8550..2bf8dbd 100644
--- a/content/renderer/webplugin_delegate_proxy.cc
+++ b/content/renderer/webplugin_delegate_proxy.cc
@@ -12,6 +12,7 @@
 
 #include <algorithm>
 
+#include "base/auto_reset.h"
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
@@ -276,6 +277,13 @@ bool WebPluginDelegateProxy::Initialize(
     const std::vector<std::string>& arg_values,
     webkit::npapi::WebPlugin* plugin,
     bool load_manually) {
+#if defined(OS_MACOSX)
+  // TODO(shess): Debugging for http://crbug.com/97285 .  See comment
+  // in plugin_channel_host.cc.
+  scoped_ptr<AutoReset<bool> > track_nested_removes(new AutoReset<bool>(
+      PluginChannelHost::GetRemoveTrackingFlag(), true));
+#endif
+
   IPC::ChannelHandle channel_handle;
   if (!RenderThreadImpl::current()->Send(new ViewHostMsg_OpenChannelToPlugin(
           render_view_->routing_id(), url, page_url_, mime_type_,
@@ -306,6 +314,9 @@ bool WebPluginDelegateProxy::Initialize(
     LOG(ERROR) << "Couldn't get PluginChannelHost";
     return false;
   }
+#if defined(OS_MACOSX)
+  track_nested_removes.reset();
+#endif
 
   int instance_id;
   bool result = channel_host->Send(new PluginMsg_CreateInstance(