Check the SRT logs upload time for an entire queue of SRT invocations at once.

chrome/browser/safe_browsing/srt_fetcher_win.cc

Index: chrome/browser/safe_browsing/srt_fetcher_win.cc
diff --git a/chrome/browser/safe_browsing/srt_fetcher_win.cc b/chrome/browser/safe_browsing/srt_fetcher_win.cc
index ac6021b6b0e89c7a599d5140855e5d58808c91a0..1bf1ba4b5b77a9391b34bba5ba26504078e03a92 100644
--- a/chrome/browser/safe_browsing/srt_fetcher_win.cc
+++ b/chrome/browser/safe_browsing/srt_fetcher_win.cc
@@ -913,6 +913,20 @@ class ReporterRunner : public chrome::BrowserListObserver {
          // Also make sure the kSwReporterLastTimeTriggered value is not set in
          // the future.
          last_time_triggered > now)) {
+      const base::Time now = base::Time::Now();

[ftirelo, 2017/02/06 14:13:24]

Please reuse |now| from line 905.

[Joe Mason, 2017/02/06 18:07:09]

Good catch. Done.

+      const base::Time last_time_sent_logs = base::Time::FromInternalValue(
+          local_state->GetInt64(prefs::kSwReporterLastTimeSentReport));
+      const base::Time next_time_send_logs =
+          last_time_sent_logs +
+          base::TimeDelta::FromDays(kDaysBetweenReporterLogsSent);
+      // Send the logs for this whole queue of invocations if the last send is
+      // in the future or if logs have been sent at least
+      // |kSwReporterLastTimeSentReport| days ago. The former is intended as a
+      // measure for failure recovery, in case the time in local state is
+      // incorrectly set to the future.
+      in_logs_upload_period_ =
+          last_time_sent_logs > now || next_time_send_logs <= now;
+
       DCHECK(current_invocations_.empty());
       current_invocations_ = pending_invocations_;
       ScheduleNextInvocation();
@@ -925,8 +939,8 @@ class ReporterRunner : public chrome::BrowserListObserver {
   }
 
   // Returns true if the experiment to send reporter logs is enabled, the user
-  // opted into Safe Browsing extended reporting, and logs have been sent at
-  // least |kSwReporterLastTimeSentReport| days ago.
+  // opted into Safe Browsing extended reporting, and this queue of invocations
+  // started during the logs upload interval.
   bool ShouldSendReporterLogs(const std::string& suffix,
                               const PrefService& local_state) {
     UMAHistogramReporter uma(suffix);
@@ -934,22 +948,12 @@ class ReporterRunner : public chrome::BrowserListObserver {
       uma.RecordLogsUploadEnabled(REPORTER_LOGS_UPLOADS_SBER_DISABLED);
       return false;
     }
-
-    const base::Time now = base::Time::Now();
-    const base::Time last_time_sent_logs = base::Time::FromInternalValue(
-        local_state.GetInt64(prefs::kSwReporterLastTimeSentReport));
-    const base::Time next_time_send_logs =
-        last_time_sent_logs +
-        base::TimeDelta::FromDays(kDaysBetweenReporterLogsSent);
-    // Send the logs if the last send is the future or if the interval has
-    // passed. The former is intended as a measure for failure recovery, in
-    // case the time in local state is incorrectly set to the future.
-    if (last_time_sent_logs > now || next_time_send_logs <= now) {
-      uma.RecordLogsUploadEnabled(REPORTER_LOGS_UPLOADS_ENABLED);
-      return true;
+    if (!in_logs_upload_period_) {
+      uma.RecordLogsUploadEnabled(REPORTER_LOGS_UPLOADS_RECENTLY_SENT_LOGS);
+      return false;
     }
-    uma.RecordLogsUploadEnabled(REPORTER_LOGS_UPLOADS_RECENTLY_SENT_LOGS);
-    return false;
+    uma.RecordLogsUploadEnabled(REPORTER_LOGS_UPLOADS_ENABLED);
+    return true;
   }
 
   // Appends switches to the next invocation that depend on the user current
@@ -1009,6 +1013,8 @@ class ReporterRunner : public chrome::BrowserListObserver {
   // should be run before adding the global error to the Chrome menu.
   int days_between_reporter_runs_ = kDaysBetweenSuccessfulSwReporterRuns;
 
+  bool in_logs_upload_period_ = false;

[grt (UTC plus 2), 2017/02/06 08:25:35]

please document this new member.

[Joe Mason, 2017/02/06 18:07:09]

Done.

+
   // A single leaky instance.
   static ReporterRunner* instance_;