handler/mac: Respect the uploads-enabled user preference and rate-limit upload attempts

handler/mac/crash_report_upload_thread.cc

Index: handler/mac/crash_report_upload_thread.cc
diff --git a/handler/mac/crash_report_upload_thread.cc b/handler/mac/crash_report_upload_thread.cc
index 9efbfd9eebb9be32e465a4f697fc6d7d4be695a3..17c5a8bea3c7fdcd62acce04f8066e20058e7c5c 100644
--- a/handler/mac/crash_report_upload_thread.cc
+++ b/handler/mac/crash_report_upload_thread.cc
@@ -15,6 +15,7 @@
 #include "handler/mac/crash_report_upload_thread.h"
 
 #include <errno.h>
+#include <time.h>
 
 #include <map>
 #include <vector>
@@ -22,6 +23,7 @@
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "client/settings.h"
 #include "snapshot/minidump/process_snapshot_minidump.h"
 #include "snapshot/module_snapshot.h"
 #include "util/file/file_reader.h"
@@ -209,8 +211,48 @@ void CrashReportUploadThread::ProcessPendingReports() {
 
 void CrashReportUploadThread::ProcessPendingReport(
     const CrashReportDatabase::Report& report) {
-  // TODO(mark): Allow uploads to be disabled.
-  // TODO(mark): Rate-limit uploads.
+  Settings* const settings = database_->GetSettings();
+
+  bool uploads_enabled;
+  if (!settings->GetUploadsEnabled(&uploads_enabled) || !uploads_enabled) {
+    // If uploads are disabled or the upload-enabled state can’t be determined,
+    // don’t attempt to upload the new report.
+    database_->SkipReportUpload(report.uuid);
+    return;
+  }
+
+  // This currently implements very simplistic rate-limiting, compatible with
+  // the Breakpad client, where the strategy is to permit one upload attempt per
+  // hour, and retire reports that would exceed this limit or for which the
+  // upload fails on the first attempt.
+  //
+  // TODO(mark): Provide a proper rate-limiting strategy and allow for failed
+  // upload attempts to be retried.
+  time_t last_upload_attempt_time;
+  if (settings->GetLastUploadAttemptTime(&last_upload_attempt_time)) {
+    time_t now = time(nullptr);
+    if (now >= last_upload_attempt_time) {
+      // If the most recent upload attempt occurred within the past hour, don’t
+      // attempt to upload the new report. If it happened longer ago, attempt to
+      // upload the report.
+      const int kUploadAttemptIntervalSeconds = 60 * 60;  // 1 hour
+      if (now - last_upload_attempt_time < kUploadAttemptIntervalSeconds) {
+        database_->SkipReportUpload(report.uuid);
+        return;
+      }
+    } else {
+      // The most recent upload attempt purportedly occurred in the future. If
+      // it “happened” at least one day in the future, assume that the last

[Robert Sesek, 2015/03/10 19:20:54]

This makes me think that saturating the last report time is maybe not the best
option and that reseting to 0 is better. Would you still handle this case that
way if so? (Maybe yes because of bad clocks).

[Mark Mentovai, 2015/03/10 19:28:24]

Robert Sesek wrote:
Yes, exactly, this is primarily for bad clocks. Saturation’s just a very tiny
part of it by comparison.

+      // upload attempt time is bogus, and attempt to upload the report. If the
+      // most recent upload time is in the future but within one day, accept it
+      // and don’t attempt to upload the report.
+      const int kBackwardsClockTolerance = 60 * 60 * 24;  // 1 day
+      if (last_upload_attempt_time - now < kBackwardsClockTolerance) {
+        database_->SkipReportUpload(report.uuid);
+        return;
+      }
+    }
+  }
 
   const CrashReportDatabase::Report* upload_report;
   CrashReportDatabase::OperationStatus status =