Reland "Deprecate uninstall_metrics."

This is a reland of e15311e1

Original change's description:
> Deprecate uninstall_metrics.
>
> Remove the tracking/persisting of these metrics in prefs. Removed
> uploading in uninstaller.
>
> Bug: 1107516
> Change-Id: I84499820cea86b5f785949d9274e8b2205714aae
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2308133
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Reviewed-by: Nate Fischer <ntfschr@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Commit-Queue: Robert Kaplow <rkaplow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#825461}

Bug: 1107516
Change-Id: Id7a71c3b8d64dbe9984b2f7f33808207071550f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2528581Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Commit-Queue: Denis Kuznetsov [CET] <antrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826222}

android_webview/browser/aw_feature_list_creator.cc

@@ -72,7 +72,6 @@ const char* const kPersistentPrefsAllowlist[] = {
     metrics::prefs::kStabilityPageLoadCount,
     metrics::prefs::kStabilityRendererHangCount,
     metrics::prefs::kStabilityRendererLaunchCount,
-    metrics::prefs::kUninstallMetricsPageLoadCount,
     // Unsent logs.
     metrics::prefs::kMetricsInitialLogs,
     metrics::prefs::kMetricsOngoingLogs,

chrome/browser/ui/webui/local_state/local_state_ui.cc

@@ -69,8 +69,8 @@ void LocalStateUIHandler::HandleRequestJson(const base::ListValue* args) {
       g_browser_process->local_state()->GetPreferenceValues(
           PrefService::EXCLUDE_DEFAULTS);
   if (ENABLE_FILTERING) {
-    std::vector<std::string> allowlisted_prefixes = {
-        "variations", "user_experience_metrics", "uninstall_metrics"};
+    std::vector<std::string> allowlisted_prefixes = {"variations",
+                                                     "user_experience_metrics"};
     internal::FilterPrefs(allowlisted_prefixes, local_state_values);
   }
   std::string json;

chrome/installer/setup/BUILD.gn

@@ -78,8 +78,6 @@ if (is_win) {
       "setup_singleton.h",
       "setup_util.cc",
       "setup_util.h",
-      "uninstall_metrics.cc",
-      "uninstall_metrics.h",
       "update_active_setup_version_work_item.cc",
       "update_active_setup_version_work_item.h",
       "user_experiment.cc",
@@ -133,7 +131,6 @@ if (is_win) {
       "setup_singleton_unittest.cc",
       "setup_util_unittest.cc",
       "setup_util_unittest.h",
-      "uninstall_metrics_unittest.cc",
       "update_active_setup_version_work_item_unittest.cc",
       "user_experiment_unittest.cc",
       "user_hive_visitor_unittest.cc",

chrome/installer/setup/google_chrome_behaviors.cc

@@ -10,21 +10,24 @@
 #include <memory>
 
 #include "base/files/file_path.h"
+#include "base/json/json_file_value_serializer.h"
 #include "base/logging.h"
 #include "base/path_service.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
 #include "base/version.h"
 #include "base/win/registry.h"
 #include "base/win/windows_version.h"
 #include "base/win/wmi.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/install_static/install_util.h"
-#include "chrome/installer/setup/uninstall_metrics.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome/installer/util/install_util.h"
+#include "components/metrics/metrics_pref_names.h"
 #include "third_party/crashpad/crashpad/client/crash_report_database.h"
 #include "third_party/crashpad/crashpad/client/settings.h"
 #include "third_party/crashpad/crashpad/util/misc/uuid.h"
@@ -79,6 +82,27 @@ void NavigateToUrlWithIExplore(const base::string16& url) {
   base::win::WmiLaunchProcess(command, &pid);
 }
 
+// Returns true if the prefs dictionary located at |local_data_path| contains
+// an enabled metrics pref.
+// Note: Due to crbug.com/1052816, it is possible a subset of users may return
+// false here even though they send UMA, as UMA can also check the registry.
+void IsMetricsEnabled(const base::FilePath& local_data_path) {
+  JSONFileValueDeserializer json_deserializer(file_path);
+
+  std::unique_ptr<base::Value> root =
+      json_deserializer.Deserialize(nullptr, nullptr);
+  // Preferences should always have a dictionary root.
+  if (!root || !root->is_dict())
+    return false;
+
+  auto path =
+      base::SplitStringPiece(metrics::prefs::kMetricsReportingEnabled, ".",
+                             base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  const auto* value = root.FindPathOfType(path, base::Value::Type::BOOLEAN);
+
+  return value && value->GetBool();
+}
+
 }  // namespace
 
 // If |archive_type| is INCREMENTAL_ARCHIVE_TYPE and |install_status| does not
@@ -161,8 +185,7 @@ base::string16 GetDistributionData() {
 // - crversion: the version of Chrome being uninstalled
 // - os: Major.Minor.Build of the OS version
 // If the user is sending crash reports and usage statistics to Google, the
-// uninstall metrics read from |local_data_path| and the query params in
-// |distribution_data| are included in the URL.
+// query params in |distribution_data| are included in the URL.
 void DoPostUninstallOperations(const base::Version& version,
                                const base::FilePath& local_data_path,
                                const base::string16& distribution_data) {
@@ -190,18 +213,9 @@ void DoPostUninstallOperations(const base::Version& version,
                                 base::ASCIIToUTF16(version.GetString()).c_str(),
                                 os_version.c_str());
 
-  base::string16 uninstall_metrics;
-  if (ExtractUninstallMetricsFromFile(local_data_path, &uninstall_metrics)) {
-    DCHECK_EQ(uninstall_metrics.front(), L'&');
-    DCHECK_NE(uninstall_metrics.back(), L'&');
-    DCHECK_EQ(uninstall_metrics.find(L'?'), base::string16::npos);
-    // The user has opted into anonymous usage data collection, so append
-    // metrics and distribution data.
-    url += uninstall_metrics;
-    if (!distribution_data.empty()) {
-      url += L"&";
-      url += distribution_data;
-    }
+  if (!distribution_data.empty() && IsMetricsEnabled(local_data_path)) {
+    url += L"&";
+    url += distribution_data;
   }
 
   if (os_info->version() < base::win::Version::WIN10 ||

chrome/installer/setup/setup_constants.cc

@@ -19,9 +19,6 @@ const wchar_t kInstallSourceChromeDir[] = L"Chrome-bin";
 const wchar_t kMediaPlayerRegPath[] =
     L"Software\\Microsoft\\MediaPlayer\\ShimInclusionList";
 
-// Local State preference names.
-const char kUninstallMetricsName[] = "uninstall_metrics";
-
 const char kCourgette[] = "courgette";
 const char kBsdiff[] = "bsdiff";
 #if BUILDFLAG(ZUCCHINI)

chrome/installer/setup/setup_constants.h

@@ -21,8 +21,6 @@ extern const wchar_t kInstallSourceChromeDir[];
 
 extern const wchar_t kMediaPlayerRegPath[];
 
-extern const char kUninstallMetricsName[];
-
 // The range of error values among the installer, Courgette, BSDiff and
 // Zucchini overlap. These offset values disambiguate between different sets
 // of errors by shifting the values up with the specified offset.

chrome/installer/setup/uninstall_metrics.cc

-// Copyright 2013 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/installer/setup/uninstall_metrics.h"
-
-#include <memory>
-#include <string>
-
-#include "base/check.h"
-#include "base/files/file_path.h"
-#include "base/json/json_file_value_serializer.h"
-#include "base/notreached.h"
-#include "base/strings/string_split.h"
-#include "base/strings/utf_string_conversions.h"
-#include "base/values.h"
-#include "chrome/common/pref_names.h"
-#include "chrome/installer/setup/setup_constants.h"
-#include "components/metrics/metrics_pref_names.h"
-
-namespace installer {
-
-namespace {
-
-// Appends an URL query parameter to |metrics| for each item in
-// |uninstall_metrics_dict|. Returns true if |metrics| was modified; otherwise,
-// false.
-bool BuildUninstallMetricsString(const base::Value& uninstall_metrics_dict,
-                                 base::string16* metrics) {
-  DCHECK(uninstall_metrics_dict.is_dict());
-  DCHECK(metrics);
-  bool has_values = false;
-
-  for (const auto& item : uninstall_metrics_dict.DictItems()) {
-    has_values = true;
-    metrics->push_back(L'&');
-    metrics->append(base::UTF8ToWide(item.first));
-    metrics->push_back(L'=');
-
-    if (item.second.is_string())
-      metrics->append(base::UTF8ToWide(item.second.GetString()));
-    else
-      NOTREACHED() << item.second.type();
-  }
-
-  return has_values;
-}
-
-}  // namespace
-
-bool ExtractUninstallMetrics(const base::Value& root,
-                             base::string16* uninstall_metrics_string) {
-  DCHECK(root.is_dict());
-  // Make sure that the user wants us reporting metrics. If not, don't add our
-  // uninstall metrics.
-  auto path =
-      base::SplitStringPiece(metrics::prefs::kMetricsReportingEnabled, ".",
-                             base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-  const auto* value = root.FindPathOfType(path, base::Value::Type::BOOLEAN);
-  if (!value || !value->GetBool())
-    return false;
-
-  value = root.FindKeyOfType(installer::kUninstallMetricsName,
-                             base::Value::Type::DICTIONARY);
-  if (!value)
-    return false;
-
-  return BuildUninstallMetricsString(*value, uninstall_metrics_string);
-}
-
-bool ExtractUninstallMetricsFromFile(const base::FilePath& file_path,
-                                     base::string16* uninstall_metrics_string) {
-  JSONFileValueDeserializer json_deserializer(file_path);
-
-  std::string json_error_string;
-  std::unique_ptr<base::Value> root =
-      json_deserializer.Deserialize(nullptr, nullptr);
-  if (!root)
-    return false;
-
-  // Preferences should always have a dictionary root.
-  if (!root->is_dict())
-    return false;
-
-  return ExtractUninstallMetrics(*root, uninstall_metrics_string);
-}
-
-}  // namespace installer

chrome/installer/setup/uninstall_metrics.h

-// Copyright 2013 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.
-
-#ifndef CHROME_INSTALLER_SETUP_UNINSTALL_METRICS_H_
-#define CHROME_INSTALLER_SETUP_UNINSTALL_METRICS_H_
-
-#include "base/strings/string16.h"
-
-namespace base {
-class FilePath;
-class Value;
-}  // namespace base
-
-namespace installer {
-
-// Extracts uninstall metrics from the given JSON value.
-bool ExtractUninstallMetrics(const base::Value& root,
-                             base::string16* uninstall_metrics);
-
-// Extracts uninstall metrics from the JSON file located at file_path.
-// Returns them in a form suitable for appending to a url that already
-// has GET parameters, i.e. &metric1=foo&metric2=bar.
-// Returns true if uninstall_metrics has been successfully populated with
-// the uninstall metrics, false otherwise.
-bool ExtractUninstallMetricsFromFile(const base::FilePath& file_path,
-                                     base::string16* uninstall_metrics);
-
-}  // namespace installer
-
-#endif  // CHROME_INSTALLER_SETUP_UNINSTALL_METRICS_H_

chrome/installer/setup/uninstall_metrics_unittest.cc

-// Copyright 2013 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/installer/setup/uninstall_metrics.h"
-
-#include <memory>
-#include <string>
-
-#include "base/json/json_string_value_serializer.h"
-#include "base/strings/string16.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace installer {
-
-TEST(UninstallMetricsTest, TestExtractUninstallMetrics) {
-  // A make-believe JSON preferences file.
-  std::string pref_string(
-      "{ \n"
-      "  \"foo\": \"bar\",\n"
-      "  \"uninstall_metrics\": { \n"
-      "    \"last_launch_time_sec\": \"1235341118\","
-      "    \"last_observed_running_time_sec\": \"1235341183\","
-      "    \"launch_count\": \"11\","
-      "    \"page_load_count\": \"68\","
-      "    \"uptime_sec\": \"809\","
-      "    \"installation_date2\": \"1235341141\"\n"
-      "  },\n"
-      "  \"blah\": {\n"
-      "    \"this_sentence_is_true\": false\n"
-      "  },\n"
-      "  \"user_experience_metrics\": { \n"
-      "    \"client_id_timestamp\": \"1234567890\","
-      "    \"reporting_enabled\": true\n"
-      "  }\n"
-      "} \n");
-
-  // The URL string we expect to be generated from said make-believe file.
-  base::string16 expected_url_string(
-      L"&installation_date2=1235341141"
-      L"&last_launch_time_sec=1235341118"
-      L"&last_observed_running_time_sec=1235341183"
-      L"&launch_count=11&page_load_count=68"
-      L"&uptime_sec=809");
-
-  JSONStringValueDeserializer json_deserializer(pref_string);
-  std::string error_message;
-
-  std::unique_ptr<base::Value> root =
-      json_deserializer.Deserialize(nullptr, &error_message);
-  ASSERT_TRUE(root.get());
-  base::string16 uninstall_metrics_string;
-
-  EXPECT_TRUE(ExtractUninstallMetrics(*root, &uninstall_metrics_string));
-  EXPECT_EQ(expected_url_string, uninstall_metrics_string);
-}
-
-}  // namespace installer

components/metrics/metrics_pref_names.cc

@@ -9,6 +9,7 @@ namespace prefs {
 
 // Set once, to the current epoch time, on the first run of chrome on this
 // machine. Attached to metrics reports forever thereafter.
+// Note: the 'uninstall_metrics' name is a legacy name and doesn't mean much.
 const char kInstallDate[] = "uninstall_metrics.installation_date2";
 
 // The metrics client GUID.
@@ -223,14 +224,6 @@ const char kStabilityStatsVersion[] =
 const char kStabilitySystemCrashCount[] =
     "user_experience_metrics.stability.system_crash_count";
 
-// The keys below are strictly increasing counters over the lifetime of
-// a chrome installation. They are (optionally) sent up to the uninstall
-// survey in the event of uninstallation.
-const char kUninstallLaunchCount[] = "uninstall_metrics.launch_count";
-const char kUninstallMetricsPageLoadCount[] =
-    "uninstall_metrics.page_load_count";
-const char kUninstallMetricsUptimeSec[] = "uninstall_metrics.uptime_sec";
-
 // Dictionary for measuring cellular data used by UKM service during last 7
 // days.
 const char kUkmCellDataUse[] = "user_experience_metrics.ukm_cell_datause";

components/metrics/metrics_pref_names.h

@@ -64,11 +64,6 @@ extern const char kStabilityStatsBuildTime[];
 extern const char kStabilityStatsVersion[];
 extern const char kStabilitySystemCrashCount[];
 
-// Preferences for generating metrics at uninstall time.
-extern const char kUninstallLaunchCount[];
-extern const char kUninstallMetricsPageLoadCount[];
-extern const char kUninstallMetricsUptimeSec[];
-
 // For measuring data use for throttling UMA log uploads on cellular.
 extern const char kUkmCellDataUse[];
 extern const char kUmaCellDataUse[];

components/metrics/metrics_service.cc

@@ -202,9 +202,6 @@ void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) {
   MetricsReportingService::RegisterPrefs(registry);
 
   registry->RegisterIntegerPref(prefs::kMetricsSessionID, -1);
-
-  registry->RegisterInt64Pref(prefs::kUninstallLaunchCount, 0);
-  registry->RegisterInt64Pref(prefs::kUninstallMetricsUptimeSec, 0);
 }
 
 MetricsService::MetricsService(MetricsStateManager* state_manager,
@@ -546,9 +543,6 @@ void MetricsService::InitializeMetricsState() {
   base::TimeDelta startup_uptime;
   GetUptimes(local_state_, &startup_uptime, &ignored_uptime_parameter);
   DCHECK_EQ(0, startup_uptime.InMicroseconds());
-
-  // Bookkeeping for the uninstall metrics.
-  IncrementLongPrefsValue(prefs::kUninstallLaunchCount);
 }
 
 void MetricsService::OnUserAction(const std::string& action,
@@ -583,13 +577,6 @@ void MetricsService::GetUptimes(PrefService* pref,
   *incremental_uptime = now - last_updated_time_;
   *uptime = now - first_updated_time_;
   last_updated_time_ = now;
-
-  const int64_t incremental_time_secs = incremental_uptime->InSeconds();
-  if (incremental_time_secs > 0) {
-    int64_t metrics_uptime = pref->GetInt64(prefs::kUninstallMetricsUptimeSec);
-    metrics_uptime += incremental_time_secs;
-    pref->SetInt64(prefs::kUninstallMetricsUptimeSec, metrics_uptime);
-  }
 }
 
 //------------------------------------------------------------------------------

components/metrics/stability_metrics_helper.cc

@@ -147,8 +147,7 @@ void StabilityMetricsHelper::ProvideStabilityMetrics(
 }
 
 void StabilityMetricsHelper::ClearSavedStabilityMetrics() {
-  // Clear all the prefs used in this class in UMA reports (which doesn't
-  // include |kUninstallMetricsPageLoadCount| as it's not sent up by UMA).
+  // Clear all the prefs used in this class in UMA reports.
   local_state_->SetInteger(prefs::kStabilityChildProcessCrashCount, 0);
   local_state_->SetInteger(prefs::kStabilityExtensionRendererCrashCount, 0);
   local_state_->SetInteger(prefs::kStabilityExtensionRendererFailedLaunchCount,
@@ -177,8 +176,6 @@ void StabilityMetricsHelper::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterIntegerPref(prefs::kStabilityRendererFailedLaunchCount, 0);
   registry->RegisterIntegerPref(prefs::kStabilityRendererHangCount, 0);
   registry->RegisterIntegerPref(prefs::kStabilityRendererLaunchCount, 0);
-
-  registry->RegisterInt64Pref(prefs::kUninstallMetricsPageLoadCount, 0);
 }
 
 void StabilityMetricsHelper::IncreaseRendererCrashCount() {
@@ -211,7 +208,6 @@ void StabilityMetricsHelper::BrowserChildProcessCrashed() {
 
 void StabilityMetricsHelper::LogLoadStarted() {
   IncrementPrefValue(prefs::kStabilityPageLoadCount);
-  IncrementLongPrefsValue(prefs::kUninstallMetricsPageLoadCount);
   RecordStabilityEvent(StabilityEventType::kPageLoad);
 }