Bug 1262852 - Create a minidump upon each plugin hang r=jimm

dom/ipc/CrashReporterParent.cpp

@@ -110,6 +110,12 @@ CrashReporterParent::GenerateCrashReportForMinidump(nsIFile* minidump,
   return result;
 }
 
+bool
+CrashReporterParent::UseMinidump(nsIFile* aMinidump)
+{
+  return CrashReporter::GetIDFromMinidump(aMinidump, mChildDumpID);
+}
+
 bool
 CrashReporterParent::GenerateChildData(const AnnotationTable* processNotes)
 {

dom/ipc/CrashReporterParent.h

@@ -68,6 +68,15 @@ public:
   GenerateMinidumpAndPair(Toplevel* aTopLevel, nsIFile* aMinidump,
                           const nsACString& aPairName);
 
+  /**
+   * Uses the specified minidump instead of taking a new one.
+   *
+   * @param aMinidump - the minidump to use for this crashreport.
+   * @returns true if successful, false otherwise.
+   */
+  bool
+  UseMinidump(nsIFile* aMinidump);
+
   /**
    * Apply child process annotations to an existing paired mindump generated
    * with GeneratePairedMinidump.

dom/ipc/PProcessHangMonitor.ipdl

@@ -21,6 +21,7 @@ struct PluginHangData
 {
   uint32_t pluginId;
   ProcessId contentProcessId;
+  ProcessId pluginProcessId;
 };
 
 union HangData

dom/ipc/ProcessHangMonitor.cpp

@@ -87,7 +87,7 @@ class HangMonitorChild
   bool IsDebuggerStartupComplete();
 
   void NotifyPluginHang(uint32_t aPluginId);
-  void NotifyPluginHangAsync(uint32_t aPluginId);
+  void NotifyPluginHangAsync(uint32_t aPluginId, ProcessId aPid);
 
   void ClearHang();
   void ClearHangAsync();
@@ -209,6 +209,8 @@ public:
 
  private:
   void ShutdownOnThread();
+  void GenerateMinidumps(uint32_t aPluginId, ProcessId aPluginPid,
+                         ProcessId aContentPid, nsString& aCrashId);
 
   const RefPtr<ProcessHangMonitor> mHangMonitor;
 
@@ -403,21 +405,24 @@ HangMonitorChild::NotifyPluginHang(uint32_t aPluginId)
 
   mSentReport = true;
 
+  base::ProcessId pluginPid = plugins::PluginProcessId(aPluginId);
   // bounce to background thread
-  MonitorLoop()->PostTask(NewNonOwningRunnableMethod<uint32_t>(this,
-                                                               &HangMonitorChild::NotifyPluginHangAsync,
-                                                               aPluginId));
+  MonitorLoop()->PostTask(
+    NewNonOwningRunnableMethod<uint32_t, uint32_t>(this,
+                                                   &HangMonitorChild::NotifyPluginHangAsync,
+                                                   aPluginId, pluginPid));
 }
 
 void
-HangMonitorChild::NotifyPluginHangAsync(uint32_t aPluginId)
+HangMonitorChild::NotifyPluginHangAsync(uint32_t aPluginId,
+                                        base::ProcessId aPid)
 {
   MOZ_RELEASE_ASSERT(MessageLoop::current() == MonitorLoop());
 
   // bounce back to parent on background thread
   if (mIPCOpen) {
     Unused << SendHangEvidence(PluginHangData(aPluginId,
-                                              base::GetCurrentProcId()));
+                                              base::GetCurrentProcId(), aPid));
   }
 }
 
@@ -566,6 +571,54 @@ private:
   nsAutoString mBrowserDumpId;
 };
 
+void
+HangMonitorParent::GenerateMinidumps(uint32_t aPluginId, ProcessId aPluginPid,
+                                     ProcessId aContentPid, nsString& aCrashId)
+{
+#ifdef MOZ_CRASHREPORTER
+  if (mBrowserCrashDumpIds.Get(aPluginId, &aCrashId)) {
+    return; // We already have a dump for this hang
+  }
+
+  nsCOMPtr<nsIFile> browserDump;
+  if (!CrashReporter::TakeMinidump(getter_AddRefs(browserDump), true)) {
+    NS_WARNING("Failed to generate a minidump for the browser process");
+    return;
+  }
+
+  nsCOMPtr<nsIFile> pluginDump;
+  mozilla::ipc::ScopedProcessHandle pluginHandle;
+  if (!base::OpenPrivilegedProcessHandle(aPluginPid, &pluginHandle.rwget()) ||
+      !CrashReporter::CreateMinidumpsAndPair(pluginHandle, 0,
+                                             NS_LITERAL_CSTRING("browser"),
+                                             browserDump,
+                                             getter_AddRefs(pluginDump))) {
+    browserDump->Remove(false);
+    NS_WARNING("Failed to generate a minidump for the plugin process");
+    return;
+  }
+
+  if (!CrashReporter::GetIDFromMinidump(pluginDump, aCrashId) ||
+      aCrashId.IsEmpty()) {
+    pluginDump->Remove(false);
+    return;
+  }
+
+  mBrowserCrashDumpIds.Put(aPluginId, aCrashId);
+
+  mozilla::ipc::ScopedProcessHandle contentHandle;
+  if (!base::OpenPrivilegedProcessHandle(aContentPid, &contentHandle.rwget()) ||
+      !CrashReporter::CreateAdditionalChildMinidump(contentHandle,
+                                                    0, pluginDump,
+                                                    NS_LITERAL_CSTRING("content"))) {
+    NS_WARNING("Failed to generate a minidump for the content process");
+    return;
+  }
+
+  return;
+#endif
+}
+
 bool
 HangMonitorParent::RecvHangEvidence(const HangData& aHangData)
 {
@@ -591,17 +644,8 @@ HangMonitorParent::RecvHangEvidence(const HangData& aHangData)
   if (aHangData.type() == HangData::TPluginHangData) {
     MutexAutoLock lock(mBrowserCrashDumpHashLock);
     const PluginHangData& phd = aHangData.get_PluginHangData();
-    if (!mBrowserCrashDumpIds.Get(phd.pluginId(), &crashId)) {
-      nsCOMPtr<nsIFile> browserDump;
-      if (CrashReporter::TakeMinidump(getter_AddRefs(browserDump), true)) {
-        if (!CrashReporter::GetIDFromMinidump(browserDump, crashId) || crashId.IsEmpty()) {
-          browserDump->Remove(false);
-          NS_WARNING("Failed to generate timely browser stack, this is bad for plugin hang analysis!");
-        } else {
-          mBrowserCrashDumpIds.Put(phd.pluginId(), crashId);
-        }
-      }
-    }
+    GenerateMinidumps(phd.pluginId(), phd.pluginProcessId(),
+                           phd.contentProcessId(), crashId);
   }
 #endif
 

dom/plugins/ipc/PluginBridge.h

@@ -26,6 +26,9 @@ FindPluginsForContent(uint32_t aPluginEpoch,
                       nsTArray<PluginTag>* aPlugins,
                       uint32_t* aNewPluginEpoch);
 
+base::ProcessId
+PluginProcessId(uint32_t aPluginId);
+
 void
 TerminatePlugin(uint32_t aPluginId,
                 base::ProcessId aContentProcessId,

dom/plugins/ipc/PluginModuleParent.cpp

@@ -353,6 +353,29 @@ bool PluginModuleMapping::sIsLoadModuleOnStack = false;
 
 } // namespace
 
+base::ProcessId
+mozilla::plugins::PluginProcessId(uint32_t aPluginId)
+{
+  RefPtr<nsPluginHost> host = nsPluginHost::GetInst();
+  if (!host) {
+    return mozilla::ipc::kInvalidProcessId;
+  }
+
+  nsPluginTag* pluginTag = host->PluginWithId(aPluginId);
+  if (!pluginTag || !pluginTag->mPlugin) {
+      return mozilla::ipc::kInvalidProcessId;
+  }
+
+  RefPtr<nsNPAPIPlugin> plugin = pluginTag->mPlugin;
+  PluginModuleChromeParent* chromeParent =
+      static_cast<PluginModuleChromeParent*>(plugin->GetLibrary());
+  if (!chromeParent) {
+    return mozilla::ipc::kInvalidProcessId;
+  }
+
+  return chromeParent->OtherPid();
+}
+
 void
 mozilla::plugins::TerminatePlugin(uint32_t aPluginId,
                                   base::ProcessId aContentProcessId,
@@ -1235,8 +1258,7 @@ PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
 
     // Check to see if we already have a browser dump id - with e10s plugin
     // hangs we take this earlier (see ProcessHangMonitor) from a background
-    // thread. We do this before we message the main thread about the hang
-    // since the posted message will trash our browser stack state.
+    // thread. It includes a content and plugin dump too.
     bool exists;
     nsCOMPtr<nsIFile> browserDumpFile;
     if (!aBrowserDumpId.IsEmpty() &&
@@ -1244,14 +1266,8 @@ PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
         browserDumpFile &&
         NS_SUCCEEDED(browserDumpFile->Exists(&exists)) && exists)
     {
-        // We have a single browser report, generate a new plugin process parent
-        // report and pair it up with the browser report handed in.
-        reportsReady = crashReporter->GenerateMinidumpAndPair(this, browserDumpFile,
-                                                              NS_LITERAL_CSTRING("browser"));
-        if (!reportsReady) {
-          browserDumpFile = nullptr;
-          CrashReporter::DeleteMinidumpFilesForID(aBrowserDumpId);
-        }
+        crashReporter->UseMinidump(browserDumpFile);
+        reportsReady = true;
     }
 
     // Generate crash report including plugin and browser process minidumps.
@@ -1286,8 +1302,10 @@ PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
             }
 #endif
             if (aContentPid != mozilla::ipc::kInvalidProcessId) {
-                // Include the content process minidump
-                if (CreatePluginMinidump(aContentPid, 0,
+                // Include the content process minidump only if we don't have
+                // it already.
+                if (exists ||
+                    CreatePluginMinidump(aContentPid, 0,
                                          pluginDumpFile,
                                          NS_LITERAL_CSTRING("content"))) {
                     additionalDumps.AppendLiteral(",content");