[webrtc] Clear cache at USER_VISIBLE priority.

As described in
https://docs.google.com/document/d/1va_h2uei_-H-AjXMSxdSOat2vkwD4FKREn_CK-wRZes/edit?usp=sharing,
clearing the cache in Lighthouse can be slow.

This CL fixes the problem by increasing the priority of the
WebRtcEventLogManager TaskRunner to USER_BLOCKING when the cache is
being cleared. This is the correct priority for an operation on which
the user is waiting.

Change-Id: If5aea24d2388dd67ecbaead8255886905202daba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1882029Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710323}

chrome/browser/media/webrtc/webrtc_event_log_manager.cc

@@ -181,9 +181,10 @@ base::FilePath WebRtcEventLogManager::GetRemoteBoundWebRtcEventLogsDir(
 }
 
 WebRtcEventLogManager::WebRtcEventLogManager()
-    : task_runner_(base::CreateSequencedTaskRunner(
+    : task_runner_(base::CreateUpdateableSequencedTaskRunner(
           {base::ThreadPool(), base::MayBlock(),
            base::TaskPriority::BEST_EFFORT,
+           base::ThreadPolicy::PREFER_BACKGROUND,
            base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})),
       remote_logging_feature_enabled_(IsRemoteLoggingFeatureEnabled()),
       local_logs_observer_(nullptr),
@@ -481,6 +482,11 @@ void WebRtcEventLogManager::ClearCacheForBrowserContext(
   const auto browser_context_id = GetBrowserContextId(browser_context);
   DCHECK_NE(browser_context_id, kNullBrowserContextId);
 
+  // |task_runner_| is USER_BLOCKING when there are pending tasks to clear the
+  // cache.
+  ++num_pending_clear_cache_;
+  task_runner_->UpdatePriority(base::TaskPriority::USER_BLOCKING);
+
   // |this| is destroyed by ~BrowserProcessImpl(), so base::Unretained(this)
   // will not be dereferenced after destruction.
   task_runner_->PostTaskAndReply(
@@ -488,7 +494,9 @@ void WebRtcEventLogManager::ClearCacheForBrowserContext(
       base::BindOnce(
           &WebRtcEventLogManager::ClearCacheForBrowserContextInternal,
           base::Unretained(this), browser_context_id, delete_begin, delete_end),
-      std::move(reply));
+      base::BindOnce(
+          &WebRtcEventLogManager::OnClearCacheForBrowserContextDoneInternal,
+          base::Unretained(this), std::move(reply)));
 }
 
 void WebRtcEventLogManager::GetHistory(
@@ -940,6 +948,18 @@ void WebRtcEventLogManager::ClearCacheForBrowserContextInternal(
                                                    delete_begin, delete_end);
 }
 
+void WebRtcEventLogManager::OnClearCacheForBrowserContextDoneInternal(
+    base::OnceClosure reply) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_GT(num_pending_clear_cache_, 0);
+  // |task_runner_| is BEST_EFFORT when there are no pending tasks to clear the
+  // cache.
+  --num_pending_clear_cache_;
+  if (num_pending_clear_cache_ == 0)
+    task_runner_->UpdatePriority(base::TaskPriority::BEST_EFFORT);
+  std::move(reply).Run();
+}
+
 void WebRtcEventLogManager::GetHistoryInternal(
     BrowserContextId browser_context_id,
     base::OnceCallback<void(const std::vector<UploadList::UploadInfo>&)>
@@ -1062,7 +1082,7 @@ void WebRtcEventLogManager::UploadConditionsHoldForTesting(
           base::Unretained(&remote_logs_manager_), std::move(callback)));
 }
 
-scoped_refptr<base::SequencedTaskRunner>&
+scoped_refptr<base::SequencedTaskRunner>
 WebRtcEventLogManager::GetTaskRunnerForTesting() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return task_runner_;

chrome/browser/media/webrtc/webrtc_event_log_manager.h

@@ -14,9 +14,9 @@
 #include "base/containers/flat_set.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_refptr.h"
-#include "base/sequenced_task_runner.h"
 #include "base/time/clock.h"
 #include "base/time/time.h"
+#include "base/updateable_sequenced_task_runner.h"
 #include "chrome/browser/media/webrtc/webrtc_event_log_manager_common.h"
 #include "chrome/browser/media/webrtc/webrtc_event_log_manager_local.h"
 #include "chrome/browser/media/webrtc/webrtc_event_log_manager_remote.h"
@@ -306,6 +306,8 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
                                            const base::Time& delete_begin,
                                            const base::Time& delete_end);
 
+  void OnClearCacheForBrowserContextDoneInternal(base::OnceClosure reply);
+
   void GetHistoryInternal(
       BrowserContextId browser_context_id,
       base::OnceCallback<void(const std::vector<UploadList::UploadInfo>&)>
@@ -353,7 +355,7 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
   // This allows unit tests that do not wish to change the task runner to still
   // check when certain operations are finished.
   // TODO(crbug.com/775415): Remove this and use PostNullTaskForTesting instead.
-  scoped_refptr<base::SequencedTaskRunner>& GetTaskRunnerForTesting();
+  scoped_refptr<base::SequencedTaskRunner> GetTaskRunnerForTesting();
 
   void PostNullTaskForTesting(base::OnceClosure reply);
 
@@ -364,7 +366,11 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
 
   // The main logic will run sequentially on this runner, on which blocking
   // tasks are allowed.
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  scoped_refptr<base::UpdateableSequencedTaskRunner> task_runner_;
+
+  // The number of pending tasks to clear the cache. The priority of
+  // |task_runner_| is increased to USER_BLOCKING when this is non-zero.
+  int num_pending_clear_cache_ = 0;
 
   // Indicates whether remote-bound logging is generally allowed, although
   // possibly not for all profiles. This makes it possible for remote-bound to