Bug 1262852 - Create a minidump of the plugin process as soon as possible during hang r=jimm

dom/ipc/ProcessHangMonitor.cpp

@@ -162,14 +162,25 @@ public:
     mActor = nullptr;
   }
 
+  /** Sets the information associated with this hang: this includes the ID of
+   * the plugin which caused the hang as well as the content PID.
+   *
+   * @param aHangData The hang information
+   */
   void SetHangData(const HangData& aHangData) { mHangData = aHangData; }
-  void SetBrowserDumpId(nsAutoString& aId) {
+
-    mBrowserDumpId = aId;
+  /** Sets the ID of the crash dump associated with this hang. When the ID has
+   * been set then the corresponding crash dump will be used for reporting
+   * instead of generating a new one.
+   *
+   * @param aId The ID of the crash dump taken when the hang was detected. */
+  void SetDumpId(nsString& aId) {
+    mDumpId = aId;
   }
 
   void ClearHang() {
     mHangData = HangData();
-    mBrowserDumpId.Truncate();
+    mDumpId.Truncate();
   }
 
 private:
@@ -179,7 +190,7 @@ private:
   HangMonitorParent* mActor;
   ContentParent* mContentParent;
   HangData mHangData;
-  nsAutoString mBrowserDumpId;
+  nsAutoString mDumpId;
 };
 
 class HangMonitorParent
@@ -551,8 +562,16 @@ public:
   {
     // chrome process, main thread
     MOZ_RELEASE_ASSERT(NS_IsMainThread());
+
+    nsString dumpId;
+    if (mHangData.type() == HangData::TPluginHangData) {
+      const PluginHangData& phd = mHangData.get_PluginHangData();
+      plugins::TakeFullMinidump(phd.pluginId(), phd.contentProcessId(),
+                                mBrowserDumpId, dumpId);
+    }
+
     mProcess->SetHangData(mHangData);
-    mProcess->SetBrowserDumpId(mBrowserDumpId);
+    mProcess->SetDumpId(dumpId);
 
     nsCOMPtr<nsIObserverService> observerService =
       mozilla::services::GetObserverService();
@@ -861,12 +880,11 @@ HangMonitoredProcess::TerminatePlugin()
     return NS_ERROR_UNEXPECTED;
   }
 
-  // generates a crash report that includes a browser report taken here
+  // Use the multi-process crash report generated earlier.
-  // earlier, the content process, and any plugin process(es).
   uint32_t id = mHangData.get_PluginHangData().pluginId();
   base::ProcessId contentPid = mHangData.get_PluginHangData().contentProcessId();
   plugins::TerminatePlugin(id, contentPid, NS_LITERAL_CSTRING("HangMonitor"),
-                           mBrowserDumpId);
+                           mDumpId);
 
   if (mActor) {
     mActor->CleanupPluginHang(id, false);

dom/plugins/ipc/PluginBridge.h

@@ -26,11 +26,17 @@ FindPluginsForContent(uint32_t aPluginEpoch,
                       nsTArray<PluginTag>* aPlugins,
                       uint32_t* aNewPluginEpoch);
 
+void
+TakeFullMinidump(uint32_t aPluginId,
+                 base::ProcessId aContentProcessId,
+                 const nsAString& aBrowserDumpId,
+                 nsString& aDumpId);
+
 void
 TerminatePlugin(uint32_t aPluginId,
                 base::ProcessId aContentProcessId,
                 const nsCString& aMonitorDescription,
-                const nsAString& aBrowserDumpId);
+                const nsAString& aDumpId);
 
 } // namespace plugins
 } // namespace mozilla

dom/plugins/ipc/PluginModuleParent.cpp

@@ -353,26 +353,50 @@ bool PluginModuleMapping::sIsLoadModuleOnStack = false;
 
 } // namespace
 
-void
+static PluginModuleChromeParent*
-mozilla::plugins::TerminatePlugin(uint32_t aPluginId,
+PluginModuleChromeParentForId(const uint32_t aPluginId)
-                                  base::ProcessId aContentProcessId,
-                                  const nsCString& aMonitorDescription,
-                                  const nsAString& aBrowserDumpId)
 {
   MOZ_ASSERT(XRE_IsParentProcess());
 
   RefPtr<nsPluginHost> host = nsPluginHost::GetInst();
   nsPluginTag* pluginTag = host->PluginWithId(aPluginId);
   if (!pluginTag || !pluginTag->mPlugin) {
-        return;
+    return nullptr;
   }
   RefPtr<nsNPAPIPlugin> plugin = pluginTag->mPlugin;
+
+  return static_cast<PluginModuleChromeParent*>(plugin->GetLibrary());
+}
+
+void
+mozilla::plugins::TakeFullMinidump(uint32_t aPluginId,
+                                   base::ProcessId aContentProcessId,
+                                   const nsAString& aBrowserDumpId,
+                                   nsString& aDumpId)
+{
   PluginModuleChromeParent* chromeParent =
-        static_cast<PluginModuleChromeParent*>(plugin->GetLibrary());
+    PluginModuleChromeParentForId(aPluginId);
+
+  if (chromeParent) {
+    chromeParent->TakeFullMinidump(aContentProcessId, aBrowserDumpId, aDumpId);
+  }
+}
+
+void
+mozilla::plugins::TerminatePlugin(uint32_t aPluginId,
+                                  base::ProcessId aContentProcessId,
+                                  const nsCString& aMonitorDescription,
+                                  const nsAString& aDumpId)
+{
+  PluginModuleChromeParent* chromeParent =
+    PluginModuleChromeParentForId(aPluginId);
+
+  if (chromeParent) {
     chromeParent->TerminateChildProcess(MessageLoop::current(),
                                         aContentProcessId,
                                         aMonitorDescription,
-                                        aBrowserDumpId);
+                                        aDumpId);
+  }
 }
 
 /* static */ PluginLibrary*
@@ -1198,38 +1222,19 @@ PluginModuleContentParent::OnExitedSyncSend()
 }
 
 void
-PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
+PluginModuleChromeParent::TakeFullMinidump(base::ProcessId aContentPid,
-                                                base::ProcessId aContentPid,
+                                           const nsAString& aBrowserDumpId,
-                                                const nsCString& aMonitorDescription,
+                                           nsString& aDumpId)
-                                                const nsAString& aBrowserDumpId)
 {
 #ifdef MOZ_CRASHREPORTER
 #ifdef XP_WIN
     mozilla::MutexAutoLock lock(mCrashReporterMutex);
-    CrashReporterParent* crashReporter = mCrashReporter;
+#endif // XP_WIN
+
+    CrashReporterParent* crashReporter = CrashReporter();
     if (!crashReporter) {
-        // If mCrashReporter is null then the hang has ended, the plugin module
-        // is shutting down. There's nothing to do here.
         return;
     }
-#else
-    CrashReporterParent* crashReporter = CrashReporter();
-#endif
-    crashReporter->AnnotateCrashReport(NS_LITERAL_CSTRING("PluginHang"),
-                                       NS_LITERAL_CSTRING("1"));
-    crashReporter->AnnotateCrashReport(NS_LITERAL_CSTRING("HangMonitorDescription"),
-                                       aMonitorDescription);
-#ifdef XP_WIN
-    if (mHangUIParent) {
-        unsigned int hangUIDuration = mHangUIParent->LastShowDurationMs();
-        if (hangUIDuration) {
-            nsPrintfCString strHangUIDuration("%u", hangUIDuration);
-            crashReporter->AnnotateCrashReport(
-                    NS_LITERAL_CSTRING("PluginHangUIDuration"),
-                    strHangUIDuration);
-        }
-    }
-#endif // XP_WIN
 
     bool reportsReady = false;
 
@@ -1266,6 +1271,7 @@ PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
         // Important to set this here, it tells the ActorDestroy handler
         // that we have an existing crash report that needs to be finalized.
         mPluginDumpID = crashReporter->ChildDumpID();
+        aDumpId = mPluginDumpID;
         PLUGIN_LOG_DEBUG(
                 ("generated paired browser/plugin minidumps: %s)",
                  NS_ConvertUTF16toUTF8(mPluginDumpID).get()));
@@ -1284,7 +1290,7 @@ PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
                                      NS_LITERAL_CSTRING("flash2"))) {
                 additionalDumps.AppendLiteral(",flash2");
             }
-#endif
+#endif // MOZ_CRASHREPORTER_INJECTOR
             if (aContentPid != mozilla::ipc::kInvalidProcessId) {
                 // Include the content process minidump
                 if (CreatePluginMinidump(aContentPid, 0,
@@ -1300,7 +1306,51 @@ PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
     } else {
         NS_WARNING("failed to capture paired minidumps from hang");
     }
-#endif
+#endif // MOZ_CRASHREPORTER
+}
+
+void
+PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
+                                                base::ProcessId aContentPid,
+                                                const nsCString& aMonitorDescription,
+                                                const nsAString& aDumpId)
+{
+#ifdef MOZ_CRASHREPORTER
+    // Start by taking a full minidump if necessary, this is done early
+    // because it also needs to lock the mCrashReporterMutex and Mutex doesn't
+    // support recrusive locking.
+    nsAutoString dumpId;
+    if (aDumpId.IsEmpty()) {
+        TakeFullMinidump(aContentPid, EmptyString(), dumpId);
+    }
+
+#ifdef XP_WIN
+    mozilla::MutexAutoLock lock(mCrashReporterMutex);
+    CrashReporterParent* crashReporter = mCrashReporter;
+    if (!crashReporter) {
+        // If mCrashReporter is null then the hang has ended, the plugin module
+        // is shutting down. There's nothing to do here.
+        return;
+    }
+#else
+    CrashReporterParent* crashReporter = CrashReporter();
+#endif // XP_WIN
+    crashReporter->AnnotateCrashReport(NS_LITERAL_CSTRING("PluginHang"),
+                                       NS_LITERAL_CSTRING("1"));
+    crashReporter->AnnotateCrashReport(NS_LITERAL_CSTRING("HangMonitorDescription"),
+                                       aMonitorDescription);
+#ifdef XP_WIN
+    if (mHangUIParent) {
+        unsigned int hangUIDuration = mHangUIParent->LastShowDurationMs();
+        if (hangUIDuration) {
+            nsPrintfCString strHangUIDuration("%u", hangUIDuration);
+            crashReporter->AnnotateCrashReport(
+                    NS_LITERAL_CSTRING("PluginHangUIDuration"),
+                    strHangUIDuration);
+        }
+    }
+#endif // XP_WIN
+#endif // MOZ_CRASHREPORTER
 
     mozilla::ipc::ScopedProcessHandle geckoChildProcess;
     bool childOpened = base::OpenProcessHandle(OtherPid(),

dom/plugins/ipc/PluginModuleParent.h

@@ -418,10 +418,29 @@ class PluginModuleChromeParent
 
     virtual ~PluginModuleChromeParent();
 
+    /*
+     * Takes a full multi-process dump including the plugin process and the
+     * content process. If aBrowserDumpId is not empty then the browser dump
+     * associated with it will be paired to the resulting minidump.
+     * Takes ownership of the file associated with aBrowserDumpId.
+     *
+     * @param aContentPid PID of the e10s content process from which a hang was
+     *   reported. May be kInvalidProcessId if not applicable.
+     * @param aBrowserDumpId (optional) previously taken browser dump id. If
+     *   provided TakeFullMinidump will use this dump file instead of
+     *   generating a new one. If not provided a browser dump will be taken at
+     *   the time of this call.
+     * @param aDumpId Returns the ID of the newly generated crash dump. Left
+     *   untouched upon failure.
+     */
+    void TakeFullMinidump(base::ProcessId aContentPid,
+                          const nsAString& aBrowserDumpId,
+                          nsString& aDumpId);
+
     /*
      * Terminates the plugin process associated with this plugin module. Also
-     * generates appropriate crash reports. Takes ownership of the file
+     * generates appropriate crash reports unless an existing one is provided.
-     * associated with aBrowserDumpId on success.
+     * Takes ownership of the file associated with aDumpId on success.
      *
      * @param aMsgLoop the main message pump associated with the module
      *   protocol.
@@ -430,15 +449,15 @@ class PluginModuleChromeParent
      * @param aMonitorDescription a string describing the hang monitor that
      *   is making this call. This string is added to the crash reporter
      *   annotations for the plugin process.
-     * @param aBrowserDumpId (optional) previously taken browser dump id. If
+     * @param aDumpId (optional) previously taken dump id. If provided
-     *   provided TerminateChildProcess will use this browser dump file in
+     *   TerminateChildProcess will use this dump file instead of generating a
-     *   generating a multi-process crash report. If not provided a browser
+     *   multi-process crash report. If not provided a multi-process dump will
-     *   dump will be taken at the time of this call.
+     *   be taken at the time of this call.
      */
     void TerminateChildProcess(MessageLoop* aMsgLoop,
                                base::ProcessId aContentPid,
                                const nsCString& aMonitorDescription,
-                               const nsAString& aBrowserDumpId);
+                               const nsAString& aDumpId);
 
 #ifdef XP_WIN
     /**