Bug 1416028 - Prevent browser_pluginCrashedCommentAndURL.js from leaking crash dump files. r=mconley, a=test-only
authorGabriele Svelto <gsvelto@mozilla.com>
Mon, 06 Nov 2017 16:16:13 +0100
changeset 444865 b1aa00643e62f7d96a0d22415637d6dd96b0dc43
parent 444864 eb4996243c5e9a10346995acc3ce982b9158bbe5
child 444866 a41f982ed3a7e8995031fa2cd15d2ab7380a4010
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmconley, test-only
bugs1416028
milestone58.0
Bug 1416028 - Prevent browser_pluginCrashedCommentAndURL.js from leaking crash dump files. r=mconley, a=test-only 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) {