Adds histograms for tracking Software Reporter logs uploads in SRT Fetcher.

chrome/browser/safe_browsing/srt_fetcher_win.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/safe_browsing/srt_fetcher_win.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 38 matching lines @@
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
 
 using content::BrowserThread;
 
 namespace safe_browsing {
 
+// TODO(b/647763) Change the registry key to properly handle cases when the user
+// runs Google Chrome stable alongside Google Chrome SxS.
 const wchar_t kSoftwareRemovalToolRegistryKey[] =
     L"Software\\Google\\Software Removal Tool";
 
 const wchar_t kCleanerSubKey[] = L"Cleaner";
 
 const wchar_t kEndTimeValueName[] = L"EndTime";
 const wchar_t kStartTimeValueName[] = L"StartTime";
 
 const char kExtendedSafeBrowsingEnabledSwitch[] =
     "extended-safebrowsing-enabled";
@@ skipping 36 matching lines @@
   SW_REPORTER_NO_LOCAL_STATE = 9,
   SW_REPORTER_NO_PROMPT_NEEDED = 10,
   SW_REPORTER_NO_PROMPT_FIELD_TRIAL = 11,
   SW_REPORTER_ALREADY_PROMPTED = 12,
   SW_REPORTER_RAN_DAILY = 13,
   SW_REPORTER_ADDED_TO_MENU = 14,
 
   SW_REPORTER_MAX,
 };
 
+// Used to send UMA information showing whether Software Reporter logs uploads

[grt (UTC plus 2), 2016/09/16 20:12:42]

feel fee to ignore this if the terminology is consistent with the project: i
find "foo logs uploads are enabled" hard to read! how about "foo logs uploading
is enabled" or "uploading of foo logs is enabled"?

[ftirelo, 2016/09/16 21:00:10]

Done.

+// are enabled, or the reason why not.
+// Replicated in the histograms.xml file, so the order MUST NOT CHANGE.
+enum SwReporterLogsUploadsEnabled {
+  REPORTER_LOGS_UPLOADS_ENABLED = 0,
+  REPORTER_LOGS_UPLOADS_SBER_DISABLED = 1,
+  REPORTER_LOGS_UPLOADS_RECENTLY_SENT_LOGS = 2,
+  REPORTER_LOGS_UPLOADS_MAX,
+};
+
+
 const char kRunningTimeErrorMetricName[] =
     "SoftwareReporter.RunningTimeRegistryError";
 
 SwReporterTestingDelegate* g_testing_delegate_ = nullptr;
 
 const wchar_t kScanTimesSubKey[] = L"ScanTimes";
 const wchar_t kFoundUwsValueName[] = L"FoundUws";
 const wchar_t kMemoryUsedValueName[] = L"MemoryUsed";
+const wchar_t kLogsUploadResultValueName[] = L"LogsUploadResult";
 
 const char kFoundUwsMetricName[] = "SoftwareReporter.FoundUwS";
 const char kFoundUwsReadErrorMetricName[] =
     "SoftwareReporter.FoundUwSReadError";
 const char kScanTimesMetricName[] = "SoftwareReporter.UwSScanTimes";
 const char kMemoryUsedMetricName[] = "SoftwareReporter.MemoryUsed";
+const char kLogsUploadEnabledMetricName[] =
+    "SoftwareReporter.LogsUploadEnabled";
+const char kLogsUploadResultMetricName[] = "SoftwareReporter.LogsUploadResult";
+
+// The max value for histogram SoftwareReporter.LogsUploadResult, which is used
+// to send UMA information about the result of Software Reporter's attempt to
+// upload logs, when logs are enabled. This value must be consistent with the
+// SoftwareReporterLogsUploadResult enum defined in the histograms.xml file.
+const int kSwReporterLogsUploadResultMax = 7;
 
 // Reports metrics about the software reporter via UMA (and sometimes Rappor).
 class UMAHistogramReporter {
  public:
   UMAHistogramReporter() : UMAHistogramReporter(std::string()) {}
 
   explicit UMAHistogramReporter(const std::string& suffix)
       : suffix_(suffix),
         registry_key_(suffix.empty() ? kSoftwareRemovalToolRegistryKey
                                      : base::StringPrintf(
@@ skipping 29 matching lines @@
     }
     RecordSparseHistogram("SoftwareReporter.MajorVersion", major_version);
   }
 
   void ReportExitCode(int exit_code) const {
     RecordSparseHistogram("SoftwareReporter.ExitCode", exit_code);
   }
 
   // Reports UwS found by the software reporter tool via UMA and RAPPOR.
   void ReportFoundUwS(bool use_rappor) const {
-    base::win::RegKey reporter_key(HKEY_CURRENT_USER, registry_key_.c_str(),
-                                   KEY_QUERY_VALUE | KEY_SET_VALUE);
+    base::win::RegKey reporter_key;
     std::vector<base::string16> found_uws_strings;
-    if (reporter_key.Valid() &&
-        reporter_key.ReadValues(kFoundUwsValueName, &found_uws_strings) ==
+    if (reporter_key.Open(HKEY_CURRENT_USER, registry_key_.c_str(),
+                          KEY_QUERY_VALUE | KEY_SET_VALUE) != ERROR_SUCCESS ||
+        reporter_key.ReadValues(kFoundUwsValueName, &found_uws_strings) !=
             ERROR_SUCCESS) {
-      rappor::RapporService* rappor_service = nullptr;
-      if (use_rappor)
-        rappor_service = g_browser_process->rappor_service();
+      return;
+    }
 
-      bool parse_error = false;
-      for (const base::string16& uws_string : found_uws_strings) {
-        // All UwS ids are expected to be integers.
-        uint32_t uws_id = 0;
-        if (base::StringToUint(uws_string, &uws_id)) {
-          RecordSparseHistogram(kFoundUwsMetricName, uws_id);
-          if (rappor_service) {
-            rappor_service->RecordSample(kFoundUwsMetricName,
-                                         rappor::COARSE_RAPPOR_TYPE,
-                                         base::UTF16ToUTF8(uws_string));
-          }
-        } else {
-          parse_error = true;
+    rappor::RapporService* rappor_service = nullptr;
+    if (use_rappor)
+      rappor_service = g_browser_process->rappor_service();
+
+    bool parse_error = false;
+    for (const base::string16& uws_string : found_uws_strings) {
+      // All UwS ids are expected to be integers.
+      uint32_t uws_id = 0;
+      if (base::StringToUint(uws_string, &uws_id)) {
+        RecordSparseHistogram(kFoundUwsMetricName, uws_id);
+        if (rappor_service) {
+          rappor_service->RecordSample(kFoundUwsMetricName,
+                                       rappor::COARSE_RAPPOR_TYPE,
+                                       base::UTF16ToUTF8(uws_string));
         }
+      } else {
+        parse_error = true;
       }
 
       // Clean up the old value.
       reporter_key.DeleteValue(kFoundUwsValueName);
 
       RecordBooleanHistogram(kFoundUwsReadErrorMetricName, parse_error);
     }
   }
 
   // Reports to UMA the memory usage of the software reporter tool as reported
   // by the tool itself in the Windows registry.
   void ReportMemoryUsage() const {
-    base::win::RegKey reporter_key(HKEY_CURRENT_USER, registry_key_.c_str(),
-                                   KEY_QUERY_VALUE | KEY_SET_VALUE);
+    base::win::RegKey reporter_key;
     DWORD memory_used = 0;
-    if (reporter_key.Valid() &&
-        reporter_key.ReadValueDW(kMemoryUsedValueName, &memory_used) ==
+    if (reporter_key.Open(HKEY_CURRENT_USER, registry_key_.c_str(),
+                          KEY_QUERY_VALUE | KEY_SET_VALUE) != ERROR_SUCCESS ||
+        reporter_key.ReadValueDW(kMemoryUsedValueName, &memory_used) !=
             ERROR_SUCCESS) {
-      RecordMemoryKBHistogram(kMemoryUsedMetricName, memory_used);
-      reporter_key.DeleteValue(kMemoryUsedValueName);
+      return;
     }
+    RecordMemoryKBHistogram(kMemoryUsedMetricName, memory_used);
+    reporter_key.DeleteValue(kMemoryUsedValueName);
   }
 
   // Reports the SwReporter run time with UMA both as reported by the tool via
   // the registry and as measured by |ReporterRunner|.
   void ReportRuntime(const base::TimeDelta& reporter_running_time) const {
     RecordLongTimesHistogram("SoftwareReporter.RunningTimeAccordingToChrome",
                              reporter_running_time);
 
-    // TODO(b/641081): This should only have KEY_QUERY_VALUE and KEY_SET_VALUE,
-    // and use Open to avoid creating the key if it doesn't already exist.
-    base::win::RegKey reporter_key(HKEY_CURRENT_USER, registry_key_.c_str(),
-                                   KEY_ALL_ACCESS);
-    if (!reporter_key.Valid()) {
+    // TODO(b/641081): This should only have KEY_QUERY_VALUE and KEY_SET_VALUE.
+    base::win::RegKey reporter_key;
+    if (reporter_key.Open(HKEY_CURRENT_USER, registry_key_.c_str(),
+                          KEY_ALL_ACCESS) != ERROR_SUCCESS) {
       RecordEnumerationHistogram(
           kRunningTimeErrorMetricName,
           REPORTER_RUNNING_TIME_ERROR_REGISTRY_KEY_INVALID,
           REPORTER_RUNNING_TIME_ERROR_MAX);
       return;
     }
 
     bool has_start_time = false;
     int64_t start_time_value = 0;
     if (reporter_key.HasValue(kStartTimeValueName) &&
@@ skipping 34 matching lines @@
       RecordEnumerationHistogram(kRunningTimeErrorMetricName,
                                  REPORTER_RUNNING_TIME_ERROR_MISSING_END_TIME,
                                  REPORTER_RUNNING_TIME_ERROR_MAX);
     }
   }
 
   // Reports the UwS scan times of the software reporter tool via UMA.
   void ReportScanTimes() const {
     base::string16 scan_times_key_path = base::StringPrintf(
         L"%ls\\%ls", registry_key_.c_str(), kScanTimesSubKey);
-    // TODO(b/641081): This should only have KEY_QUERY_VALUE and KEY_SET_VALUE,
-    // and use Open to avoid creating the key if it doesn't already exist.
-    base::win::RegKey scan_times_key(
-        HKEY_CURRENT_USER, scan_times_key_path.c_str(), KEY_ALL_ACCESS);
-    if (!scan_times_key.Valid())
+    // TODO(b/641081): This should only have KEY_QUERY_VALUE and KEY_SET_VALUE.
+    base::win::RegKey scan_times_key;
+    if (scan_times_key.Open(HKEY_CURRENT_USER, scan_times_key_path.c_str(),
+                            KEY_ALL_ACCESS) != ERROR_SUCCESS) {
       return;
+    }
 
     base::string16 value_name;
     int uws_id = 0;
     int64_t raw_scan_time = 0;
     int num_scan_times = scan_times_key.GetValueCount();
     for (int i = 0; i < num_scan_times; ++i) {
       if (scan_times_key.GetValueNameAt(i, &value_name) == ERROR_SUCCESS &&
           base::StringToInt(value_name, &uws_id) &&
           scan_times_key.ReadInt64(value_name.c_str(), &raw_scan_time) ==
               ERROR_SUCCESS) {
         base::TimeDelta scan_time =
             base::TimeDelta::FromInternalValue(raw_scan_time);
         // We report the number of seconds plus one because it can take less
         // than one second to scan some UwS and the count passed to |AddCount|
         // must be at least one.
         RecordSparseHistogramCount(kScanTimesMetricName, uws_id,
                                    scan_time.InSeconds() + 1);
       }
     }
     // Clean up by deleting the scan times key, which is a subkey of the main
     // reporter key.
     scan_times_key.Close();
-    base::win::RegKey reporter_key(HKEY_CURRENT_USER, registry_key_.c_str(),
-                                   KEY_ENUMERATE_SUB_KEYS);
-    if (reporter_key.Valid())
+    base::win::RegKey reporter_key;
+    if (reporter_key.Open(HKEY_CURRENT_USER, registry_key_.c_str(),
+                          KEY_ENUMERATE_SUB_KEYS) == ERROR_SUCCESS) {
       reporter_key.DeleteKey(kScanTimesSubKey);
+    }
   }
 
   void RecordReporterStep(SwReporterUmaValue value) {
     RecordEnumerationHistogram("SoftwareReporter.Step", value, SW_REPORTER_MAX);
   }
 
+  void RecordLogsUploadEnabled(SwReporterLogsUploadsEnabled value) {
+    RecordEnumerationHistogram(kLogsUploadEnabledMetricName, value,
+                               REPORTER_LOGS_UPLOADS_MAX);
+  }
+
+  void RecordLogsUploadResult() {
+    base::win::RegKey reporter_key;
+    DWORD logs_upload_result = 0;
+    if (reporter_key.Open(HKEY_CURRENT_USER, registry_key_.c_str(),
+                          KEY_QUERY_VALUE | KEY_SET_VALUE) != ERROR_SUCCESS ||
+        reporter_key.ReadValueDW(kLogsUploadResultValueName,
+                                 &logs_upload_result) != ERROR_SUCCESS) {
+      return;
+    }
+
+    RecordEnumerationHistogram(kLogsUploadResultMetricName,
+                               static_cast<Sample>(logs_upload_result),
+                               kSwReporterLogsUploadResultMax);
+    reporter_key.DeleteValue(kLogsUploadResultValueName);
+  }
+
  private:
   using Sample = base::HistogramBase::Sample;
 
   static constexpr base::HistogramBase::Flags kUmaHistogramFlag =
       base::HistogramBase::kUmaTargetedHistogramFlag;
 
   // Helper functions to record histograms with an optional suffix added to the
   // histogram name. The UMA_HISTOGRAM macros can't be used because they
   // require a constant string.
 
@@ skipping 336 matching lines @@
     if (g_testing_delegate_)
       g_testing_delegate_->NotifyLaunchReady();
 
     // Add switches for users who opted into extended Safe Browsing reporting.
     // The invocation object is changed locally right before the actual process
     // is launched because user status can change between this and the next run
     // for this ReporterRunner object. For example, the ReporterDone() callback
     // schedules the next run for a few days later, and the user might have
     // changed settings in the meantime.
     PrefService* local_state = g_browser_process->local_state();
-    if (next_invocation.flags & SwReporterInvocation::FLAG_SEND_REPORTER_LOGS &&
+    if (BehaviourIsEnabled(next_invocation.supported_behaviours,
+                           SwReporterBehaviours::SEND_REPORTER_LOGS) &&
         local_state && ShouldSendReporterLogs(*local_state)) {
+      next_invocation.logs_upload_enabled = true;
       AddSwitchesForExtendedReporterUser(&next_invocation);
       // Set the local state value before the first attempt to run the
       // reporter, because we only want to upload logs once in the window
       // defined by |kDaysBetweenReporterLogsSent|. If we set with other local
       // state values after the reporter runs, we could send logs again too
       // quickly (for example, if Chrome stops before the reporter finishes).
       local_state->SetInt64(prefs::kSwReporterLastTimeSentReport,
                             base::Time::Now().ToInternalValue());
     }
 
@@ skipping 42 matching lines @@
 
     // If the reporter failed to launch, do not process the results. (The exit
     // code itself doesn't need to be logged in this case because
     // SW_REPORTER_FAILED_TO_START is logged in |LaunchAndWaitForExit|.)
     if (exit_code == kReporterFailureExitCode)
       return;
 
     UMAHistogramReporter uma(finished_invocation.suffix);
     uma.ReportVersion(version);
     uma.ReportExitCode(exit_code);
-    uma.ReportFoundUwS(finished_invocation.flags &
-                       SwReporterInvocation::FLAG_LOG_TO_RAPPOR);
+    uma.ReportFoundUwS(
+        BehaviourIsEnabled(finished_invocation.supported_behaviours,
+                           SwReporterBehaviours::LOG_TO_RAPPOR));
 
     PrefService* local_state = g_browser_process->local_state();
     if (local_state) {
-      if (finished_invocation.flags &
-          SwReporterInvocation::FLAG_LOG_EXIT_CODE_TO_PREFS)
+      if (BehaviourIsEnabled(finished_invocation.supported_behaviours,
+                             SwReporterBehaviours::LOG_EXIT_CODE_TO_PREFS)) {
         local_state->SetInteger(prefs::kSwReporterLastExitCode, exit_code);
+      }
       local_state->SetInt64(prefs::kSwReporterLastTimeTriggered,
                             base::Time::Now().ToInternalValue());
     }
     uma.ReportRuntime(reporter_running_time);
     uma.ReportScanTimes();
     uma.ReportMemoryUsage();
+    if (finished_invocation.logs_upload_enabled)
+      uma.RecordLogsUploadResult();
 
-    if (!(finished_invocation.flags &
-          SwReporterInvocation::FLAG_TRIGGER_PROMPT))
+    if (!BehaviourIsEnabled(finished_invocation.supported_behaviours,
+                            SwReporterBehaviours::TRIGGER_PROMPT)) {
       return;
+    }
 
     if (!IsInSRTPromptFieldTrialGroups()) {
       // Knowing about disabled field trial is more important than reporter not
       // finding anything to remove, so check this case first.
       RecordReporterStepHistogram(SW_REPORTER_NO_PROMPT_FIELD_TRIAL);
       return;
     }
 
     if (exit_code != kSwReporterPostRebootCleanupNeeded &&
         exit_code != kSwReporterCleanupNeeded) {
@@ skipping 57 matching lines @@
           FROM_HERE,
           base::Bind(&ReporterRunner::TryToRun, base::Unretained(this)),
           next_trigger - now);
     }
   }
 
   // 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.
   bool ShouldSendReporterLogs(const PrefService& local_state) {
-    if (!base::FeatureList::IsEnabled(kSwReporterExtendedSafeBrowsingFeature) ||
-        !SafeBrowsingExtendedReportingEnabled()) {
+    if (!base::FeatureList::IsEnabled(kSwReporterExtendedSafeBrowsingFeature))
+      return false;
+
+    UMAHistogramReporter uma;
+    if (!SafeBrowsingExtendedReportingEnabled()) {
+      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));
-    // Send the logs if the last send was in the future. This 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)
+    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;
-    // Otherwise, send them if the interval has passed.
-    return last_time_sent_logs +
-               base::TimeDelta::FromDays(kDaysBetweenReporterLogsSent) <=
-           now;
+    }
+    uma.RecordLogsUploadEnabled(REPORTER_LOGS_UPLOADS_RECENTLY_SENT_LOGS);
+    return false;
   }
 
   void AddSwitchesForExtendedReporterUser(SwReporterInvocation* invocation) {
     invocation->command_line.AppendSwitch(kExtendedSafeBrowsingEnabledSwitch);
     invocation->command_line.AppendSwitchASCII(
         kChromeVersionSwitch, version_info::GetVersionNumber());
     invocation->command_line.AppendSwitchNative(
         kChromeChannelSwitch, base::IntToString16(ChannelAsInt()));
   }
 
@@ skipping 18 matching lines @@
   // A single leaky instance.
   static ReporterRunner* instance_;
 
   DISALLOW_COPY_AND_ASSIGN(ReporterRunner);
 };
 
 ReporterRunner* ReporterRunner::instance_ = nullptr;
 
 }  // namespace
 
+SwReporterBehaviours operator&(SwReporterBehaviours a, SwReporterBehaviours b) {
+  return static_cast<SwReporterBehaviours>(static_cast<uint8_t>(a) &
+                                           static_cast<uint8_t>(b));
+}
+
+SwReporterBehaviours operator|(SwReporterBehaviours a, SwReporterBehaviours b) {
+  return static_cast<SwReporterBehaviours>(static_cast<uint8_t>(a) |
+                                           static_cast<uint8_t>(b));
+}
+
+bool BehaviourIsEnabled(SwReporterBehaviours behaviours,
+                        SwReporterBehaviours intended_behaviour) {
+  return (behaviours & intended_behaviour) !=
+         SwReporterBehaviours::ALL_DISABLED;
+}
+
 SwReporterInvocation::SwReporterInvocation()
     : command_line(base::CommandLine::NO_PROGRAM) {}
 
 SwReporterInvocation SwReporterInvocation::FromFilePath(
     const base::FilePath& exe_path) {
   SwReporterInvocation invocation;
   invocation.command_line = base::CommandLine(exe_path);
   return invocation;
 }
 
 SwReporterInvocation SwReporterInvocation::FromCommandLine(
     const base::CommandLine& command_line) {
   SwReporterInvocation invocation;
   invocation.command_line = command_line;
   return invocation;
 }
 
 bool SwReporterInvocation::operator==(const SwReporterInvocation& other) const {
   return command_line.argv() == other.command_line.argv() &&
-         suffix == other.suffix && flags == other.flags;
+         suffix == other.suffix &&
+         supported_behaviours == other.supported_behaviours &&
+         logs_upload_enabled == other.logs_upload_enabled;
 }
 
 void RunSwReporters(const SwReporterQueue& invocations,
                     const base::Version& version,
                     scoped_refptr<base::TaskRunner> main_thread_task_runner,
                     scoped_refptr<base::TaskRunner> blocking_task_runner) {
   DCHECK(!invocations.empty());
   DCHECK(version.IsValid());
   ReporterRunner::ScheduleInvocations(invocations, version,
                                       std::move(main_thread_task_runner),
                                       std::move(blocking_task_runner));
 }
 
 bool ReporterFoundUws() {
   PrefService* local_state = g_browser_process->local_state();
   if (!local_state)
     return false;
   int exit_code = local_state->GetInteger(prefs::kSwReporterLastExitCode);
   return exit_code == kSwReporterCleanupNeeded;
 }
 
 bool UserHasRunCleaner() {
   base::string16 cleaner_key_path(kSoftwareRemovalToolRegistryKey);
   cleaner_key_path.append(L"\\").append(kCleanerSubKey);
 
-  base::win::RegKey srt_cleaner_key(HKEY_CURRENT_USER, cleaner_key_path.c_str(),
-                                    KEY_QUERY_VALUE);
-
-  return srt_cleaner_key.Valid() && srt_cleaner_key.GetValueCount() > 0;
+  base::win::RegKey srt_cleaner_key;
+  return srt_cleaner_key.Open(HKEY_CURRENT_USER, cleaner_key_path.c_str(),
+                              KEY_QUERY_VALUE) == ERROR_SUCCESS &&
+         srt_cleaner_key.GetValueCount() > 0;
 }
 
 void SetSwReporterTestingDelegate(SwReporterTestingDelegate* delegate) {
   g_testing_delegate_ = delegate;
 }
 
 }  // namespace safe_browsing