Update Breakpad to r842, picking up:

------------------------------------------------------------------------
r842 | mark@chromium.org | 2011-10-03 15:54:28 -0400 (Mon, 03 Oct 2011) | 11 lines

Use a bootstrap subset port for the inspector, tying the subset to the
lifetime of the task to be monitored, the invoking task. This allows the
bootstrap server (in launchd) to automatically clean up the Mach server
registration when the task being monitored exits, avoiding leaks of
com.Breakpad.Inspector(pid) ports in "launchctl bslist".

BUG=chromium:28547
TEST=Handler should still crash catches, but inspector ports should no longer
     show up in "launchctl bslist". They should show up under a subset port in
     "launchctl bstree" instead. "launchctl bstree" must be invoked as root.
Review URL: http://breakpad.appspot.com/306001
------------------------------------------------------------------------

This gives each Breakpad instance its own bootstrap subset port as a subset
of the process' bootstrap port, and places the Breakpad inspector port into
the bootstrap subset port.

This completely eliminates leaks of on-demand Mach services advertised via the
bootstrap server.

This also reverts r34318 and r34534, "temporary" (21-month) hacks to mitigate
the leak. The temporary hacks were never completely effective against Breakpad
ports leaking from child processes.

DestructCrashReporter at process shutdown was deemed unnecessary and is being
removed. As this was the last caller to that function, the implementation is
removed as well.

This is addressed in Breakpad rather than Chrome because a mechanism to fix it
without changing the process' bootstrap port was discovered. It was determined
that the rohitfork port is already immune to leaks of the sort that the
Breakpad inspector port experiences. A previous attempt to fix this bug was
r103089, but that caused bug 98550. This strategy is safer.

The forked Breakpad module.cc is updated to match upstream r835, which allows
the unforking of the CFI-disabling code. A new CFI-disabling "-c" option to
dump_syms is now available and used by the dump_product_syms script.

BUG=28547
TEST=1. "launchctl bslist" should no longer show on-demand
        com.Breakpad.Inspector ports (in Breakpad-enabled builds with Breakpad
        on) ports.
     2. "launchctl bstree" (as root) should reveal a bootstrap subset for each
        process as a child of the per-user/per-session bootstrap namespace if
        crash reporting is on. One com.Breakpad.Inspector port should show up
        as a child of each bootstrap subset. As each process exits (even if
        mercilessly killed by "kill -9"), the associated subsets and inspector
        ports should disappear.
     3. Breakpad reports should be generated on crashes. For example,
        about:crash and about:inducebrowsercrashforrealz should each cause a
        minidump to be written in
        ~/Library/Application Support/Google/Chrome/Crash Reports
        when Breakpad is enabled. This tests that the Breakpad ports are
        functioning properly.
     4. The browser process should be able to access child process data.
        Window:Task Manager should show valid values for the Memory, CPU, and
        Network columns for all child processes. This tests that the rohitfork
        port is functioning properly. Note that this version of the change
        does not change the handling of rohitfork ports at all, but this test
        case was useful in a previous version of this patch.
     5. Test case from bug 98550: Have a link in a web browser window (in a
        different browser) and drag it into Chrome. Expect the drag operation
        to operate properly in both the tab strip and in web content. See bug
        98550 for details.
     6. Unreported test case that failed in r103089: browser relaunch should
        work. Visit chrome://flags, change some flags to get the "Relaunch Now"
        button at the bottom (you can put the flags back to how you found them
        initially), and click "Relaunch Now". The browser should close and then
        be relaunched.
Review URL: http://codereview.chromium.org/8120007

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

6 files changed, 5 insertions, 160 deletions

diff --git a/chrome/app/breakpad_mac.h b/chrome/app/breakpad_mac.h
index b737f0d..8ff1d26 100644
--- a/chrome/app/breakpad_mac.h
+++ b/chrome/app/breakpad_mac.h
@@ -19,7 +19,4 @@ void InitCrashProcessInfo();
 // Is Breakpad enabled?
 bool IsCrashReporterEnabled();
 
-// Call on clean process shutdown.
-void DestructCrashReporter();
-
 #endif  // CHROME_APP_BREAKPAD_MAC_H_
diff --git a/chrome/app/breakpad_mac.mm b/chrome/app/breakpad_mac.mm
index 4f4d11c..f078271 100644
--- a/chrome/app/breakpad_mac.mm
+++ b/chrome/app/breakpad_mac.mm
@@ -56,13 +56,6 @@ bool IsCrashReporterEnabled() {
   return gBreakpadRef != NULL;
 }
 
-void DestructCrashReporter() {
-  if (gBreakpadRef) {
-    BreakpadRelease(gBreakpadRef);
-    gBreakpadRef = NULL;
-  }
-}
-
 // Only called for a branded build of Chrome.app.
 void InitCrashReporter() {
   DCHECK(!gBreakpadRef);
diff --git a/chrome/app/breakpad_mac_stubs.mm b/chrome/app/breakpad_mac_stubs.mm
index db2db6b..f8a651a 100644
--- a/chrome/app/breakpad_mac_stubs.mm
+++ b/chrome/app/breakpad_mac_stubs.mm
@@ -16,8 +16,5 @@ bool IsCrashReporterEnabled() {
 void InitCrashProcessInfo() {
 }
 
-void DestructCrashReporter() {
-}
-
 void InitCrashReporter() {
 }
diff --git a/chrome/app/chrome_main.cc b/chrome/app/chrome_main.cc
index 4d7fbe4..a24e1b3 100644
--- a/chrome/app/chrome_main.cc
+++ b/chrome/app/chrome_main.cc
@@ -673,11 +673,6 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
       ResourceBundle::CleanupSharedInstance();
 
     logging::CleanupChromeLogging();
-
-#if defined(OS_MACOSX) && defined(GOOGLE_CHROME_BUILD)
-    // TODO(mark): See the TODO(mark) at InitCrashReporter.
-    DestructCrashReporter();
-#endif  // OS_MACOSX && GOOGLE_CHROME_BUILD
   }
 
 #if defined(OS_MACOSX)
diff --git a/chrome/renderer/chrome_render_process_observer.cc b/chrome/renderer/chrome_render_process_observer.cc
index 8288b73..ad41476 100644
--- a/chrome/renderer/chrome_render_process_observer.cc
+++ b/chrome/renderer/chrome_render_process_observer.cc
@@ -47,11 +47,6 @@
 #include "base/win/iat_patch_function.h"
 #endif
 
-#if defined(OS_MACOSX)
-#include "base/eintr_wrapper.h"
-#include "chrome/app/breakpad_mac.h"
-#endif
-
 using WebKit::WebCache;
 using WebKit::WebCrossOriginPreflightResultCache;
 using WebKit::WebFontCache;
@@ -190,121 +185,11 @@ class SuicideOnChannelErrorFilter : public IPC::ChannelProxy::MessageFilter {
     // So, we install a filter on the channel so that we can process this event
     // here and kill the process.
 
-#if defined(OS_MACOSX)
-    // TODO(viettrungluu): crbug.com/28547: The following is needed, as a
-    // stopgap, to avoid leaking due to not releasing Breakpad properly.
-    // TODO(viettrungluu): Investigate why this is being called.
-    if (IsCrashReporterEnabled()) {
-      VLOG(1) << "Cleaning up Breakpad.";
-      DestructCrashReporter();
-    } else {
-      VLOG(1) << "Breakpad not enabled; no clean-up needed.";
-    }
-#endif  // OS_MACOSX
-
     _exit(0);
   }
 };
 #endif  // OS_POSIX
 
-#if defined(OS_MACOSX)
-// TODO(viettrungluu): crbug.com/28547: The following signal handling is needed,
-// as a stopgap, to avoid leaking due to not releasing Breakpad properly.
-// Without this problem, this could all be eliminated. Remove when Breakpad is
-// fixed?
-// TODO(viettrungluu): Code taken from browser_main.cc (with a bit of editing).
-// The code should be properly shared (or this code should be eliminated).
-int g_shutdown_pipe_write_fd = -1;
-
-void SIGTERMHandler(int signal) {
-  RAW_CHECK(signal == SIGTERM);
-
-  // Reinstall the default handler.  We had one shot at graceful shutdown.
-  struct sigaction action;
-  memset(&action, 0, sizeof(action));
-  action.sa_handler = SIG_DFL;
-  CHECK(sigaction(signal, &action, NULL) == 0);
-
-  RAW_CHECK(g_shutdown_pipe_write_fd != -1);
-  size_t bytes_written = 0;
-  do {
-    int rv = HANDLE_EINTR(
-        write(g_shutdown_pipe_write_fd,
-              reinterpret_cast<const char*>(&signal) + bytes_written,
-              sizeof(signal) - bytes_written));
-    RAW_CHECK(rv >= 0);
-    bytes_written += rv;
-  } while (bytes_written < sizeof(signal));
-}
-
-class ShutdownDetector : public base::PlatformThread::Delegate {
- public:
-  explicit ShutdownDetector(int shutdown_fd) : shutdown_fd_(shutdown_fd) {
-    CHECK(shutdown_fd_ != -1);
-  }
-
-  virtual void ThreadMain() {
-    int signal;
-    size_t bytes_read = 0;
-    ssize_t ret;
-    do {
-      ret = HANDLE_EINTR(
-          read(shutdown_fd_,
-               reinterpret_cast<char*>(&signal) + bytes_read,
-               sizeof(signal) - bytes_read));
-      if (ret < 0) {
-        NOTREACHED() << "Unexpected error: " << strerror(errno);
-        break;
-      } else if (ret == 0) {
-        NOTREACHED() << "Unexpected closure of shutdown pipe.";
-        break;
-      }
-      bytes_read += ret;
-    } while (bytes_read < sizeof(signal));
-
-    if (bytes_read == sizeof(signal))
-      VLOG(1) << "Handling shutdown for signal " << signal << ".";
-    else
-      VLOG(1) << "Handling shutdown for unknown signal.";
-
-    // Clean up Breakpad if necessary.
-    if (IsCrashReporterEnabled()) {
-      VLOG(1) << "Cleaning up Breakpad.";
-      DestructCrashReporter();
-    } else {
-      VLOG(1) << "Breakpad not enabled; no clean-up needed.";
-    }
-
-    // Something went seriously wrong, so get out.
-    if (bytes_read != sizeof(signal)) {
-      LOG(WARNING) << "Failed to get signal. Quitting ungracefully.";
-      _exit(1);
-    }
-
-    // Re-raise the signal.
-    kill(getpid(), signal);
-
-    // The signal may be handled on another thread. Give that a chance to
-    // happen.
-    sleep(3);
-
-    // We really should be dead by now.  For whatever reason, we're not. Exit
-    // immediately, with the exit status set to the signal number with bit 8
-    // set.  On the systems that we care about, this exit status is what is
-    // normally used to indicate an exit by this signal's default handler.
-    // This mechanism isn't a de jure standard, but even in the worst case, it
-    // should at least result in an immediate exit.
-    LOG(WARNING) << "Still here, exiting really ungracefully.";
-    _exit(signal | (1 << 7));
-  }
-
- private:
-  const int shutdown_fd_;
-
-  DISALLOW_COPY_AND_ASSIGN(ShutdownDetector);
-};
-#endif  // OS_MACOSX
-
 }  // namespace
 
 bool ChromeRenderProcessObserver::is_incognito_process_ = false;
@@ -330,31 +215,6 @@ ChromeRenderProcessObserver::ChromeRenderProcessObserver(
   thread->AddFilter(new SuicideOnChannelErrorFilter());
 #endif
 
-#if defined(OS_MACOSX)
-  // TODO(viettrungluu): Code taken from browser_main.cc.
-  int pipefd[2];
-  int ret = pipe(pipefd);
-  if (ret < 0) {
-    PLOG(DFATAL) << "Failed to create pipe";
-  } else {
-    int shutdown_pipe_read_fd = pipefd[0];
-    g_shutdown_pipe_write_fd = pipefd[1];
-    const size_t kShutdownDetectorThreadStackSize = 4096;
-    if (!base::PlatformThread::CreateNonJoinable(
-            kShutdownDetectorThreadStackSize,
-            new ShutdownDetector(shutdown_pipe_read_fd))) {
-      LOG(DFATAL) << "Failed to create shutdown detector task.";
-    }
-  }
-
-  // crbug.com/28547: When Breakpad is in use, handle SIGTERM to avoid leaking
-  // Mach ports.
-  struct sigaction action;
-  memset(&action, 0, sizeof(action));
-  action.sa_handler = SIGTERMHandler;
-  CHECK(sigaction(SIGTERM, &action, NULL) == 0);
-#endif
-
   // Configure modules that need access to resources.
   net::NetModule::SetResourceProvider(chrome_common_net::NetResourceProvider);
 
diff --git a/chrome/tools/build/mac/dump_product_syms b/chrome/tools/build/mac/dump_product_syms
index 0ad1447..c46ffac 100755
--- a/chrome/tools/build/mac/dump_product_syms
+++ b/chrome/tools/build/mac/dump_product_syms
@@ -105,9 +105,12 @@ for SRC_NAME in "${SRC_NAMES[@]}"; do
   BPAD_SYM_NAME="${SRC_NAME}-${FULL_VERSION}-${ARCH}.breakpad"
   BPAD_SYM_PATH="${BUILT_PRODUCTS_DIR}/${BPAD_SYM_NAME}"
 
-  # Only run dump_syms if the file has changed since the last dump.
+  # Only run dump_syms if the file has changed since the last dump. Use -c to
+  # avoid dumping CFI, because the Breakpad stackwalk is incompatible with CFI
+  # produced by clang.
+  # http://code.google.com/p/google-breakpad/issues/detail?id=443
   if [ "${DWARF_PATH}" -nt "${BPAD_SYM_PATH}" ] ; then
-    "${BREAKPAD_DUMP_SYMS}" -a "${ARCH}" "${DWARF_PATH}" > "${BPAD_SYM_PATH}"
+    "${BREAKPAD_DUMP_SYMS}" -a "${ARCH}" -c "${DWARF_PATH}" > "${BPAD_SYM_PATH}"
   fi
 
   # Some executables will show up with variant names. The Breakpad symbol