From fb56def1a5fefa9ff21aec5c0114250b4bcb2260 Mon Sep 17 00:00:00 2001
From: "Roberto A. Vitillo" <rvitillo@mozilla.com>
Date: Tue, 18 Feb 2014 11:26:11 +0000
Subject: [PATCH] Bug 973579 - TelemetryFile should fail silently when not
 overwriting an existing file. r=Yoric

---
 .../components/telemetry/TelemetryFile.jsm    | 20 +++++++++++++------
 .../components/telemetry/TelemetryPing.jsm    | 14 ++++++-------
 .../tests/unit/test_TelemetryPing.js          |  9 +++++++++
 3 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/toolkit/components/telemetry/TelemetryFile.jsm b/toolkit/components/telemetry/TelemetryFile.jsm
index 31606974ffc..926a29a433d 100644
--- a/toolkit/components/telemetry/TelemetryFile.jsm
+++ b/toolkit/components/telemetry/TelemetryFile.jsm
@@ -64,14 +64,20 @@ this.TelemetryFile = {
    *
    * @param {object} ping The content of the ping to save.
    * @param {string} file The destination file.
-   * @param {bool} overwrite If |true|, the file will be overwritten
-   * if it exists.
+   * @param {bool} overwrite If |true|, the file will be overwritten if it exists,
+   * if |false| the file will not be overwritten and no error will be reported if
+   * the file exists.
    * @returns {promise}
    */
   savePingToFile: function(ping, file, overwrite) {
-    let pingString = JSON.stringify(ping);
-    return OS.File.writeAtomic(file, pingString, {tmpPath: file + ".tmp",
-                                noOverwrite: !overwrite});
+    return Task.spawn(function*() {
+      try {
+        let pingString = JSON.stringify(ping);
+        yield OS.File.writeAtomic(file, pingString, {tmpPath: file + ".tmp",
+                                  noOverwrite: !overwrite});
+      } catch(e if e.becauseExists) {
+      }
+    })
   },
 
   /**
@@ -86,7 +92,7 @@ this.TelemetryFile = {
     return Task.spawn(function*() {
       yield getPingDirectory();
       let file = pingFilePath(ping);
-      return this.savePingToFile(ping, file, overwrite);
+      yield this.savePingToFile(ping, file, overwrite);
     }.bind(this));
   },
 
@@ -98,6 +104,8 @@ this.TelemetryFile = {
    */
   savePendingPings: function(sessionPing) {
     let p = pendingPings.reduce((p, ping) => {
+      // Restore the files with the previous pings if for some reason they have
+      // been deleted, don't overwrite them otherwise.
       p.push(this.savePing(ping, false));
       return p;}, [this.savePing(sessionPing, true)]);
 
diff --git a/toolkit/components/telemetry/TelemetryPing.jsm b/toolkit/components/telemetry/TelemetryPing.jsm
index c6dcaa31d50..46fa4d0dae4 100644
--- a/toolkit/components/telemetry/TelemetryPing.jsm
+++ b/toolkit/components/telemetry/TelemetryPing.jsm
@@ -789,13 +789,13 @@ let Impl = {
 
     function handler(success) {
       return function(event) {
-        this.finishPingRequest(success, startTime, ping);
-
-        if (success) {
-          deferred.resolve();
-        } else {
-          deferred.reject(event);
-        }
+        this.finishPingRequest(success, startTime, ping).then(() => {
+          if (success) {
+            deferred.resolve();
+          } else {
+            deferred.reject(event);
+          }
+        });
       };
     }
     request.addEventListener("error", handler(false).bind(this), false);
diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js b/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
index 8eacd96057c..4d650a928ff 100644
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js
@@ -18,6 +18,7 @@ Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/LightweightThemeManager.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/TelemetryPing.jsm", this);
+Cu.import("resource://gre/modules/TelemetryFile.jsm", this);
 Cu.import("resource://gre/modules/Task.jsm", this);
 Cu.import("resource://gre/modules/Promise.jsm", this);
 
@@ -399,6 +400,14 @@ function actualTest() {
   run_next_test();
 }
 
+// Ensure that not overwriting an existing file fails silently
+add_task(function* test_overwritePing() {
+  let ping = {slug: "foo"}
+  yield TelemetryFile.savePing(ping, true);
+  yield TelemetryFile.savePing(ping, false);
+  yield TelemetryFile.cleanupPingFile(ping);
+});
+
 // Ensures that expired histograms are not part of the payload.
 add_task(function* test_expiredHistogram() {
   let histogram_id = "FOOBAR";