Delete entries from Breakpad crash log file when clearing history.

This patch erases lines from the log file that Breakpad uses to record
crashes that fall within the history clear range. This file is used on
Linux, Android, and Chrome OS, and may be present even when the data in
the file does not show up in chrome://crashes.

Bug: 825994
Change-Id: I41f73ec9698398923fe67ed14c703375a9f046f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1460065Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646506}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -30,6 +30,7 @@
 #include "chrome/browser/browsing_data/navigation_entry_remover.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/crash_upload_list/crash_upload_list.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry_factory.h"
 #include "chrome/browser/data_reduction_proxy/data_reduction_proxy_chrome_settings.h"
@@ -597,9 +598,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
     }
 #endif
 
-#if !defined(OS_CHROMEOS)
-    crash_reporter::ClearReportsBetween(delete_begin_, delete_end_);
-#endif
+    CreateCrashUploadList()->Clear(delete_begin_, delete_end_);
   }
 
   //////////////////////////////////////////////////////////////////////////////

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -118,6 +118,11 @@
 #include "components/user_manager/scoped_user_manager.h"
 #endif  // defined(OS_CHROMEOS)
 
+#if defined(OS_LINUX) || defined(OS_CHROMEOS)
+#include "chrome/common/chrome_paths.h"
+#include "components/upload_list/crash_upload_list.h"
+#endif  // defined(OS_LINUX) || defined(OS_CHROMEOS)
+
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/mock_extension_special_storage_policy.h"
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
@@ -2954,3 +2959,34 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, WipeOriginVerifierData) {
       customtabs::OriginVerifier::GetClearBrowsingDataCallCountForTesting());
 }
 #endif  // defined(OS_ANDROID)
+
+#if defined(OS_LINUX) || defined(OS_CHROMEOS)
+TEST_F(ChromeBrowsingDataRemoverDelegateTest, WipeCrashData) {
+  base::FilePath crash_dir_path;
+  base::PathService::Get(chrome::DIR_CRASH_DUMPS, &crash_dir_path);
+  base::FilePath upload_log_path =
+      crash_dir_path.AppendASCII(CrashUploadList::kReporterLogFilename);
+
+  constexpr char kCrashEntry1[] = "12345,abc\n";
+  constexpr char kCrashEntry2[] = "67890,def\n";
+  std::string initial_contents = kCrashEntry1;
+  initial_contents.append(kCrashEntry2);
+  ASSERT_GT(base::WriteFile(upload_log_path, initial_contents.c_str(),
+                            static_cast<int>(initial_contents.size())),
+            0);
+
+  BlockUntilBrowsingDataRemoved(
+      base::Time::FromTimeT(67890u), base::Time::Max(),
+      ChromeBrowsingDataRemoverDelegate::DATA_TYPE_HISTORY, false);
+
+  std::string contents;
+  base::ReadFileToString(upload_log_path, &contents);
+  EXPECT_EQ(kCrashEntry1, contents);
+
+  BlockUntilBrowsingDataRemoved(
+      base::Time(), base::Time::Max(),
+      ChromeBrowsingDataRemoverDelegate::DATA_TYPE_HISTORY, false);
+
+  EXPECT_FALSE(base::PathExists(upload_log_path));
+}
+#endif

chrome/browser/crash_upload_list/crash_upload_list_crashpad.cc

@@ -60,6 +60,11 @@ std::vector<UploadList::UploadInfo> CrashUploadListCrashpad::LoadUploadList() {
   return uploads;
 }
 
+void CrashUploadListCrashpad::ClearUploadList(const base::Time& begin,
+                                              const base::Time& end) {
+  crash_reporter::ClearReportsBetween(begin, end);
+}
+
 void CrashUploadListCrashpad::RequestSingleUpload(const std::string& local_id) {
   crash_reporter::RequestSingleCrashUpload(local_id);
 }

chrome/browser/crash_upload_list/crash_upload_list_crashpad.h

@@ -8,6 +8,10 @@
 #include "base/macros.h"
 #include "components/upload_list/upload_list.h"
 
+namespace base {
+class Time;
+}
+
 // An UploadList that retrieves the list of crash reports from the
 // Crashpad database.
 class CrashUploadListCrashpad : public UploadList {
@@ -19,6 +23,7 @@ class CrashUploadListCrashpad : public UploadList {
 
   base::TaskTraits LoadingTaskTraits() override;
   std::vector<UploadInfo> LoadUploadList() override;
+  void ClearUploadList(const base::Time& begin, const base::Time& end) override;
   void RequestSingleUpload(const std::string& local_id) override;
 
   DISALLOW_COPY_AND_ASSIGN(CrashUploadListCrashpad);

chrome/browser/ui/webui/crashes_ui.cc

@@ -106,7 +106,7 @@ CrashesDOMHandler::CrashesDOMHandler()
 }
 
 CrashesDOMHandler::~CrashesDOMHandler() {
-  upload_list_->CancelCallback();
+  upload_list_->CancelLoadCallback();
 }
 
 void CrashesDOMHandler::RegisterMessages() {

chrome/browser/ui/webui/media/webrtc_logs_ui.cc

@@ -184,7 +184,7 @@ WebRtcLogsDOMHandler::WebRtcLogsDOMHandler(Profile* profile)
 
 WebRtcLogsDOMHandler::~WebRtcLogsDOMHandler() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  text_log_upload_list_->CancelCallback();
+  text_log_upload_list_->CancelLoadCallback();
 }
 
 void WebRtcLogsDOMHandler::RegisterMessages() {

components/upload_list/text_log_upload_list.cc

@@ -4,13 +4,31 @@
 
 #include "components/upload_list/text_log_upload_list.h"
 
-#include <string>
+#include <algorithm>
+#include <sstream>
 
 #include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 
+namespace {
+
+constexpr size_t kUploadTimeIndex = 0;
+constexpr size_t kCaptureTimeIndex = 3;
+
+std::vector<std::string> SplitIntoLines(const std::string& file_contents) {
+  return base::SplitString(file_contents, base::kWhitespaceASCII,
+                           base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+}
+
+std::vector<std::string> SplitIntoComponents(const std::string& line) {
+  return base::SplitString(line, ",", base::TRIM_WHITESPACE,
+                           base::SPLIT_WANT_ALL);
+}
+
+}  // namespace
+
 TextLogUploadList::TextLogUploadList(const base::FilePath& upload_log_path)
     : upload_log_path_(upload_log_path) {}
 
@@ -27,29 +45,69 @@ std::vector<UploadList::UploadInfo> TextLogUploadList::LoadUploadList() {
   if (base::PathExists(upload_log_path_)) {
     std::string contents;
     base::ReadFileToString(upload_log_path_, &contents);
-    std::vector<std::string> log_entries =
-        base::SplitString(contents, base::kWhitespaceASCII,
-                          base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-    ParseLogEntries(log_entries, &uploads);
+    ParseLogEntries(SplitIntoLines(contents), &uploads);
   }
 
   return uploads;
 }
 
+void TextLogUploadList::ClearUploadList(const base::Time& begin,
+                                        const base::Time& end) {
+  if (!base::PathExists(upload_log_path_))
+    return;
+
+  std::string contents;
+  base::ReadFileToString(upload_log_path_, &contents);
+  std::vector<std::string> log_entries = SplitIntoLines(contents);
+
+  std::ostringstream new_contents_stream;
+  for (const std::string& line : log_entries) {
+    std::vector<std::string> components = SplitIntoComponents(line);
+    double seconds_since_epoch;
+    if (components.size() > kUploadTimeIndex &&
+        !components[kUploadTimeIndex].empty() &&
+        base::StringToDouble(components[kUploadTimeIndex],
+                             &seconds_since_epoch)) {
+      base::Time upload_time = base::Time::FromDoubleT(seconds_since_epoch);
+      if (begin <= upload_time && upload_time <= end)
+        continue;
+    }
+
+    if (components.size() > kCaptureTimeIndex &&
+        !components[kCaptureTimeIndex].empty() &&
+        base::StringToDouble(components[kCaptureTimeIndex],
+                             &seconds_since_epoch)) {
+      base::Time capture_time = base::Time::FromDoubleT(seconds_since_epoch);
+      if (begin <= capture_time && capture_time <= end)
+        continue;
+    }
+
+    new_contents_stream << line << std::endl;
+  }
+
+  std::string new_contents = new_contents_stream.str();
+  if (new_contents.size() == 0) {
+    base::DeleteFile(upload_log_path_, /*recursive*/ false);
+  } else {
+    base::WriteFile(upload_log_path_, new_contents.c_str(),
+                    new_contents.size());
+  }
+}
+
 void TextLogUploadList::ParseLogEntries(
     const std::vector<std::string>& log_entries,
     std::vector<UploadInfo>* uploads) {
   std::vector<std::string>::const_reverse_iterator i;
-  for (i = log_entries.rbegin(); i != log_entries.rend(); ++i) {
-    std::vector<std::string> components =
-        base::SplitString(*i, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+  for (const std::string& line : log_entries) {
+    std::vector<std::string> components = SplitIntoComponents(line);
     // Skip any blank (or corrupted) lines.
     if (components.size() < 2 || components.size() > 5)
       continue;
     base::Time upload_time;
     double seconds_since_epoch;
-    if (!components[0].empty()) {
-      if (!base::StringToDouble(components[0], &seconds_since_epoch))
+    if (!components[kUploadTimeIndex].empty()) {
+      if (!base::StringToDouble(components[kUploadTimeIndex],
+                                &seconds_since_epoch))
         continue;
       upload_time = base::Time::FromDoubleT(seconds_since_epoch);
     }
@@ -60,8 +118,10 @@ void TextLogUploadList::ParseLogEntries(
       info.local_id = components[2];
 
     // Add capture time if present.
-    if (components.size() > 3 && !components[3].empty() &&
-        base::StringToDouble(components[3], &seconds_since_epoch)) {
+    if (components.size() > kCaptureTimeIndex &&
+        !components[kCaptureTimeIndex].empty() &&
+        base::StringToDouble(components[kCaptureTimeIndex],
+                             &seconds_since_epoch)) {
       info.capture_time = base::Time::FromDoubleT(seconds_since_epoch);
     }
 

components/upload_list/text_log_upload_list.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_UPLOAD_LIST_TEXT_LOG_UPLOAD_LIST_H_
 #define COMPONENTS_UPLOAD_LIST_TEXT_LOG_UPLOAD_LIST_H_
 
+#include <string>
 #include <vector>
 
 #include "base/files/file_path.h"
@@ -31,6 +32,7 @@ class TextLogUploadList : public UploadList {
   // UploadList:
   base::TaskTraits LoadingTaskTraits() override;
   std::vector<UploadList::UploadInfo> LoadUploadList() override;
+  void ClearUploadList(const base::Time& begin, const base::Time& end) override;
 
   // Parses upload log lines, converting them to UploadInfo entries.
   void ParseLogEntries(const std::vector<std::string>& log_entries,

components/upload_list/text_log_upload_list_unittest.cc

@@ -276,6 +276,69 @@ TEST_F(TextLogUploadListTest, ParseWithState) {
   }
 }
 
+TEST_F(TextLogUploadListTest, ClearUsingUploadTime) {
+  constexpr time_t kTestTime = 1234u;
+  constexpr char kOtherEntry[] = "4567,def\n";
+  std::string test_entry = base::NumberToString(kTestTime);
+  test_entry.append(",abc\n");
+  test_entry.append(kOtherEntry);
+  WriteUploadLog(test_entry);
+
+  scoped_refptr<TextLogUploadList> upload_list =
+      new TextLogUploadList(log_path());
+
+  base::RunLoop run_loop;
+  upload_list->Clear(base::Time::FromTimeT(kTestTime),
+                     base::Time::FromTimeT(kTestTime + 1),
+                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  std::string contents;
+  base::ReadFileToString(log_path(), &contents);
+  EXPECT_EQ(kOtherEntry, contents);
+}
+
+TEST_F(TextLogUploadListTest, ClearUsingCaptureTime) {
+  constexpr time_t kTestTime = 1234u;
+  constexpr char kOtherEntry[] = "4567,def,def,7890\n";
+  std::string test_entry = kOtherEntry;
+  test_entry.append("4567,abc,abc,");
+  test_entry.append(base::NumberToString(kTestTime));
+  test_entry.append("\n");
+  WriteUploadLog(test_entry);
+
+  scoped_refptr<TextLogUploadList> upload_list =
+      new TextLogUploadList(log_path());
+
+  base::RunLoop run_loop;
+  upload_list->Clear(base::Time::FromTimeT(kTestTime),
+                     base::Time::FromTimeT(kTestTime + 1),
+                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  std::string contents;
+  ASSERT_TRUE(base::ReadFileToString(log_path(), &contents));
+  EXPECT_EQ(kOtherEntry, contents);
+}
+
+TEST_F(TextLogUploadListTest, ClearingAllDataDeletesFile) {
+  constexpr time_t kTestTime = 1234u;
+  std::string test_entry = base::NumberToString(kTestTime);
+  test_entry.append(",abc\n");
+  WriteUploadLog(test_entry);
+
+  scoped_refptr<TextLogUploadList> upload_list =
+      new TextLogUploadList(log_path());
+
+  base::RunLoop run_loop;
+  upload_list->Clear(base::Time::FromTimeT(kTestTime),
+                     base::Time::FromTimeT(kTestTime + 1),
+                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_FALSE(base::PathExists(log_path()));
+}
+
 // https://crbug.com/597384
 TEST_F(TextLogUploadListTest, SimultaneousAccess) {
   std::string test_entry = kTestUploadTime;

components/upload_list/upload_list.cc

@@ -51,15 +51,26 @@ UploadList::~UploadList() = default;
 
 void UploadList::Load(base::OnceClosure callback) {
   DCHECK(sequence_checker_.CalledOnValidSequence());
-  callback_ = std::move(callback);
+  load_callback_ = std::move(callback);
   base::PostTaskWithTraitsAndReplyWithResult(
       FROM_HERE, LoadingTaskTraits(),
       base::Bind(&UploadList::LoadUploadList, this),
       base::Bind(&UploadList::OnLoadComplete, this));
 }
 
-void UploadList::CancelCallback() {
-  callback_.Reset();
+void UploadList::Clear(const base::Time& begin,
+                       const base::Time& end,
+                       base::OnceClosure callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
+  clear_callback_ = std::move(callback);
+  base::PostTaskWithTraitsAndReply(
+      FROM_HERE, LoadingTaskTraits(),
+      base::BindOnce(&UploadList::ClearUploadList, this, begin, end),
+      base::BindOnce(&UploadList::OnClearComplete, this));
+}
+
+void UploadList::CancelLoadCallback() {
+  load_callback_.Reset();
 }
 
 void UploadList::RequestSingleUploadAsync(const std::string& local_id) {
@@ -85,6 +96,11 @@ void UploadList::RequestSingleUpload(const std::string& local_id) {
 
 void UploadList::OnLoadComplete(const std::vector<UploadInfo>& uploads) {
   uploads_ = uploads;
-  if (!callback_.is_null())
-    std::move(callback_).Run();
+  if (!load_callback_.is_null())
+    std::move(load_callback_).Run();
+}
+
+void UploadList::OnClearComplete() {
+  if (!clear_callback_.is_null())
+    std::move(clear_callback_).Run();
 }

components/upload_list/upload_list.h

@@ -71,8 +71,14 @@ class UploadList : public base::RefCountedThreadSafe<UploadList> {
   // overwrite the previously supplied one, and the first will not be called.
   void Load(base::OnceClosure callback);
 
+  // Clears any data associated with the upload list, where the upload time or
+  // capture time falls within the given range.
+  void Clear(const base::Time& begin,
+             const base::Time& end,
+             base::OnceClosure callback = base::OnceClosure());
+
   // Clears any callback specified in Load().
-  void CancelCallback();
+  void CancelLoadCallback();
 
   // Asynchronously requests a user triggered upload.
   void RequestSingleUploadAsync(const std::string& local_id);
@@ -92,6 +98,10 @@ class UploadList : public base::RefCountedThreadSafe<UploadList> {
   // Reads the upload log and stores the entries in |uploads|.
   virtual std::vector<UploadInfo> LoadUploadList() = 0;
 
+  // Clears data within the given time range. See Clear.
+  virtual void ClearUploadList(const base::Time& begin,
+                               const base::Time& end) = 0;
+
   // Requests a user triggered upload for a crash report with a given id.
   virtual void RequestSingleUpload(const std::string& local_id);
 
@@ -99,14 +109,18 @@ class UploadList : public base::RefCountedThreadSafe<UploadList> {
   friend class base::RefCountedThreadSafe<UploadList>;
 
   // When LoadUploadList() finishes, the results are reported in |uploads|
-  // and the |callback_| is run.
+  // and the |load_callback_| is run.
   void OnLoadComplete(const std::vector<UploadInfo>& uploads);
 
+  // Called when ClearUploadList() finishes.
+  void OnClearComplete();
+
   // Ensures that this class' thread unsafe state is only accessed from the
   // sequence that owns this UploadList.
   base::SequenceChecker sequence_checker_;
 
-  base::OnceClosure callback_;
+  base::OnceClosure load_callback_;
+  base::OnceClosure clear_callback_;
 
   std::vector<UploadInfo> uploads_;
 

ios/chrome/browser/ui/webui/crashes_ui.cc

@@ -89,7 +89,7 @@ CrashesDOMHandler::CrashesDOMHandler()
 }
 
 CrashesDOMHandler::~CrashesDOMHandler() {
-  upload_list_->CancelCallback();
+  upload_list_->CancelLoadCallback();
 }
 
 void CrashesDOMHandler::RegisterMessages() {