Bug 1416028 - Prevent browser_pluginCrashedCommentAndURL.js from leaking crash dump files; r?mconley draft
authorGabriele Svelto <gsvelto@mozilla.com>
Mon, 06 Nov 2017 16:16:13 +0100
changeset 696748 8a6ae3b98f3ca9986af44ef679318261eca9c81c
parent 696686 933f9cd9b3b9030399a11c19cb4e5117b29e2772
child 739916 e747a0243ede0c2124f0fb41833480ddfb9a14ae
push id88779
push usergsvelto@mozilla.com
push dateSat, 11 Nov 2017 07:23:53 +0000
reviewersmconley
bugs1416028
milestone58.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) {