Bug 1345153 - When the pingsender fails to send a ping, persist it to disk so that it can be sent later via regular telemetry. r=chutten, r=ted, a=gchang
authorGabriele Svelto <gsvelto@mozilla.com>
Mon, 27 Mar 2017 12:38:39 +0200
changeset 393360 b3b878775ada1356980114ab985c5a5fe2e2d914
parent 393359 eb0d361dc73c32867d5fd54dc47733994bd48e7a
child 393361 16ea72b646fe6a9b089c353231283e939c4bca21
push id7198
push userjlorenzo@mozilla.com
push dateTue, 18 Apr 2017 12:07:49 +0000
treeherdermozilla-beta@d57aa49c3948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerschutten, ted, gchang
bugs1345153
milestone54.0a2
Bug 1345153 - When the pingsender fails to send a ping, persist it to disk so that it can be sent later via regular telemetry. r=chutten, r=ted, a=gchang Currently we hand over a crash ping to the pingsender via a pipe; if the pingsender fails to send the ping we rely on the CrashManager assembling and sending one instead. Since the crashmanager is not aware of whether the ping was sent or not this causes duplication on the server side. To solve this problem we save the ping to disk instead, read it from the pingsender and delete the file only if the ping was sent. In this scenario the CrashManager will know that a ping was already sent and will not send a new one. This patch removes all the code used to deal with pipes between the telemetry, crashreporter and pingsender code and also tries to cut down the amount of platform-specific code we have in this machinery. MozReview-Commit-ID: ASm2jnDagCK
toolkit/components/crashes/CrashManager.jsm
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/telemetry/TelemetryController.jsm
toolkit/components/telemetry/docs/internals/pingsender.rst
toolkit/components/telemetry/nsITelemetry.idl
toolkit/components/telemetry/pingsender/pingsender.cpp
toolkit/components/telemetry/tests/unit/test_PingSender.js
toolkit/components/telemetry/tests/unit/test_TelemetryController.js
toolkit/crashreporter/client/crashreporter.cpp
toolkit/crashreporter/client/crashreporter.h
toolkit/crashreporter/client/crashreporter_unix_common.cpp
toolkit/crashreporter/client/crashreporter_win.cpp
toolkit/crashreporter/client/ping.cpp
toolkit/crashreporter/nsExceptionHandler.cpp
--- a/toolkit/components/crashes/CrashManager.jsm
+++ b/toolkit/components/crashes/CrashManager.jsm
@@ -635,19 +635,16 @@ this.CrashManager.prototype = Object.fre
     // If we have a saved environment, use it. Otherwise report
     // the current environment.
     let reportMeta = Cu.cloneInto(metadata, myScope);
     let crashEnvironment = parseAndRemoveField(reportMeta,
                                                "TelemetryEnvironment");
     let sessionId = parseAndRemoveField(reportMeta, "TelemetrySessionId",
                                         /* parseAsJson */ false);
     let stackTraces = parseAndRemoveField(reportMeta, "StackTraces");
-    // If CrashPingUUID is not present then Telemetry will generate a new UUID
-    let pingId = parseAndRemoveField(reportMeta, "CrashPingUUID",
-                                     /* parseAsJson */ false);
 
     // Filter the remaining annotations to remove privacy-sensitive ones
     reportMeta = this._filterAnnotations(reportMeta);
 
     this._pingPromise = TelemetryController.submitExternalPing("crash",
       {
         version: 1,
         crashDate: date.toISOString().slice(0, 10), // YYYY-MM-DD
@@ -657,17 +654,16 @@ this.CrashManager.prototype = Object.fre
         stackTraces,
         metadata: reportMeta,
         hasCrashEnvironment: (crashEnvironment !== null),
       },
       {
         addClientId: true,
         addEnvironment: true,
         overrideEnvironment: crashEnvironment,
-        overridePingId: pingId
       }
     );
   },
 
   _handleEventFilePayload(store, entry, type, date, payload) {
       // The payload types and formats are documented in docs/crash-events.rst.
       // Do not change the format of an existing type. Instead, invent a new
       // type.
@@ -683,17 +679,24 @@ this.CrashManager.prototype = Object.fre
           }
           // fall-through
         case "crash.main.2":
           let crashID = lines[0];
           let metadata = parseKeyValuePairsFromLines(lines.slice(1));
           store.addCrash(this.PROCESS_TYPE_MAIN, this.CRASH_TYPE_CRASH,
                          crashID, date, metadata);
 
-          this._sendCrashPing(crashID, this.PROCESS_TYPE_MAIN, date, metadata);
+          if (!("CrashPingUUID" in metadata)) {
+            // If CrashPingUUID is not present then a ping was not generated
+            // by the crashreporter for this crash so we need to send one from
+            // here.
+            this._sendCrashPing(crashID, this.PROCESS_TYPE_MAIN, date,
+                                metadata);
+          }
+
           break;
 
         case "crash.submission.1":
           if (lines.length == 3) {
             let [crashID, result, remoteID] = lines;
             store.addCrash(this.PROCESS_TYPE_MAIN, this.CRASH_TYPE_CRASH,
                            crashID, date);
 
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -13,17 +13,16 @@
 
 #include "mozilla/dom/ToJSValue.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MathAlgorithms.h"
-#include "mozilla/Scoped.h"
 #include "mozilla/Unused.h"
 
 #include "base/pickle.h"
 #include "nsIComponentManager.h"
 #include "nsIServiceManager.h"
 #include "nsThreadManager.h"
 #include "nsXPCOMCIDInternal.h"
 #include "nsCOMArray.h"
@@ -75,31 +74,25 @@
 #include "mozilla/Preferences.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/IOInterposer.h"
 #include "mozilla/PoisonIOInterposer.h"
 #include "mozilla/StartupTimeline.h"
 #include "mozilla/HangMonitor.h"
 #include "nsNativeCharsetUtils.h"
 #include "nsProxyRelease.h"
+#include "nsDirectoryServiceDefs.h"
 
 #if defined(MOZ_GECKO_PROFILER)
 #include "shared-libraries.h"
 #define ENABLE_STACK_CAPTURE
 #include "mozilla/StackWalk.h"
 #include "nsPrintfCString.h"
 #endif // MOZ_GECKO_PROFILER
 
-namespace mozilla {
-  // Scoped auto-close for PRFileDesc file descriptors
-  MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE(ScopedPRFileDesc,
-                                            PRFileDesc,
-                                            PR_Close);
-}
-
 namespace {
 
 using namespace mozilla;
 using namespace mozilla::HangMonitor;
 using Telemetry::Common::AutoHashtable;
 using mozilla::dom::Promise;
 using mozilla::dom::AutoJSAPI;
 
@@ -2856,95 +2849,79 @@ TelemetryImpl::FlushBatchedChildTelemetr
   return NS_OK;
 }
 
 #ifndef MOZ_WIDGET_ANDROID
 
 static nsresult
 LocatePingSender(nsAString& aPath)
 {
-  nsCOMPtr<nsIFile> xreAppDistDir;
-  nsresult rv = NS_GetSpecialDirectory(XRE_APP_DISTRIBUTION_DIR,
-                                       getter_AddRefs(xreAppDistDir));
+  nsCOMPtr<nsIFile> xreGreBinDir;
+  nsresult rv = NS_GetSpecialDirectory(NS_GRE_BIN_DIR,
+                                       getter_AddRefs(xreGreBinDir));
 
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return NS_ERROR_FAILURE;
   }
 
-  xreAppDistDir->AppendNative(NS_LITERAL_CSTRING("pingsender" BIN_SUFFIX));
-  xreAppDistDir->GetPath(aPath);
+  // Make sure that the executable exists. Otherwise, quit.
+  bool exists = false;
+  if (NS_FAILED(xreGreBinDir->Exists(&exists)) || !exists) {
+    return NS_ERROR_FAILURE;
+  }
+
+  xreGreBinDir->AppendNative(NS_LITERAL_CSTRING("pingsender" BIN_SUFFIX));
+  xreGreBinDir->GetPath(aPath);
   return NS_OK;
 }
 
 #endif // MOZ_WIDGET_ANDROID
 
 NS_IMETHODIMP
-TelemetryImpl::RunPingSender(const nsACString& aUrl, const nsACString& aPing)
+TelemetryImpl::RunPingSender(const nsACString& aUrl,
+                             const nsACString& aPingFilePath)
 {
 #ifdef MOZ_WIDGET_ANDROID
   Unused << aUrl;
-  Unused << aPing;
+  Unused << aPingFilePath;
 
   return NS_ERROR_NOT_IMPLEMENTED;
 #else // Windows, Mac, Linux, etc...
   // Obtain the path of the pingsender executable
   nsAutoString path;
   nsresult rv = LocatePingSender(path);
 
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return NS_ERROR_FAILURE;
   }
 
-  // Create a pipe to send the ping contents to the ping sender
-  ScopedPRFileDesc pipeRead;
-  ScopedPRFileDesc pipeWrite;
-
-  if (PR_CreatePipe(&pipeRead.rwget(), &pipeWrite.rwget()) != PR_SUCCESS) {
-    return NS_ERROR_FAILURE;
-  }
-
-  if ((PR_SetFDInheritable(pipeRead, PR_TRUE) != PR_SUCCESS) ||
-      (PR_SetFDInheritable(pipeWrite, PR_FALSE) != PR_SUCCESS)) {
-    return NS_ERROR_FAILURE;
-  }
-
   PRProcessAttr* attr = PR_NewProcessAttr();
   if (!attr) {
     return NS_ERROR_FAILURE;
   }
 
-  // Connect the pingsender standard input to the pipe and launch it
-  PR_ProcessAttrSetStdioRedirect(attr, PR_StandardInput, pipeRead);
-
-  UniquePtr<char[]> arg0(ToNewCString(path));
-  UniquePtr<char[]> arg1(ToNewCString(aUrl));
-
+  // We pretend we're redirecting stdout to force NSPR not to show a
+  // command prompt when launching the program.
+  PR_ProcessAttrSetStdioRedirect(attr, PR_StandardOutput, PR_STDOUT);
+
+  UniquePtr<char[]> asciiPath(ToNewCString(aPingFilePath));
+  UniquePtr<char[]> pingSenderPath(ToNewCString(path));
+  UniquePtr<char[]> serverURL(ToNewCString(aUrl));
+
+  // The next lines are needed as |args| is not const.
   char* args[] = {
-    arg0.get(),
-    arg1.get(),
+    pingSenderPath.get(),
+    serverURL.get(),
+    asciiPath.get(),
     nullptr,
   };
+
   Unused << NS_WARN_IF(PR_CreateProcessDetached(args[0], args, nullptr, attr));
   PR_DestroyProcessAttr(attr);
 
-  // Send the ping contents to the ping sender
-  size_t length = aPing.Length();
-  const char* s = aPing.BeginReading();
-
-  while (length > 0) {
-    int result = PR_Write(pipeWrite, s, length);
-
-    if (result <= 0) {
-      return NS_ERROR_FAILURE;
-    }
-
-    s += result;
-    length -= result;
-  }
-
   return NS_OK;
 #endif
 }
 
 size_t
 TelemetryImpl::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf)
 {
   size_t n = aMallocSizeOf(this);
--- a/toolkit/components/telemetry/TelemetryController.jsm
+++ b/toolkit/components/telemetry/TelemetryController.jsm
@@ -194,18 +194,16 @@ this.TelemetryController = Object.freeze
    * @param {String} aType The type of the ping.
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overridePingId=null] set to override the
-   *                 generated ping id.
    * @returns {Promise} Test-only - a promise that resolves with the ping id once the ping is stored or sent.
    */
   submitExternalPing(aType, aPayload, aOptions = {}) {
     aOptions.addClientId = aOptions.addClientId || false;
     aOptions.addEnvironment = aOptions.addEnvironment || false;
 
     return Impl.submitExternalPing(aType, aPayload, aOptions);
   },
@@ -228,18 +226,16 @@ this.TelemetryController = Object.freeze
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Boolean} [aOptions.overwrite=false] true overwrites a ping with the same name,
    *                  if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overridePingId=null] set to override the
-   *                 generated ping id.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
    */
   addPendingPing(aType, aPayload, aOptions = {}) {
     let options = aOptions;
     options.addClientId = aOptions.addClientId || false;
     options.addEnvironment = aOptions.addEnvironment || false;
@@ -287,18 +283,16 @@ this.TelemetryController = Object.freeze
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Boolean} [aOptions.overwrite=false] true overwrites a ping with the same name,
    *                  if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overridePingId=null] set to override the
-   *                 generated ping id.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
    */
   savePing(aType, aPayload, aFilePath, aOptions = {}) {
     let options = aOptions;
     options.addClientId = aOptions.addClientId || false;
     options.addEnvironment = aOptions.addEnvironment || false;
@@ -398,33 +392,31 @@ var Impl = {
    * @param {String} aType The type of the ping.
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} aOptions Options object.
    * @param {Boolean} aOptions.addClientId true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} aOptions.addEnvironment true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overridePingId=null] set to override the
-   *                 generated ping id.
    *
    * @returns {Object} An object that contains the assembled ping data.
    */
   assemblePing: function assemblePing(aType, aPayload, aOptions = {}) {
     this._log.trace("assemblePing - Type " + aType + ", aOptions " + JSON.stringify(aOptions));
 
     // Clone the payload data so we don't race against unexpected changes in subobjects that are
     // still referenced by other code.
     // We can't trust all callers to do this properly on their own.
     let payload = Cu.cloneInto(aPayload, myScope);
 
     // Fill the common ping fields.
     let pingData = {
       type: aType,
-      id: aOptions.overridePingId || Policy.generatePingId(),
+      id: Policy.generatePingId(),
       creationDate: (Policy.now()).toISOString(),
       version: PING_FORMAT_VERSION,
       application: this._getApplicationSection(),
       payload,
     };
 
     if (aOptions.addClientId) {
       pingData.clientId = this._clientID;
@@ -456,18 +448,16 @@ var Impl = {
    * @param {String} aType The type of the ping.
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overridePingId=null] set to override the
-   *                 generated ping id.
    * @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
    */
   _submitPingLogic: Task.async(function* (aType, aPayload, aOptions) {
     // Make sure to have a clientId if we need one. This cover the case of submitting
     // a ping early during startup, before Telemetry is initialized, if no client id was
     // cached.
     if (!this._clientID && aOptions.addClientId) {
       Telemetry.getHistogramById("TELEMETRY_PING_SUBMISSION_WAITING_CLIENTID").add();
@@ -496,18 +486,16 @@ var Impl = {
    * @param {String} aType The type of the ping.
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} [aOptions] Options object.
    * @param {Boolean} [aOptions.addClientId=false] true if the ping should contain the client
    *                  id, false otherwise.
    * @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
    *                  environment data.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overridePingId=null] set to override the
-   *                 generated ping id.
    * @returns {Promise} Test-only - a promise that is resolved with the ping id once the ping is stored or sent.
    */
   submitExternalPing: function send(aType, aPayload, aOptions) {
     this._log.trace("submitExternalPing - type: " + aType + ", aOptions: " + JSON.stringify(aOptions));
 
     // Reject pings sent after shutdown.
     if (this._shuttingDown) {
       const errorMessage = "submitExternalPing - Submission is not allowed after shutdown, discarding ping of type: " + aType;
@@ -543,18 +531,16 @@ var Impl = {
    * @param {Object} aPayload The actual data payload for the ping.
    * @param {Object} aOptions Options object.
    * @param {Boolean} aOptions.addClientId true if the ping should contain the client id,
    *                  false otherwise.
    * @param {Boolean} aOptions.addEnvironment true if the ping should contain the
    *                  environment data.
    * @param {Boolean} aOptions.overwrite true overwrites a ping with the same name, if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overridePingId=null] set to override the
-   *                 generated ping id.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
    */
   addPendingPing: function addPendingPing(aType, aPayload, aOptions) {
     this._log.trace("addPendingPing - Type " + aType + ", aOptions " + JSON.stringify(aOptions));
 
     let pingData = this.assemblePing(aType, aPayload, aOptions);
@@ -581,18 +567,16 @@ var Impl = {
    * @param {String} aFilePath The path to save the ping to.
    * @param {Object} aOptions Options object.
    * @param {Boolean} aOptions.addClientId true if the ping should contain the client id,
    *                  false otherwise.
    * @param {Boolean} aOptions.addEnvironment true if the ping should contain the
    *                  environment data.
    * @param {Boolean} aOptions.overwrite true overwrites a ping with the same name, if found.
    * @param {Object}  [aOptions.overrideEnvironment=null] set to override the environment data.
-   * @param {String} [aOptions.overridePingId=null] set to override the
-   *                 generated ping id.
    *
    * @returns {Promise} A promise that resolves with the ping id when the ping is saved to
    *                    disk.
    */
   savePing: function savePing(aType, aPayload, aFilePath, aOptions) {
     this._log.trace("savePing - Type " + aType + ", File Path " + aFilePath +
                     ", aOptions " + JSON.stringify(aOptions));
     let pingData = this.assemblePing(aType, aPayload, aOptions);
--- a/toolkit/components/telemetry/docs/internals/pingsender.rst
+++ b/toolkit/components/telemetry/docs/internals/pingsender.rst
@@ -1,32 +1,36 @@
 Ping Sender
 ===========
 
 The ping sender is a minimalistic program whose sole purpose is to deliver a
-telemetry ping. It accepts a single parameter which is the URL the ping will
-be sent to (as an HTTP POST command) and once started it will wait to read the
-ping contents on its stdin stream. Once the ping has been read from stdin the
-ping sender will try to post it once, exiting with a non-zero value if it
-fails.
+telemetry ping. It accepts the following parameters:
+
+- the URL the ping will be sent to (as an HTTP POST command);
+- the path to an uncompressed file holding the ping contents.
+
+Once the ping has been read from disk the ping sender will try to post it once, exiting
+with a non-zero value if it fails. If sending the ping succeeds then the ping file is removed.
 
 The content of the HTTP request is *gzip* encoded. The request comes with a few
 additional headers:
 
 - ``User-Agent: pingsender/1.0``
 - ``X-PingSender-Version: 1.0``. Even if this data is already included by the user agent, this
   header is needed as the pipeline is not currently storing use agent strings and doing that
   could require storing a fair chunk of redundant extra data. We need to discern between pings
   sent using the ping sender and the ones sent using the normal flow, at the end of the
   ingestion pipeline, for validation purposes.
 
-The ping sender relies on libcurl for Linux and Mac build and on WinInet for
-Windows ones for its HTTP functionality. It currently ignores Firefox or the
-system proxy configuration.
+.. note::
+
+  The ping sender relies on libcurl for Linux and Mac build and on WinInet for
+  Windows ones for its HTTP functionality. It currently ignores Firefox or the
+  system proxy configuration.
 
 In non-debug mode the ping sender doesn't print anything, not even on error,
 this is done deliberately to prevent startling the user on architectures such
 as Windows that would open a seperate console window just to display the
 program output. If you need runtime information to be printed out compile the
-ping sender with debuggging enabled.
+ping sender with debugging enabled.
 
 The pingsender is not supported on Firefox for Android at the moment
 (see `bug 1335917 <https://bugzilla.mozilla.org/show_bug.cgi?id=1335917>`_)
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -538,16 +538,16 @@ interface nsITelemetry : nsISupports
   void clearEvents();
 
   /**
    * Send a ping using the ping sender.
    * This method will not wait for the ping to be sent, instead it will return
    * as soon as the contents of the ping have been handed over to the ping
    * sender.
    *
-   * @param aUrl The telemetry server URL
-   * @param aPing A string holding the ping contents
+   * @param {String} aUrl The telemetry server URL
+   * @param {String} aPingFilePath The path to the file holding the ping
+   *        contents, if if sent successfully the pingsender will delete it.
    *
-   * @throws NS_ERROR_FAILURE if we couldn't run the pingsender or if we
-   *         couldn't hand it the ping data
+   * @throws NS_ERROR_FAILURE if we couldn't find or run the pingsender executable.
    */
-  void runPingSender(in ACString aUrl, in ACString aPing);
+  void runPingSender(in ACString aUrl, in ACString aPingFilePath);
 };
--- a/toolkit/components/telemetry/pingsender/pingsender.cpp
+++ b/toolkit/components/telemetry/pingsender/pingsender.cpp
@@ -1,22 +1,24 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <cstdlib>
 #include <ctime>
+#include <fstream>
 #include <iomanip>
 #include <string>
-#include <cstdio>
 #include <zlib.h>
 
 #include "pingsender.h"
 
+using std::ifstream;
+using std::ios;
 using std::string;
 
 namespace PingSender {
 
 const char* kUserAgent = "pingsender/1.0";
 const char* kCustomVersionHeader = "X-PingSender-Version: 1.0";
 const char* kContentEncodingHeader = "Content-Encoding: gzip";
 
@@ -29,35 +31,43 @@ GenerateDateHeader()
 {
   char buffer[128];
   std::time_t t = std::time(nullptr);
   strftime(buffer, sizeof(buffer), "Date: %a, %d %b %Y %H:%M:%S GMT", std::gmtime(&t));
   return string(buffer);
 }
 
 /**
- * Read the ping contents from stdin as a string
+ * Read the ping contents from the specified file
  */
 static std::string
-ReadPingFromStdin()
+ReadPing(const string& aPingPath)
 {
-  const size_t kBufferSize = 32768;
-  char buff[kBufferSize];
   string ping;
-  size_t readBytes = 0;
+  ifstream file;
+
+  file.open(aPingPath.c_str(), ios::in | ios::binary);
+
+  if (!file.is_open()) {
+    PINGSENDER_LOG("ERROR: Could not open ping file\n");
+    return "";
+  }
 
   do {
-    readBytes = fread(buff, 1, kBufferSize, stdin);
-    ping.append(buff, readBytes);
-  } while (!feof(stdin) && !ferror(stdin));
+    char buff[4096];
+
+    file.read(buff, sizeof(buff));
 
-  if (ferror(stdin)) {
-    PINGSENDER_LOG("ERROR: Could not read from stdin\n");
-    return "";
-  }
+    if (file.bad()) {
+      PINGSENDER_LOG("ERROR: Could not read ping contents\n");
+      return "";
+    }
+
+    ping.append(buff, file.gcount());
+  } while (!file.eof());
 
   return ping;
 }
 
 std::string
 GzipCompress(const std::string& rawData)
 {
   z_stream deflater = {};
@@ -131,32 +141,30 @@ GzipCompress(const std::string& rawData)
 
 } // namespace PingSender
 
 using namespace PingSender;
 
 int main(int argc, char* argv[])
 {
   string url;
+  string pingPath;
 
-  if (argc == 2) {
+  if (argc == 3) {
     url = argv[1];
+    pingPath = argv[2];
   } else {
-    PINGSENDER_LOG("Usage: pingsender URL\n"
-                   "Send the payload passed via stdin to the specified URL "
-                   "using an HTTP POST message\n");
+    PINGSENDER_LOG("Usage: pingsender URL PATH\n"
+                   "Send the payload stored in PATH to the specified URL using "
+                   "an HTTP POST message\n"
+                   "then delete the file after a successful send.\n");
     exit(EXIT_FAILURE);
   }
 
-  if (url.empty()) {
-    PINGSENDER_LOG("ERROR: No URL was provided\n");
-    exit(EXIT_FAILURE);
-  }
-
-  string ping(ReadPingFromStdin());
+  string ping(ReadPing(pingPath));
 
   if (ping.empty()) {
     PINGSENDER_LOG("ERROR: Ping payload is empty\n");
     exit(EXIT_FAILURE);
   }
 
   // Compress the ping using gzip.
   string gzipPing(GzipCompress(ping));
@@ -168,10 +176,16 @@ int main(int argc, char* argv[])
     PINGSENDER_LOG("ERROR: Ping compression failed\n");
     exit(EXIT_FAILURE);
   }
 
   if (!Post(url, gzipPing)) {
     exit(EXIT_FAILURE);
   }
 
+  // If the ping was successfully sent, delete the file.
+  if (!pingPath.empty() && std::remove(pingPath.c_str())) {
+    // We failed to remove the pending ping file.
+    exit(EXIT_FAILURE);
+  }
+
   exit(EXIT_SUCCESS);
 }
--- a/toolkit/components/telemetry/tests/unit/test_PingSender.js
+++ b/toolkit/components/telemetry/tests/unit/test_PingSender.js
@@ -1,63 +1,126 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/
 */
 
 // This tests submitting a ping using the stand-alone pingsender program.
 
 "use strict";
 
-Cu.import("resource://gre/modules/TelemetryUtils.jsm", this);
+Cu.import("resource://gre/modules/osfile.jsm", this);
+Cu.import("resource://gre/modules/Preferences.jsm", this);
+Cu.import("resource://gre/modules/PromiseUtils.jsm", this);
 Cu.import("resource://gre/modules/Services.jsm", this);
-Cu.import("resource://gre/modules/osfile.jsm", this);
+Cu.import("resource://gre/modules/TelemetryStorage.jsm", this);
+Cu.import("resource://gre/modules/TelemetryUtils.jsm", this);
+Cu.import("resource://gre/modules/Timer.jsm", this);
+
+const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
+const PREF_TELEMETRY_SERVER = "toolkit.telemetry.server";
+
+/**
+ * Wait for a ping file to be deleted from the pending pings directory.
+ */
+function waitForPingDeletion(pingId) {
+  const path = OS.Path.join(TelemetryStorage.pingDirectoryPath, pingId);
+
+  let checkFn = (resolve, reject) => setTimeout(() => {
+    OS.File.exists(path).then(exists => {
+      if (!exists) {
+        Assert.ok(true, pingId + " was deleted");
+        resolve();
+      } else {
+        checkFn(resolve, reject);
+      }
+    }, reject);
+  }, 250);
+
+  return new Promise((resolve, reject) => checkFn(resolve, reject));
+}
+
+add_task(function* setup() {
+  // Init the profile.
+  do_get_profile(true);
+
+  Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);
+  Services.prefs.setBoolPref(PREF_FHR_UPLOAD_ENABLED, true);
+
+  // Start the ping server and let Telemetry know about it.
+  PingServer.start();
+});
 
 add_task(function* test_pingSender() {
-  // Make sure the code can find the pingsender executable
-  const XRE_APP_DISTRIBUTION_DIR = "XREAppDist";
-  let dir = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
-  dir.initWithPath(OS.Constants.Path.libDir);
-
-  Services.dirsvc.registerProvider({
-    getFile(aProp, aPersistent) {
-      aPersistent.value = true;
-      if (aProp == XRE_APP_DISTRIBUTION_DIR) {
-        return dir.clone();
-      }
-      return null;
-    }
-  });
-
-  PingServer.start();
-
-  const url = "http://localhost:" + PingServer.port + "/submit/telemetry/";
+  // Generate a new ping and save it among the pending pings.
   const data = {
     type: "test-pingsender-type",
     id: TelemetryUtils.generateUUID(),
     creationDate: (new Date(1485810000)).toISOString(),
     version: 4,
     payload: {
       dummy: "stuff"
     }
   };
+  yield TelemetryStorage.savePing(data, true);
 
-  Telemetry.runPingSender(url, JSON.stringify(data));
+  // Get the local path of the saved ping.
+  const pingPath = OS.Path.join(TelemetryStorage.pingDirectoryPath, data.id);
+
+  // Spawn an HTTP server that returns an error. We will be running the
+  // PingSender twice, trying to send the ping to this server. After the
+  // second time, we will resolve |deferred404Hit|.
+  let failingServer = new HttpServer();
+  let deferred404Hit = PromiseUtils.defer();
+  let hitCount = 0;
+  failingServer.registerPathHandler("/lookup_fail", (metadata, response) => {
+    response.setStatusLine("1.1", 404, "Not Found");
+    hitCount++;
+
+    if (hitCount >= 2) {
+      // Resolve the promise on the next tick.
+      Services.tm.currentThread.dispatch(() => deferred404Hit.resolve(),
+                                         Ci.nsIThread.DISPATCH_NORMAL);
+    }
+  });
+  failingServer.start(-1);
+
+  // Try to send the ping twice using the pingsender (we expect 404 both times).
+  const errorUrl = "http://localhost:" + failingServer.identity.primaryPort + "/lookup_fail";
+  Telemetry.runPingSender(errorUrl, pingPath);
+  Telemetry.runPingSender(errorUrl, pingPath);
+
+  // Wait until we hit the 404 server twice. After that, make sure that the ping
+  // still exists locally.
+  yield deferred404Hit.promise;
+  Assert.ok((yield OS.File.exists(pingPath)),
+            "The pending ping must not be deleted if we fail to send using the PingSender");
+
+  // Try to send it using the pingsender.
+  const url = "http://localhost:" + PingServer.port + "/submit/telemetry/";
+  Telemetry.runPingSender(url, pingPath);
 
   let req = yield PingServer.promiseNextRequest();
   let ping = decodeRequestPayload(req);
 
   Assert.equal(req.getHeader("User-Agent"), "pingsender/1.0",
                "Should have received the correct user agent string.");
   Assert.equal(req.getHeader("X-PingSender-Version"), "1.0",
                "Should have received the correct PingSender version string.");
   Assert.equal(req.getHeader("Content-Encoding"), "gzip",
                "Should have a gzip encoded ping.");
   Assert.ok(req.getHeader("Date"), "Should have received a Date header.");
   Assert.equal(ping.id, data.id, "Should have received the correct ping id.");
   Assert.equal(ping.type, data.type,
                "Should have received the correct ping type.");
   Assert.deepEqual(ping.payload, data.payload,
                    "Should have received the correct payload.");
+
+  // Check that the PingSender removed the pending ping.
+  yield waitForPingDeletion(data.id);
+
+  // Shut down the failing server. We do this now, and not right after using it,
+  // to make sure we're not interfering with the test.
+  yield new Promise(r => failingServer.stop(r));
 });
 
 add_task(function* cleanup() {
   yield PingServer.stop();
 });
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryController.js
@@ -31,27 +31,26 @@ const APP_NAME = "XPCShell";
 const PREF_BRANCH = "toolkit.telemetry.";
 const PREF_ENABLED = PREF_BRANCH + "enabled";
 const PREF_ARCHIVE_ENABLED = PREF_BRANCH + "archive.enabled";
 const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
 const PREF_UNIFIED = PREF_BRANCH + "unified";
 
 var gClientID = null;
 
-function sendPing(aSendClientId, aSendEnvironment, aOverridePingId) {
+function sendPing(aSendClientId, aSendEnvironment) {
   if (PingServer.started) {
     TelemetrySend.setServer("http://localhost:" + PingServer.port);
   } else {
     TelemetrySend.setServer("http://doesnotexist");
   }
 
   let options = {
     addClientId: aSendClientId,
     addEnvironment: aSendEnvironment,
-    overridePingId: aOverridePingId || null,
   };
   return TelemetryController.submitExternalPing(TEST_PING_TYPE, {}, options);
 }
 
 function checkPingFormat(aPing, aType, aHasClientId, aHasEnvironment) {
   const MANDATORY_PING_FIELDS = [
     "type", "id", "creationDate", "version", "application", "payload"
   ];
@@ -283,28 +282,16 @@ add_task(function* test_pingHasEnvironme
   checkPingFormat(ping, TEST_PING_TYPE, true, true);
 
   // Test a field in the environment build section.
   Assert.equal(ping.application.buildId, ping.environment.build.buildId);
   // Test that we have the correct clientId.
   Assert.equal(ping.clientId, gClientID, "The correct clientId must be reported.");
 });
 
-add_task(function* test_pingIdCanBeOverridden() {
-  // Send a ping with an overridden id
-  const myPingId = TelemetryUtils.generateUUID();
-  yield sendPing(/* aSendClientId */ false,
-                 /* aSendEnvironment */ false,
-                 myPingId);
-  let ping = yield PingServer.promiseNextPing();
-  checkPingFormat(ping, TEST_PING_TYPE, false, false);
-
-  Assert.equal(ping.id, myPingId, "The ping id must be the one we provided.");
-});
-
 add_task(function* test_archivePings() {
   let now = new Date(2009, 10, 18, 12, 0, 0);
   fakeNow(now);
 
   // Disable ping upload so that pings don't get sent.
   // With unified telemetry the FHR upload pref controls this,
   // with non-unified telemetry the Telemetry enabled pref.
   const isUnified = Preferences.get(PREF_UNIFIED, false);
--- a/toolkit/crashreporter/client/crashreporter.cpp
+++ b/toolkit/crashreporter/client/crashreporter.cpp
@@ -28,16 +28,17 @@ using std::ofstream;
 using std::vector;
 using std::auto_ptr;
 
 namespace CrashReporter {
 
 StringTable  gStrings;
 string       gSettingsPath;
 string       gEventsPath;
+string       gPingPath;
 int          gArgc;
 char**       gArgv;
 
 enum SubmissionResult {Succeeded, Failed};
 
 static auto_ptr<ofstream> gLogStream(nullptr);
 static string             gReporterDumpFile;
 static string             gExtraFile;
@@ -543,19 +544,20 @@ int main(int argc, char** argv)
     gReporterDumpFile = argv[1];
   }
 
   if (gReporterDumpFile.empty()) {
     // no dump file specified, run the default UI
     UIShowDefaultUI();
   } else {
     // Start by running minidump analyzer to gather stack traces.
-    string empty;
+    string reporterDumpFile = gReporterDumpFile;
+    vector<string> args = { reporterDumpFile };
     UIRunProgram(GetProgramPath(UI_MINIDUMP_ANALYZER_FILENAME),
-                 gReporterDumpFile, empty, /* wait */ true);
+                 args, /* wait */ true);
 
     // go ahead with the crash reporter
     gExtraFile = GetAdditionalFilename(gReporterDumpFile, kExtraDataExtension);
     if (gExtraFile.empty()) {
       UIError(gStrings[ST_ERROR_BADARGUMENTS]);
       return 0;
     }
 
@@ -587,65 +589,39 @@ int main(int argc, char** argv)
 
     if (queryParameters.find("ServerURL") == queryParameters.end()) {
       UIError(gStrings[ST_ERROR_NOSERVERURL]);
       return 0;
     }
 
     // Hopefully the settings path exists in the environment. Try that before
     // asking the platform-specific code to guess.
-#ifdef XP_WIN32
-    static const wchar_t kDataDirKey[] = L"MOZ_CRASHREPORTER_DATA_DIRECTORY";
-    const wchar_t *settingsPath = _wgetenv(kDataDirKey);
-    if (settingsPath && *settingsPath) {
-      gSettingsPath = WideToUTF8(settingsPath);
-    }
-#else
-    static const char kDataDirKey[] = "MOZ_CRASHREPORTER_DATA_DIRECTORY";
-    const char *settingsPath = getenv(kDataDirKey);
-    if (settingsPath && *settingsPath) {
-      gSettingsPath = settingsPath;
-    }
-#endif
-    else {
+    gSettingsPath = UIGetEnv("MOZ_CRASHREPORTER_DATA_DIRECTORY");
+    if (gSettingsPath.empty()) {
       string product = queryParameters["ProductName"];
       string vendor = queryParameters["Vendor"];
       if (!UIGetSettingsPath(vendor, product, gSettingsPath)) {
         gSettingsPath.clear();
       }
     }
 
     if (gSettingsPath.empty() || !UIEnsurePathExists(gSettingsPath)) {
       UIError(gStrings[ST_ERROR_NOSETTINGSPATH]);
       return 0;
     }
 
     OpenLogFile();
 
-#ifdef XP_WIN32
-    static const wchar_t kEventsDirKey[] = L"MOZ_CRASHREPORTER_EVENTS_DIRECTORY";
-    const wchar_t *eventsPath = _wgetenv(kEventsDirKey);
-    if (eventsPath && *eventsPath) {
-      gEventsPath = WideToUTF8(eventsPath);
-    }
-#else
-    static const char kEventsDirKey[] = "MOZ_CRASHREPORTER_EVENTS_DIRECTORY";
-    const char *eventsPath = getenv(kEventsDirKey);
-    if (eventsPath && *eventsPath) {
-      gEventsPath = eventsPath;
-    }
-#endif
-    else {
-      gEventsPath.clear();
-    }
+    gEventsPath = UIGetEnv("MOZ_CRASHREPORTER_EVENTS_DIRECTORY");
+    gPingPath = UIGetEnv("MOZ_CRASHREPORTER_PING_DIRECTORY");
 
     // Assemble and send the crash ping
     string pingUuid;
 
-    if (SendCrashPing(queryParameters, pingUuid)) {
+    if (SendCrashPing(queryParameters, pingUuid, gPingPath)) {
       AppendToEventFile("CrashPingUUID", pingUuid);
     }
 
     // Update the crash event with stacks if they are present
     auto stackTracesItr = queryParameters.find("StackTraces");
     if (stackTracesItr != queryParameters.end()) {
       AppendToEventFile(stackTracesItr->first, stackTracesItr->second);
     }
--- a/toolkit/crashreporter/client/crashreporter.h
+++ b/toolkit/crashreporter/client/crashreporter.h
@@ -113,17 +113,18 @@ namespace CrashReporter {
                           const std::string& header,
                           StringTable& strings,
                           bool escape);
   void LogMessage(const std::string& message);
   void DeleteDump();
   bool ShouldEnableSending();
 
   // Telemetry ping
-  bool SendCrashPing(StringTable& strings, std::string& pingUuid);
+  bool SendCrashPing(StringTable& strings, std::string& pingUuid,
+                     const std::string& pingDir);
 
   static const unsigned int kSaveCount = 10;
 }
 
 //=============================================================================
 // implemented in the platform-specific files
 //=============================================================================
 
@@ -152,23 +153,24 @@ bool UIFileExists(const std::string& pat
 bool UIMoveFile(const std::string& oldfile, const std::string& newfile);
 bool UIDeleteFile(const std::string& oldfile);
 std::ifstream* UIOpenRead(const std::string& filename);
 std::ofstream* UIOpenWrite(const std::string& filename,
                            bool append=false,
                            bool binary=false);
 void UIPruneSavedDumps(const std::string& directory);
 
-// Run the program specified by exename, passing it the parameter in arg and
-// then sending it the contents of data (if any) via a pipe tied to its stdin.
+// Run the program specified by exename, passing it the parameters in arg.
 // If wait is true, wait for the program to terminate execution before
 // returning. Returns true if the program was launched correctly, false
 // otherwise.
 bool UIRunProgram(const std::string& exename,
-                  const std::string& arg,
-                  const std::string& data,
+                  const std::vector<std::string>& args,
                   bool wait = false);
 
+// Read the environment variable specified by name
+std::string UIGetEnv(const std::string name);
+
 #ifdef _MSC_VER
 # pragma warning( pop )
 #endif
 
 #endif
--- a/toolkit/crashreporter/client/crashreporter_unix_common.cpp
+++ b/toolkit/crashreporter/client/crashreporter_unix_common.cpp
@@ -65,72 +65,55 @@ void UIPruneSavedDumps(const std::string
     // s/.dmp/.extra/
     path.replace(path.size() - 4, 4, ".extra");
     UIDeleteFile(path.c_str());
 
     dumpfiles.pop_back();
   }
 }
 
-bool UIRunProgram(const std::string& exename, const std::string& arg,
-                  const std::string& data, bool wait)
+bool UIRunProgram(const std::string& exename,
+                  const std::vector<std::string>& args,
+                  bool wait)
 {
-  bool usePipes = !data.empty();
-  int fd[2];
-
-  if (usePipes && (pipe(fd) != 0)) {
-    return false;
-  }
-
   pid_t pid = fork();
 
   if (pid == -1) {
     return false;
   } else if (pid == 0) {
     // Child
-    if (usePipes) {
-      if (dup2(fd[0], STDIN_FILENO) == -1) {
-        exit(EXIT_FAILURE);
-      }
+    size_t argvLen = args.size() + 2;
+    char** argv = new char*[argvLen];
 
-      close(fd[0]);
-      close(fd[1]);
+    argv[0] = const_cast<char*>(exename.c_str());
+
+    for (size_t i = 0; i < args.size(); i++) {
+      argv[i + 1] = const_cast<char*>(args[i].c_str());
     }
 
-    char* argv[] = {
-      const_cast<char*>(exename.c_str()),
-      const_cast<char*>(arg.c_str()),
-      nullptr
-    };
+    argv[argvLen - 1] = nullptr;
 
     // Run the program
     int rv = execv(exename.c_str(), argv);
+    delete[] argv;
 
     if (rv == -1) {
       exit(EXIT_FAILURE);
     }
   } else {
     // Parent
-    if (usePipes) {
-      ssize_t rv;
-      size_t offset = 0;
-      size_t size = data.size();
-
-      do {
-        rv = write(fd[1], data.c_str() + offset, size);
-
-        if (rv > 0) {
-          size -= rv;
-          offset += rv;
-        }
-      } while (size && ((rv > 0) || ((rv == -1) && errno == EINTR)));
-
-      close(fd[0]);
-      close(fd[1]);
-    }
-
     if (wait) {
       waitpid(pid, nullptr, 0);
     }
   }
 
   return true;
 }
+
+string UIGetEnv(const string name)
+{
+  const char *var = getenv(name.c_str());
+  if (var && *var) {
+    return var;
+  }
+
+  return "";
+}
--- a/toolkit/crashreporter/client/crashreporter_win.cpp
+++ b/toolkit/crashreporter/client/crashreporter_win.cpp
@@ -1540,86 +1540,53 @@ void UIPruneSavedDumps(const std::string
     // s/.dmp/.extra/
     path.replace(path.size() - 4, 4, L".extra");
     DeleteFile(path.c_str());
 
     dumpfiles.pop_back();
   }
 }
 
-bool UIRunProgram(const string& exename, const string& arg,
-                  const string& data, bool wait)
+bool UIRunProgram(const string& exename,
+                  const std::vector<std::string>& args,
+                  bool wait)
 {
-  bool usePipes = !data.empty();
-  HANDLE childStdinPipeRead = nullptr;
-  HANDLE childStdinPipeWrite = nullptr;
-
-  if (usePipes) {
-    // Set up the pipes
-    SECURITY_ATTRIBUTES sa = {};
-    sa.nLength = sizeof(SECURITY_ATTRIBUTES);
-    sa.lpSecurityDescriptor = nullptr;
-    sa.bInheritHandle = true;
+  wstring cmdLine = L"\"" + UTF8ToWide(exename) + L"\" ";
 
-    // Create a pipe for the child process's stdin and ensure its writable end is
-    // not inherited by the child process.
-    if (!CreatePipe(&childStdinPipeRead, &childStdinPipeWrite, &sa, 0)) {
-      return false;
-    }
-
-    if (!SetHandleInformation(childStdinPipeWrite, HANDLE_FLAG_INHERIT, 0)) {
-      return false;
-    }
+  for (auto arg : args) {
+    cmdLine += L"\"" + UTF8ToWide(arg) + L"\" ";
   }
 
-  wstring cmdLine = L"\"" + UTF8ToWide(exename) + L"\" " +
-                    L"\"" + UTF8ToWide(arg) + L"\" ";
-
   STARTUPINFO si = {};
   si.cb = sizeof(si);
-
-  if (usePipes) {
-    si.dwFlags |= STARTF_USESTDHANDLES;
-    si.hStdInput = childStdinPipeRead;
-    si.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
-    si.hStdError = GetStdHandle(STD_ERROR_HANDLE);
-  }
-
   PROCESS_INFORMATION pi = {};
 
   if (!CreateProcess(/* lpApplicationName */ nullptr,
                      (LPWSTR)cmdLine.c_str(),
                      /* lpProcessAttributes */ nullptr,
                      /* lpThreadAttributes */ nullptr,
-                     usePipes,
+                     /* bInheritHandles */ false,
                      NORMAL_PRIORITY_CLASS | CREATE_NO_WINDOW,
                      /* lpEnvironment */ nullptr,
                      /* lpCurrentDirectory */ nullptr,
                      &si, &pi)) {
     return false;
   }
 
-  if (usePipes) {
-    size_t offset = 0;
-    size_t size = data.size();
-    while (size > 0) {
-      DWORD written = 0;
-      bool success = WriteFile(childStdinPipeWrite, data.data() + offset, size,
-                               &written, nullptr);
-      if (!success) {
-        break;
-      } else {
-        size -= written;
-        offset += written;
-      }
-    }
-
-    CloseHandle(childStdinPipeWrite);
-  }
-
   if (wait) {
     WaitForSingleObject(pi.hProcess, INFINITE);
   }
 
   CloseHandle(pi.hProcess);
   CloseHandle(pi.hThread);
   return true;
 }
+
+string
+UIGetEnv(const string name)
+{
+  const wchar_t *var = _wgetenv(UTF8ToWide(name).c_str());
+  if (var && *var) {
+    return WideToUTF8(var);
+  }
+
+  return "";
+}
--- a/toolkit/crashreporter/client/ping.cpp
+++ b/toolkit/crashreporter/client/ping.cpp
@@ -264,28 +264,52 @@ GenerateSubmissionUrl(const string& aUrl
                       const string& aName, const string& aVersion,
                       const string& aChannel, const string& aBuildId)
 {
   return aUrl + "/submit/telemetry/" + aId + "/crash/" + aName + "/" +
          aVersion + "/" + aChannel + "/" + aBuildId +
          "?v=" + std::to_string(kTelemetryVersion);
 }
 
+// Write out the ping into the specified file.
+//
+// Returns true if the ping was written out successfully, false otherwise.
+static bool
+WritePing(const string& aPath, const string& aPing)
+{
+  ofstream* f = UIOpenWrite(aPath.c_str());
+  bool success = false;
+
+  if (f->is_open()) {
+    *f << aPing;
+    f->flush();
+
+    if (f->good()) {
+      success = true;
+    }
+
+    f->close();
+  }
+
+  delete f;
+  return success;
+}
+
 // Assembles the crash ping using the strings extracted from the .extra file
 // and sends it using the crash sender. All the telemetry specific data but the
 // environment will be stripped from the annotations so that it won't be sent
 // together with the crash report.
 //
 // Note that the crash ping sender is invoked in a fire-and-forget way so this
 // won't block waiting for the ping to be delivered.
 //
 // Returns true if the ping was assembled and handed over to the pingsender
 // correctly, false otherwise and populates the aUUID field with the ping UUID.
 bool
-SendCrashPing(StringTable& strings, string& pingUuid)
+SendCrashPing(StringTable& strings, string& pingUuid, const string& pingDir)
 {
   string clientId    = strings[kTelemetryClientId];
   string serverUrl   = strings[kTelemetryUrl];
   string sessionId   = strings[kTelemetrySessionId];
 
   // Remove the telemetry-related data from the crash annotations
   strings.erase(kTelemetryClientId);
   strings.erase(kTelemetryUrl);
@@ -301,22 +325,28 @@ SendCrashPing(StringTable& strings, stri
 
   if (serverUrl.empty() || uuid.empty()) {
     return false;
   }
 
   Json::Value root = CreateRootNode(strings, uuid, clientId, sessionId,
                                     name, version, channel, buildId);
 
-  // Write out the result
+  // Write out the result to the pending pings directory
   Json::FastWriter writer;
   string ping = writer.write(root);
+  string pingPath = pingDir + UI_DIR_SEPARATOR + uuid + ".json";
+
+  if (!WritePing(pingPath, ping)) {
+    return false;
+  }
 
   // Hand over the ping to the sender
-  if (UIRunProgram(GetProgramPath(UI_PING_SENDER_FILENAME), url, ping)) {
+  vector<string> args = { url, pingPath };
+  if (UIRunProgram(GetProgramPath(UI_PING_SENDER_FILENAME), args)) {
     pingUuid = uuid;
     return true;
   } else {
     return false;
   }
 }
 
 } // namespace crashreporter
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -1983,58 +1983,91 @@ EnsureDirectoryExists(nsIFile* dir)
 
   if (NS_WARN_IF(NS_FAILED(rv) && rv != NS_ERROR_FILE_ALREADY_EXISTS)) {
     return rv;
   }
 
   return NS_OK;
 }
 
+// Creates a directory that will be accessible by the crash reporter. The
+// directory will live under Firefox default data directory and will use the
+// specified name. The directory path will be passed to the crashreporter via
+// the specified environment variable.
+static nsresult
+SetupCrashReporterDirectory(nsIFile* aAppDataDirectory,
+                            const char* aDirName,
+                            const XP_CHAR* aEnvVarName,
+                            nsIFile** aDirectory = nullptr)
+{
+  nsCOMPtr<nsIFile> directory;
+  nsresult rv = aAppDataDirectory->Clone(getter_AddRefs(directory));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = directory->AppendNative(nsDependentCString(aDirName));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  EnsureDirectoryExists(directory);
+
+  xpstring dirEnv(aEnvVarName);
+  dirEnv.append(XP_TEXT("="));
+
+  xpstring* directoryPath = CreatePathFromFile(directory);
+
+  if (!directoryPath) {
+    return NS_ERROR_FAILURE;
+  }
+
+  dirEnv.append(*directoryPath);
+  delete directoryPath;
+
+#if defined(XP_WIN32)
+  _wputenv(dirEnv.c_str());
+#else
+  XP_CHAR* str = new XP_CHAR[dirEnv.size() + 1];
+  strncpy(str, dirEnv.c_str(), dirEnv.size() + 1);
+  PR_SetEnv(str);
+#endif
+
+  if (aDirectory) {
+    directory.forget(aDirectory);
+  }
+
+  return NS_OK;
+}
+
 // Annotate the crash report with a Unique User ID and time
 // since install.  Also do some prep work for recording
 // time since last crash, which must be calculated at
 // crash time.
 // If any piece of data doesn't exist, initialize it first.
 nsresult SetupExtraData(nsIFile* aAppDataDirectory,
                         const nsACString& aBuildID)
 {
   nsCOMPtr<nsIFile> dataDirectory;
-  nsresult rv = aAppDataDirectory->Clone(getter_AddRefs(dataDirectory));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = dataDirectory->AppendNative(NS_LITERAL_CSTRING("Crash Reports"));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  EnsureDirectoryExists(dataDirectory);
-
-#if defined(XP_WIN32)
-  nsAutoString dataDirEnv(NS_LITERAL_STRING("MOZ_CRASHREPORTER_DATA_DIRECTORY="));
-
-  nsAutoString dataDirectoryPath;
-  rv = dataDirectory->GetPath(dataDirectoryPath);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  dataDirEnv.Append(dataDirectoryPath);
-
-  _wputenv(dataDirEnv.get());
-#else
-  // Save this path in the environment for the crash reporter application.
-  nsAutoCString dataDirEnv("MOZ_CRASHREPORTER_DATA_DIRECTORY=");
-
-  nsAutoCString dataDirectoryPath;
-  rv = dataDirectory->GetNativePath(dataDirectoryPath);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  dataDirEnv.Append(dataDirectoryPath);
-
-  char* env = ToNewCString(dataDirEnv);
-  NS_ENSURE_TRUE(env, NS_ERROR_OUT_OF_MEMORY);
-
-  PR_SetEnv(env);
-#endif
+  nsresult rv = SetupCrashReporterDirectory(
+    aAppDataDirectory,
+    "Crash Reports",
+    XP_TEXT("MOZ_CRASHREPORTER_DATA_DIRECTORY"),
+    getter_AddRefs(dataDirectory)
+  );
+
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  rv = SetupCrashReporterDirectory(
+    aAppDataDirectory,
+    "Pending Pings",
+    XP_TEXT("MOZ_CRASHREPORTER_PING_DIRECTORY")
+  );
+
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   nsAutoCString data;
   if(NS_SUCCEEDED(GetOrInit(dataDirectory,
                             NS_LITERAL_CSTRING("InstallTime") + aBuildID,
                             data, InitInstallTime)))
     AnnotateCrashReport(NS_LITERAL_CSTRING("InstallTime"), data);
 
   // this is a little different, since we can't init it with anything,