Bug 1360308 - Part 1: De-templatize CrashReporterHost::GenerateMinidumpAndPair(). r=gsvelto
authorCervantes Yu <cyu@mozilla.com>
Thu, 22 Jun 2017 18:52:42 +0800
changeset 365491 fef8b4d5bd1d3abbbd5ed278fb51a49a59575793
parent 365490 295e163e66f882afffe8371a7e99f8c16be9a947
child 365492 2b28e4ee269cb5559f0f28a1421d284cb0c98658
push id32077
push userkwierso@gmail.com
push dateThu, 22 Jun 2017 21:02:06 +0000
treeherdermozilla-central@4c6668cbaccb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgsvelto
bugs1360308
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1360308 - Part 1: De-templatize CrashReporterHost::GenerateMinidumpAndPair(). r=gsvelto Cleans up CrashReporterHost::GenerateMinidumpAndPair() and move it to CrashReporterHost.cpp. Also removes the call to open a privileged process handle and get the already open handle from GeckoChildProcessHost. MozReview-Commit-ID: 8wLm1hf8zrJ
dom/ipc/ContentParent.cpp
dom/plugins/ipc/PluginModuleParent.cpp
ipc/glue/CrashReporterHost.cpp
ipc/glue/CrashReporterHost.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -2972,17 +2972,17 @@ ContentParent::KillHard(const char* aRea
       NS_LITERAL_CSTRING("additional_minidumps"),
       additionalDumps);
     nsDependentCString reason(aReason);
     mCrashReporter->AddNote(
       NS_LITERAL_CSTRING("ipc_channel_error"),
       reason);
 
     // Generate the report and insert into the queue for submittal.
-    if (mCrashReporter->GenerateMinidumpAndPair(this,
+    if (mCrashReporter->GenerateMinidumpAndPair(Process(),
                                                 nullptr,
                                                 NS_LITERAL_CSTRING("browser")))
     {
       mCreatedPairedMinidumps = mCrashReporter->FinalizeCrashReport();
     }
 
     Telemetry::Accumulate(Telemetry::SUBPROCESS_KILL_HARD, reason, 1);
   }
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -1200,33 +1200,33 @@ PluginModuleChromeParent::TakeFullMinidu
     // thread. We do this before we message the main thread about the hang
     // since the posted message will trash our browser stack state.
     nsCOMPtr<nsIFile> browserDumpFile;
     if (CrashReporter::GetMinidumpForID(aBrowserDumpId,
                                         getter_AddRefs(browserDumpFile))) {
         // We have a single browser report, generate a new plugin process parent
         // report and pair it up with the browser report handed in.
         reportsReady = mCrashReporter->GenerateMinidumpAndPair(
-          this,
+          Process(),
           browserDumpFile,
           NS_LITERAL_CSTRING("browser"));
 
         if (!reportsReady) {
           browserDumpFile = nullptr;
           CrashReporter::DeleteMinidumpFilesForID(aBrowserDumpId);
         }
     }
 
     // Generate crash report including plugin and browser process minidumps.
     // The plugin process is the parent report with additional dumps including
     // the browser process, content process when running under e10s, and
     // various flash subprocesses if we're the flash module.
     if (!reportsReady) {
         reportsReady = mCrashReporter->GenerateMinidumpAndPair(
-          this,
+          Process(),
           nullptr, // Pair with a dump of this process and thread.
           NS_LITERAL_CSTRING("browser"));
     }
 
     if (reportsReady) {
         aDumpId = mCrashReporter->MinidumpID();
         PLUGIN_LOG_DEBUG(
                 ("generated paired browser/plugin minidumps: %s)",
--- a/ipc/glue/CrashReporterHost.cpp
+++ b/ipc/glue/CrashReporterHost.cpp
@@ -1,29 +1,31 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CrashReporterHost.h"
 #include "CrashReporterMetadataShmem.h"
+#include "mozilla/ipc/GeckoChildProcessHost.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/SyncRunnable.h"
 #include "mozilla/Telemetry.h"
 #ifdef MOZ_CRASHREPORTER
+# include "nsExceptionHandler.h"
 # include "nsICrashService.h"
 #endif
 
 namespace mozilla {
 namespace ipc {
 
 CrashReporterHost::CrashReporterHost(GeckoProcessType aProcessType,
                                      const Shmem& aShmem,
-                                     CrashReporter::ThreadId aThreadId)
+                                     ThreadId aThreadId)
  : mProcessType(aProcessType),
    mShmem(aShmem),
    mThreadId(aThreadId),
    mStartTime(::time(nullptr)),
    mFinalized(false)
 {
 }
 
@@ -98,16 +100,44 @@ CrashReporterHost::FinalizeCrashReport()
   // Use mExtraNotes, since NotifyCrashService looks for "PluginHang" which is
   // set in the parent process.
   NotifyCrashService(mProcessType, mDumpID, &mExtraNotes);
 
   mFinalized = true;
   return true;
 }
 
+bool
+CrashReporterHost::GenerateMinidumpAndPair(GeckoChildProcessHost* aChildProcess,
+                                           nsIFile* aMinidumpToPair,
+                                           const nsACString& aPairName)
+{
+  base::ProcessHandle childHandle;
+#ifdef XP_MACOSX
+  childHandle = aChildProcess->GetChildTask();
+#else
+  childHandle = aChildProcess->GetChildProcessHandle();
+  if (!childHandle) {
+    NS_WARNING("Failed to get child process handle.");
+    return false;
+  }
+#endif
+
+  nsCOMPtr<nsIFile> targetDump;
+  if (!CrashReporter::CreateMinidumpsAndPair(childHandle,
+                                             mThreadId,
+                                             aPairName,
+                                             aMinidumpToPair,
+                                             getter_AddRefs(targetDump))) {
+    return false;
+  }
+
+  return CrashReporter::GetIDFromMinidump(targetDump, mDumpID);
+}
+
 /* static */ void
 CrashReporterHost::NotifyCrashService(GeckoProcessType aProcessType,
                                       const nsString& aChildDumpID,
                                       const AnnotationTable* aNotes)
 {
   if (!NS_IsMainThread()) {
     RefPtr<Runnable> runnable = NS_NewRunnableFunction([=] () -> void {
       CrashReporterHost::NotifyCrashService(aProcessType, aChildDumpID, aNotes);
--- a/ipc/glue/CrashReporterHost.h
+++ b/ipc/glue/CrashReporterHost.h
@@ -5,35 +5,45 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_ipc_CrashReporterHost_h
 #define mozilla_ipc_CrashReporterHost_h
 
 #include "mozilla/UniquePtr.h"
 #include "mozilla/ipc/Shmem.h"
 #include "base/process.h"
+#ifdef MOZ_CRASHREPORTER
 #include "nsExceptionHandler.h"
+#endif
 
 namespace mozilla {
 namespace ipc {
 
+class GeckoChildProcessHost;
+
 // This is the newer replacement for CrashReporterParent. It is created in
 // response to a InitCrashReporter message on a top-level actor, and simply
 // holds the metadata shmem alive until the process ends. When the process
 // terminates abnormally, the top-level should call GenerateCrashReport to
 // automatically integrate metadata.
 class CrashReporterHost
 {
   typedef mozilla::ipc::Shmem Shmem;
+#ifdef MOZ_CRASHREPORTER
   typedef CrashReporter::AnnotationTable AnnotationTable;
+  typedef CrashReporter::ThreadId ThreadId;
+#else
+  // unused in this case
+  typedef int32_t ThreadId;
+#endif
 
 public:
   CrashReporterHost(GeckoProcessType aProcessType,
                     const Shmem& aShmem,
-                    CrashReporter::ThreadId aThreadId);
+                    ThreadId aThreadId);
 
 #ifdef MOZ_CRASHREPORTER
   // Helper function for generating a crash report for a process that probably
   // crashed (i.e., had an AbnormalShutdown in ActorDestroy). Returns true if
   // the process has a minidump attached and we were able to generate a report.
   bool GenerateCrashReport(base::ProcessId aPid);
 
   // Given an existing minidump for a crashed child process, take ownership of
@@ -48,45 +58,20 @@ public:
   // finalizes the existing report by attaching metadata and notifying the
   // crash service.
   bool FinalizeCrashReport();
 
   // Generate a paired minidump. This does not take the crash report, as
   // GenerateCrashReport does. After this, FinalizeCrashReport may be called.
   //
   // This calls TakeCrashedChildMinidump and FinalizeCrashReport.
-  template <typename Toplevel>
-  bool GenerateMinidumpAndPair(Toplevel* aToplevelProtocol,
-                               nsIFile* aMinidumpToPair,
-                               const nsACString& aPairName)
-  {
-    ScopedProcessHandle childHandle;
-#ifdef XP_MACOSX
-    childHandle = aToplevelProtocol->Process()->GetChildTask();
-#else
-    if (!base::OpenPrivilegedProcessHandle(aToplevelProtocol->OtherPid(),
-                                           &childHandle.rwget()))
-    {
-      NS_WARNING("Failed to open child process handle.");
-      return false;
-    }
-#endif
-
-    nsCOMPtr<nsIFile> targetDump;
-    if (!CrashReporter::CreateMinidumpsAndPair(childHandle,
-                                               mThreadId,
-                                               aPairName,
-                                               aMinidumpToPair,
-                                               getter_AddRefs(targetDump)))
-    {
-      return false;
-    }
-
-    return CrashReporter::GetIDFromMinidump(targetDump, mDumpID);
-  }
+  bool
+  GenerateMinidumpAndPair(GeckoChildProcessHost* aChildProcess,
+                          nsIFile* aMinidumpToPair,
+                          const nsACString& aPairName);
 
   // This is a static helper function to notify the crash service that a
   // crash has occurred. When PCrashReporter is removed, we can make this
   // a member function. This can be called from any thread, and if not
   // called from the main thread, will post a synchronous message to the
   // main thread.
   static void NotifyCrashService(
     GeckoProcessType aProcessType,
@@ -106,19 +91,21 @@ public:
 
 private:
   static void AsyncAddCrash(int32_t aProcessType, int32_t aCrashType,
                             const nsString& aChildDumpID);
 
 private:
   GeckoProcessType mProcessType;
   Shmem mShmem;
-  CrashReporter::ThreadId mThreadId;
+  ThreadId mThreadId;
   time_t mStartTime;
+#ifdef MOZ_CRASHREPORTER
   AnnotationTable mExtraNotes;
+#endif
   nsString mDumpID;
   bool mFinalized;
 };
 
 } // namespace ipc
 } // namespace mozilla
 
 #endif // mozilla_ipc_CrashReporterHost_h