Bug 1416028 - Prevent browser_pluginCrashedCommentAndURL.js from leaking crash dump files; r=mconley
authorGabriele Svelto <gsvelto@mozilla.com>
Mon, 06 Nov 2017 16:16:13 +0100
changeset 436109 4d80bc4f0161ae83915e7323eefe49ee39224199
parent 436108 a736899d6d3186dacfdd4867e0484ac855486361
child 436110 983664f404549a4aaabab09a12a4cf458f1c6f55
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewersmconley
bugs1416028
milestone59.0a1
Bug 1416028 - Prevent browser_pluginCrashedCommentAndURL.js from leaking crash dump files; r=mconley This patch also checks for I/O errors when deleting leftover crashdump files in the test harness and reports them if the files couldn't be deleted. This won't prevent tests from leaking crash dumps if they're not written correctly, but it will make their presence visible and won't cause the harness to hang if it cannot delete them. MozReview-Commit-ID: FLvBJxEKkH5
browser/base/content/test/plugins/browser_pluginCrashCommentAndURL.js
testing/mochitest/browser-test.js
--- a/browser/base/content/test/plugins/browser_pluginCrashCommentAndURL.js
+++ b/browser/base/content/test/plugins/browser_pluginCrashCommentAndURL.js
@@ -1,8 +1,9 @@
+/* global gBrowser */
 Cu.import("resource://gre/modules/CrashSubmit.jsm", this);
 
 const SERVER_URL = "http://example.com/browser/toolkit/crashreporter/test/browser/crashreport.sjs";
 
 var gTestRoot = getRootDirectory(gTestPath).replace("chrome://mochitests/content/", "http://127.0.0.1:8888/");
 var gTestBrowser = null;
 var config = {};
 
@@ -114,16 +115,51 @@ add_task(async function() {
 
 add_task(async function() {
   config = {
     shouldSubmissionUIBeVisible: false,
     comment: "",
     urlOptIn: true
   };
 
+  // Deferred promise object used by the test to wait for the crash handler
+  let crashDeferred = PromiseUtils.defer();
+
+  // Clear out any minidumps we create from plugin crashes, this is needed
+  // because we do not submit the crash otherwise the submission process would
+  // have deleted the crash dump files for us.
+  let crashObserver = (subject, topic, data) => {
+    if (topic != "plugin-crashed") {
+      return;
+    }
+
+    let propBag = subject.QueryInterface(Ci.nsIPropertyBag2);
+    let minidumpID = propBag.getPropertyAsAString("pluginDumpID");
+
+    Services.crashmanager.ensureCrashIsPresent(minidumpID).then(() => {
+      let dirSvc = Cc["@mozilla.org/file/directory_service;1"]
+                     .getService(Ci.nsIProperties);
+      let minidumpDir = dirSvc.get("UAppData", Ci.nsIFile);
+      minidumpDir.append("Crash Reports");
+      minidumpDir.append("pending");
+
+      let pluginDumpFile = minidumpDir.clone();
+      pluginDumpFile.append(minidumpID + ".dmp");
+
+      let extraFile = minidumpDir.clone();
+      extraFile.append(minidumpID + ".extra");
+
+      pluginDumpFile.remove(false);
+      extraFile.remove(false);
+      crashDeferred.resolve();
+    });
+  };
+
+  Services.obs.addObserver(crashObserver, "plugin-crashed");
+
   setTestPluginEnabledState(Ci.nsIPluginTag.STATE_ENABLED);
 
   let pluginCrashed = promisePluginCrashed();
 
   // Make sure that the plugin container is too small to display the crash submission UI
   await promiseTabLoadEvent(gBrowser.selectedTab, gTestRoot + "plugin_crashCommentAndURL.html?" +
                             encodeURIComponent(JSON.stringify({width: 300, height: 300})));
 
@@ -136,33 +172,38 @@ add_task(async function() {
   // Test that the crash submission UI is not visible and do not submit a crash report.
   await ContentTask.spawn(gTestBrowser, config, async function(aConfig) {
     let doc = content.document;
     let plugin = doc.getElementById("plugin");
     let pleaseSubmit = doc.getAnonymousElementByAttribute(plugin, "anonid", "pleaseSubmit");
     Assert.equal(!!pleaseSubmit && content.getComputedStyle(pleaseSubmit).display == "block",
       aConfig.shouldSubmissionUIBeVisible, "Plugin crash UI should not be visible");
   });
+
+  await crashDeferred.promise;
+  Services.obs.removeObserver(crashObserver, "plugin-crashed");
 });
 
 function promisePluginCrashed() {
   return new ContentTask.spawn(gTestBrowser, {}, async function() {
     await new Promise((resolve) => {
       addEventListener("PluginCrashReporterDisplayed", function onPluginCrashed() {
         removeEventListener("PluginCrashReporterDisplayed", onPluginCrashed);
         resolve();
       });
     });
   });
 }
 
 function onSubmitStatus(aSubject, aData) {
-  // Wait for success or failed, doesn't matter which.
-  if (aData != "success" && aData != "failed")
+  if (aData === "submitting") {
     return false;
+  }
+
+  is(aData, "success", "The crash report should be submitted successfully");
 
   let propBag = aSubject.QueryInterface(Ci.nsIPropertyBag);
   if (aData == "success") {
     let remoteID = getPropertyBagValue(propBag, "serverCrashID");
     ok(!!remoteID, "serverCrashID should be set");
 
     // Remove the submitted report file.
     let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
@@ -171,30 +212,32 @@ function onSubmitStatus(aSubject, aData)
     ok(file.exists(), "Submitted report file should exist");
     file.remove(false);
   }
 
   let extra = getPropertyBagValue(propBag, "extra");
   ok(extra instanceof Ci.nsIPropertyBag, "Extra data should be property bag");
 
   let val = getPropertyBagValue(extra, "PluginUserComment");
-  if (config.comment)
+  if (config.comment) {
     is(val, config.comment,
        "Comment in extra data should match comment in textbox");
-  else
+  } else {
     ok(val === undefined,
        "Comment should be absent from extra data when textbox is empty");
+  }
 
   val = getPropertyBagValue(extra, "PluginContentURL");
-  if (config.urlOptIn)
+  if (config.urlOptIn) {
     is(val, gBrowser.currentURI.spec,
        "URL in extra data should match browser URL when opt-in checked");
-  else
+  } else {
     ok(val === undefined,
        "URL should be absent from extra data when opt-in not checked");
+  }
 
   return true;
 }
 
 function getPropertyBagValue(bag, key) {
   try {
     var val = bag.getProperty(key);
   } catch (e) {
--- a/testing/mochitest/browser-test.js
+++ b/testing/mochitest/browser-test.js
@@ -733,18 +733,23 @@ Tester.prototype = {
         let gdir = Services.dirsvc.get("UAppData", Ci.nsIFile);
         gdir.append("Crash Reports");
         gdir.append("pending");
         if (gdir.exists()) {
           let entries = gdir.directoryEntries;
           while (entries.hasMoreElements()) {
             let entry = entries.getNext().QueryInterface(Ci.nsIFile);
             if (entry.isFile()) {
-              entry.remove(false);
-              let msg = "this test left a pending crash report; deleted " + entry.path;
+              let msg = "this test left a pending crash report; ";
+              try {
+                entry.remove(false);
+                msg += "deleted " + entry.path;
+              } catch (e) {
+                msg += "could not delete " + entry.path;
+              }
               this.structuredLogger.info(msg);
             }
           }
         }
       }
 
       // Notify a long running test problem if it didn't end up in a timeout.
       if (this.currentTest.unexpectedTimeouts && !this.currentTest.timedOut) {