BrowsingData: Add per-task histograms for data deletion

Help debugging deletion regressions by recording per task duration
histograms.

Bug: 1312955
Change-Id: Iba3a3977fa9820137eaf3a2bc4e7829700f8ba42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3571775
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#990309}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -19,8 +19,10 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
+#include "base/strings/strcat.h"
 #include "base/task/bind_post_task.h"
 #include "base/task/thread_pool.h"
+#include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
@@ -1231,6 +1233,7 @@ void ChromeBrowsingDataRemoverDelegate::OnTaskStarted(
 void ChromeBrowsingDataRemoverDelegate::OnTaskComplete(
     TracingDataType data_type,
     uint64_t data_type_mask,
+    base::TimeTicks started,
     bool success) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   size_t num_erased = pending_sub_tasks_.erase(data_type);
@@ -1240,6 +1243,10 @@ void ChromeBrowsingDataRemoverDelegate::OnTaskComplete(
       TRACE_ID_WITH_SCOPE("ChromeBrowsingDataRemoverDelegate",
                           static_cast<int>(data_type)),
       "data_type", static_cast<int>(data_type));
+  base::UmaHistogramMediumTimes(
+      base::StrCat({"History.ClearBrowsingData.Duration.ChromeTask.",
+                    GetHistogramSuffix(data_type)}),
+      base::TimeTicks::Now() - started);
 
   if (!success) {
     base::UmaHistogramEnumeration("History.ClearBrowsingData.FailedTasksChrome",
@@ -1269,6 +1276,92 @@ void ChromeBrowsingDataRemoverDelegate::OnTaskComplete(
   std::move(callback_).Run(failed_data_types_);
 }
 
+const char* ChromeBrowsingDataRemoverDelegate::GetHistogramSuffix(
+    TracingDataType task) {
+  switch (task) {
+    case TracingDataType::kSynchronous:
+      return "Synchronous";
+    case TracingDataType::kHistory:
+      return "History";
+    case TracingDataType::kHostNameResolution:
+      return "HostNameResolution";
+    case TracingDataType::kNaclCache:
+      return "NaclCache";
+    case TracingDataType::kPnaclCache:
+      return "PnaclCache";
+    case TracingDataType::kAutofillData:
+      return "AutofillData";
+    case TracingDataType::kAutofillOrigins:
+      return "AutofillOrigins";
+    case TracingDataType::kPluginData:
+      return "PluginData";
+    case TracingDataType::kDomainReliability:
+      return "DomainReliability";
+    case TracingDataType::kNetworkPredictor:
+      return "NetworkPredictor";
+    case TracingDataType::kWebrtcLogs:
+      return "WebrtcLogs";
+    case TracingDataType::kVideoDecodeHistory:
+      return "VideoDecodeHistory";
+    case TracingDataType::kCookies:
+      return "Cookies";
+    case TracingDataType::kPasswords:
+      return "Passwords";
+    case TracingDataType::kHttpAuthCache:
+      return "HttpAuthCache";
+    case TracingDataType::kDisableAutoSignin:
+      return "DisableAutoSignin";
+    case TracingDataType::kPasswordsStatistics:
+      return "PasswordsStatistics";
+    case TracingDataType::kKeywordsModel:
+      return "KeywordsModel";
+    case TracingDataType::kReportingCache:
+      return "ReportingCache";
+    case TracingDataType::kNetworkErrorLogging:
+      return "NetworkErrorLogging";
+    case TracingDataType::kFlashDeauthorization:
+      return "FlashDeauthorization";
+    case TracingDataType::kOfflinePages:
+      return "OfflinePages";
+    case TracingDataType::kExploreSites:
+      return "ExploreSites";
+    case TracingDataType::kLegacyStrikes:
+      return "LegacyStrikes";
+    case TracingDataType::kWebrtcEventLogs:
+      return "WebrtcEventLogs";
+    case TracingDataType::kCdmLicenses:
+      return "CdmLicenses";
+    case TracingDataType::kHostCache:
+      return "HostCache";
+    case TracingDataType::kTpmAttestationKeys:
+      return "TpmAttestationKeys";
+    case TracingDataType::kStrikes:
+      return "Strikes";
+    case TracingDataType::kFieldInfo:
+      return "FieldInfo";
+    case TracingDataType::kCompromisedCredentials:
+      return "CompromisedCredentials";
+    case TracingDataType::kUserDataSnapshot:
+      return "UserDataSnapshot";
+    case TracingDataType::kMediaFeeds:
+      return "MediaFeeds";
+    case TracingDataType::kAccountPasswords:
+      return "AccountPasswords";
+    case TracingDataType::kAccountPasswordsSynced:
+      return "AccountPasswordsSynced";
+    case TracingDataType::kAccountCompromisedCredentials:
+      return "AccountCompromisedCredentials";
+    case TracingDataType::kFaviconCacheExpiration:
+      return "FaviconCacheExpiration";
+    case TracingDataType::kSecurePaymentConfirmationCredentials:
+      return "SecurePaymentConfirmationCredentials";
+    case TracingDataType::kWebAppHistory:
+      return "WebAppHistory";
+    case TracingDataType::kWebAuthnCredentials:
+      return "WebAuthnCredentials";
+  }
+}
+
 void ChromeBrowsingDataRemoverDelegate::OnStartRemoving() {
   profile_keep_alive_ = std::make_unique<ScopedProfileKeepAlive>(
       profile_->GetOriginalProfile(),
@@ -1294,7 +1387,7 @@ ChromeBrowsingDataRemoverDelegate::CreateTaskCompletionCallback(
   OnTaskStarted(data_type);
   return base::BindOnce(&ChromeBrowsingDataRemoverDelegate::OnTaskComplete,
                         weak_ptr_factory_.GetWeakPtr(), data_type,
-                        data_type_mask);
+                        data_type_mask, base::TimeTicks::Now());
 }
 
 base::OnceClosure
@@ -1307,7 +1400,8 @@ ChromeBrowsingDataRemoverDelegate::CreateTaskCompletionClosureForMojo(
       CreateTaskCompletionClosure(data_type),
       base::BindOnce(&ChromeBrowsingDataRemoverDelegate::OnTaskComplete,
                      weak_ptr_factory_.GetWeakPtr(), data_type,
-                     /*data_type_mask=*/0, /*success=*/true));
+                     /*data_type_mask=*/0, base::TimeTicks::Now(),
+                     /*success=*/true));
 }
 
 void ChromeBrowsingDataRemoverDelegate::RecordUnfinishedSubTasks() {

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h

@@ -99,7 +99,9 @@ class ChromeBrowsingDataRemoverDelegate
 
   // For debugging purposes. Please add new deletion tasks at the end.
   // This enum is recorded in a histogram, so don't change or reuse ids.
-  // Entries must also be added to ChromeBrowsingDataRemoverTasks in enums.xml.
+  // Entries must also be added to ChromeBrowsingDataRemoverTasks in enums.xml
+  // and History.ClearBrowsingData.Duration.ChromeTask.{Task}
+  // in histograms/metadata/history/histograms.xml.
   enum class TracingDataType {
     kSynchronous = 1,
     kHistory = 2,
@@ -109,7 +111,7 @@ class ChromeBrowsingDataRemoverDelegate
     kAutofillData = 6,
     kAutofillOrigins = 7,
     kPluginData = 8,
-    kFlashLsoHelper = 9,  // deprecated
+    // kFlashLsoHelper = 9, deprecated
     kDomainReliability = 10,
     kNetworkPredictor = 11,
     kWebrtcLogs = 12,
@@ -124,15 +126,15 @@ class ChromeBrowsingDataRemoverDelegate
     kNetworkErrorLogging = 21,
     kFlashDeauthorization = 22,
     kOfflinePages = 23,
-    kPrecache = 24,  // deprecated
+    // kPrecache = 24, deprecated
     kExploreSites = 25,
     kLegacyStrikes = 26,
     kWebrtcEventLogs = 27,
     kCdmLicenses = 28,
     kHostCache = 29,
     kTpmAttestationKeys = 30,
     kStrikes = 31,
-    kLeakedCredentials = 32,  // deprecated
+    // kLeakedCredentials = 32, deprecated
     kFieldInfo = 33,
     kCompromisedCredentials = 34,
     kUserDataSnapshot = 35,
@@ -144,16 +146,25 @@ class ChromeBrowsingDataRemoverDelegate
     kSecurePaymentConfirmationCredentials = 41,
     kWebAppHistory = 42,
     kWebAuthnCredentials = 43,
+
+    // Please update ChromeBrowsingDataRemoverTasks in enums.xml and
+    // History.ClearBrowsingData.Duration.ChromeTask.{Task}
+    // in histograms/metadata/history/histograms.xml when adding entries!
     kMaxValue = kWebAuthnCredentials,
   };
 
+  // Returns the suffix for the
+  // History.ClearBrowsingData.Duration.ChromeTask.{Task} histogram
+  const char* GetHistogramSuffix(TracingDataType task);
+
   // Called by CreateTaskCompletionClosure().
   void OnTaskStarted(TracingDataType data_type);
 
   // Called by the closures returned by CreateTaskCompletionClosure().
   // Checks if all tasks have completed, and if so, calls callback_.
   void OnTaskComplete(TracingDataType data_type,
                       uint64_t data_type_mask,
+                      base::TimeTicks started,
                       bool success);
 
   // Increments the number of pending tasks by one, and returns a OnceClosure

content/browser/browsing_data/browsing_data_remover_impl.cc

@@ -19,6 +19,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/observer_list.h"
+#include "base/strings/strcat.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
@@ -721,7 +722,8 @@ void BrowsingDataRemoverImpl::Notify() {
       base::BindOnce(&BrowsingDataRemoverImpl::RunNextTask, GetWeakPtr()));
 }
 
-void BrowsingDataRemoverImpl::OnTaskComplete(TracingDataType data_type) {
+void BrowsingDataRemoverImpl::OnTaskComplete(TracingDataType data_type,
+                                             base::TimeTicks started) {
   // TODO(brettw) http://crbug.com/305259: This should also observe session
   // clearing (what about other things such as passwords, etc.?) and wait for
   // them to complete before continuing.
@@ -734,6 +736,12 @@ void BrowsingDataRemoverImpl::OnTaskComplete(TracingDataType data_type) {
       TRACE_ID_WITH_SCOPE("BrowsingDataRemoverImpl",
                           static_cast<int>(data_type)),
       "data_type", static_cast<int>(data_type));
+
+  base::UmaHistogramMediumTimes(
+      base::StrCat({"History.ClearBrowsingData.Duration.Task.",
+                    GetHistogramSuffix(data_type)}),
+      base::TimeTicks::Now() - started);
+
   if (!pending_sub_tasks_.empty())
     return;
 
@@ -778,6 +786,39 @@ void BrowsingDataRemoverImpl::OnTaskComplete(TracingDataType data_type) {
   Notify();
 }
 
+const char* BrowsingDataRemoverImpl::GetHistogramSuffix(TracingDataType task) {
+  switch (task) {
+    case TracingDataType::kSynchronous:
+      return "Synchronous";
+    case TracingDataType::kEmbedderData:
+      return "EmbedderData";
+    case TracingDataType::kStoragePartition:
+      return "StoragePartition";
+    case TracingDataType::kHttpCache:
+      return "HttpCache";
+    case TracingDataType::kHttpAndMediaCaches:
+      return "HttpAndMediaCaches";
+    case TracingDataType::kReportingCache:
+      return "ReportingCache";
+    case TracingDataType::kChannelIds:
+      return "ChannelIds";
+    case TracingDataType::kNetworkHistory:
+      return "NetworkHistory";
+    case TracingDataType::kAuthCache:
+      return "AuthCache";
+    case TracingDataType::kCodeCaches:
+      return "CodeCaches";
+    case TracingDataType::kNetworkErrorLogging:
+      return "NetworkErrorLogging";
+    case TracingDataType::kTrustTokens:
+      return "TrustTokens";
+    case TracingDataType::kConversions:
+      return "Conversions";
+    case TracingDataType::kDeferredCookies:
+      return "DeferredCookies";
+  }
+}
+
 base::OnceClosure BrowsingDataRemoverImpl::CreateTaskCompletionClosure(
     TracingDataType data_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -790,7 +831,7 @@ base::OnceClosure BrowsingDataRemoverImpl::CreateTaskCompletionClosure(
                           static_cast<int>(data_type)),
       "data_type", static_cast<int>(data_type));
   return base::BindOnce(&BrowsingDataRemoverImpl::OnTaskComplete, GetWeakPtr(),
-                        data_type);
+                        data_type, base::TimeTicks::Now());
 }
 
 base::OnceClosure BrowsingDataRemoverImpl::CreateTaskCompletionClosureForMojo(
@@ -799,7 +840,7 @@ base::OnceClosure BrowsingDataRemoverImpl::CreateTaskCompletionClosureForMojo(
   return RunsOrPostOnCurrentTaskRunner(mojo::WrapCallbackWithDropHandler(
       CreateTaskCompletionClosure(data_type),
       base::BindOnce(&BrowsingDataRemoverImpl::OnTaskComplete, GetWeakPtr(),
-                     data_type)));
+                     data_type, base::TimeTicks::Now())));
 }
 
 void BrowsingDataRemoverImpl::RecordUnfinishedSubTasks() {

content/browser/browsing_data/browsing_data_remover_impl.h

@@ -104,7 +104,9 @@ class CONTENT_EXPORT BrowsingDataRemoverImpl
 
   // For debugging purposes. Please add new deletion tasks at the end.
   // This enum is recorded in a histogram, so don't change or reuse ids.
-  // Entries must also be added to BrowsingDataRemoverTasks in enums.xml.
+  // Entries must also be added to BrowsingDataRemoverTasks in enums.xml and
+  // History.ClearBrowsingData.Duration.Task.{Task} in
+  // histograms/metadata/history/histograms.xml.
   enum class TracingDataType {
     kSynchronous = 1,
     kEmbedderData = 2,
@@ -123,6 +125,10 @@ class CONTENT_EXPORT BrowsingDataRemoverImpl
     kMaxValue = kDeferredCookies,
   };
 
+  // Returns the suffix for the History.ClearBrowsingData.Duration.Task.{Task}
+  // histogram
+  const char* GetHistogramSuffix(TracingDataType task);
+
   // Represents a single removal task. Contains all parameters needed to execute
   // it and a pointer to the observer that added it. CONTENT_EXPORTed to be
   // visible in tests.
@@ -179,7 +185,7 @@ class CONTENT_EXPORT BrowsingDataRemoverImpl
 
   // Called by the closures returned by CreateTaskCompletionClosure().
   // Checks if all tasks have completed, and if so, calls Notify().
-  void OnTaskComplete(TracingDataType data_type);
+  void OnTaskComplete(TracingDataType data_type, base::TimeTicks started);
 
   // Increments the number of pending tasks by one, and returns a OnceClosure
   // that calls OnTaskComplete(). The Remover is complete once all the closures

tools/metrics/histograms/enums.xml

@@ -13137,6 +13137,7 @@ Called by update_protocol_commands_enum.py.-->
   <int value="40" label="kFaviconCacheExpiration"/>
   <int value="41" label="kSecurePaymentConfirmationInstruments"/>
   <int value="42" label="kWebAppHistory"/>
+  <int value="43" label="kWebAuthnCredentials"/>
 </enum>
 
 <enum name="ChromeChannelForHistogram">