Proactive pruning of pending remote-bound WebRTC event logs

Occasionally prune pending remote-bound WebRTC event logs.
This prevents them from staying on disk for longer than the
retention time, in cases where the triggers to non-proactive
pruning do not happen (Chrome is not restarted, peer connections
are not started/stopped, etc.).

Bug: 775415
Change-Id: I1a4e81495b12813776528ed678a445add1fe185f
Reviewed-on: https://chromium-review.googlesource.com/1005234Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549707}

chrome/browser/media/webrtc/webrtc_event_log_manager_remote.cc

@@ -18,6 +18,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/rand_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
@@ -31,6 +32,9 @@ static_assert(kMaxRemoteLogFileMetadataSizeBytes <= 0xFFFFFFu,
 const size_t kMaxRemoteLogFileSizeBytes = 50000000u;
 
 namespace {
+const base::TimeDelta kDefaultProactivePruningDelta =
+    base::TimeDelta::FromMinutes(5);
+
 const base::FilePath::CharType kRemoteBoundLogSubDirectory[] =
     FILE_PATH_LITERAL("webrtc_event_logs");
 
@@ -73,6 +77,27 @@ bool AreLogParametersValid(size_t max_file_size_bytes,
 
   return true;
 }
+
+base::Optional<base::TimeDelta> GetProactivePruningDelta() {
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          ::switches::kWebRtcRemoteEventLogProactivePruningDelta)) {
+    const std::string delta_seconds_str =
+        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            ::switches::kWebRtcRemoteEventLogProactivePruningDelta);
+    int64_t seconds;
+    if (base::StringToInt64(delta_seconds_str, &seconds) && seconds >= 0) {
+      // A delta of 0 seconds is used to signal the intention of disabling
+      // proactive pruning altogether. (From the command line. Past the command
+      // line, we use an unset optional to signal that.)
+      return (seconds == 0) ? base::Optional<base::TimeDelta>()
+                            : base::TimeDelta::FromSeconds(seconds);
+    } else {
+      LOG(WARNING) << "Proactive pruning delta could not be parsed.";
+    }
+  }
+
+  return kDefaultProactivePruningDelta;
+}
 }  // namespace
 
 const size_t kMaxActiveRemoteBoundWebRtcEventLogs = 3;
@@ -96,10 +121,15 @@ WebRtcRemoteEventLogManager::WebRtcRemoteEventLogManager(
     : upload_suppression_disabled_(
           base::CommandLine::ForCurrentProcess()->HasSwitch(
               ::switches::kWebRtcRemoteEventLogUploadNoSuppression)),
+      proactive_prune_scheduling_delta_(GetProactivePruningDelta()),
+      proactive_prune_scheduling_started_(false),
       observer_(observer),
       uploader_factory_(new WebRtcEventLogUploaderImpl::Factory) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DETACH_FROM_SEQUENCE(io_task_sequence_checker_);
+  // Proactive pruning would not do anything at the moment; it will be started
+  // with the first enabled browser context. This will all have the benefit
+  // of doing so on io_task_sequence_checker_ rather than the UI thread.
 }
 
 WebRtcRemoteEventLogManager::~WebRtcRemoteEventLogManager() {
@@ -126,6 +156,12 @@ void WebRtcRemoteEventLogManager::EnableForBrowserContext(
   AddPendingLogs(browser_context_id, remote_bound_logs_dir);
 
   enabled_browser_contexts_.insert(browser_context_id);
+
+  if (proactive_prune_scheduling_delta_.has_value() &&
+      !proactive_prune_scheduling_started_) {
+    proactive_prune_scheduling_started_ = true;
+    RecurringPendingLogsPrune();
+  }
 }
 
 // TODO(crbug.com/775415): Add unit tests.
@@ -513,6 +549,21 @@ void WebRtcRemoteEventLogManager::PrunePendingLogs() {
       base::Time::Now() - kRemoteBoundWebRtcEventLogsMaxRetention);
 }
 
+void WebRtcRemoteEventLogManager::RecurringPendingLogsPrune() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(io_task_sequence_checker_);
+  DCHECK(proactive_prune_scheduling_delta_.has_value());
+  DCHECK_GT(*proactive_prune_scheduling_delta_, base::TimeDelta());
+  DCHECK(proactive_prune_scheduling_started_);
+
+  PrunePendingLogs();
+
+  base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(&WebRtcRemoteEventLogManager::RecurringPendingLogsPrune,
+                     base::Unretained(this)),
+      *proactive_prune_scheduling_delta_);
+}
+
 void WebRtcRemoteEventLogManager::RemovePendingLogs(
     const base::Time& delete_begin,
     const base::Time& delete_end,

chrome/browser/media/webrtc/webrtc_event_log_manager_remote.h

@@ -201,6 +201,9 @@ class WebRtcRemoteEventLogManager final
   // this check is not too expensive.
   void PrunePendingLogs();
 
+  // PrunePendingLogs() and schedule the next proactive prune.
+  void RecurringPendingLogsPrune();
+
   // Removes pending logs whose last modification date was between at or later
   // than |delete_begin|, and earlier than |delete_end|.
   // If a null time-point is given as either |delete_begin| or |delete_begin|,
@@ -257,6 +260,17 @@ class WebRtcRemoteEventLogManager final
   // This may be disabled from the command line.
   const bool upload_suppression_disabled_;
 
+  // Proactive pruning will be done only if this has a value, in which case,
+  // every |proactive_prune_scheduling_delta_|, pending logs will be pruned.
+  // This avoids them staying around on disk for longer than their expiration
+  // if no event occurs which triggers reactive pruning.
+  const base::Optional<base::TimeDelta> proactive_prune_scheduling_delta_;
+
+  // Proactive pruning, if enabled, starts with the first enabled browser
+  // context. To avoid unnecessary complexity, if that browser context is
+  // disabled, proactive pruning is not disabled.
+  bool proactive_prune_scheduling_started_;
+
   // This is used to inform WebRtcEventLogManager when remote-bound logging
   // of a peer connection starts/stops, which allows WebRtcEventLogManager to
   // decide when to ask WebRTC to start/stop sending event logs.

chrome/browser/media/webrtc/webrtc_event_log_manager_unittest.cc

@@ -146,6 +146,12 @@ class WebRtcEventLogManagerTestBase : public ::testing::TestWithParam<bool> {
         uploader_run_loop_(std::make_unique<base::RunLoop>()),
         browser_context_(nullptr),
         upload_suppressing_browser_context_(nullptr) {
+    // Avoid proactive pruning; it has the potential to mess up tests, as well
+    // as keep pendings tasks around with a dangling reference to the unit
+    // under test. (Zero is a sentinel value for disabling proactive pruning.)
+    scoped_command_line_.GetProcessCommandLine()->AppendSwitchASCII(
+        ::switches::kWebRtcRemoteEventLogProactivePruningDelta, "0");
+
     EXPECT_TRUE(local_logs_base_dir_.CreateUniqueTempDir());
     local_logs_base_path_ = local_logs_base_dir_.GetPath().Append(
         FILE_PATH_LITERAL("local_event_logs"));

chrome/common/chrome_switches.cc

@@ -707,6 +707,14 @@ const char kVersion[]                       = "version";
 //   will be used. (This behavior may depend on additional factors.)
 const char kWebRtcRemoteEventLog[] = "webrtc-remote-event-log";
 
+// Sets the delay (in seconds) between proactive prunings of remote-bound
+// WebRTC event logs which are pending upload.
+// All positive values are legal.
+// All negative values are illegal, and ignored.
+// If set to 0, the meaning is "no proactive pruning".
+const char kWebRtcRemoteEventLogProactivePruningDelta[] =
+    "webrtc-event-log-proactive-pruning-delta";
+
 // Normally, remote-bound WebRTC event logs are uploaded only when no
 // peer connections are active. With this flag, the upload is never suppressed.
 const char kWebRtcRemoteEventLogUploadNoSuppression[] =

chrome/common/chrome_switches.h

@@ -199,6 +199,7 @@ extern const char kUserDataDir[];
 extern const char kValidateCrx[];
 extern const char kVersion[];
 extern const char kWebRtcRemoteEventLog[];
+extern const char kWebRtcRemoteEventLogProactivePruningDelta[];
 extern const char kWebRtcRemoteEventLogUploadNoSuppression[];
 extern const char kWindowPosition[];
 extern const char kWindowSize[];