[iOS] Persist Breadcrumbs across application launches

Update BreadcrumbPersistentStorageKeyedSerice to perform file writes on
background thread. Additionally, trim the stored breadcrumbs file size
to prevent it from growing too large.

BreadcrumbPersistentStorageKeyedSerice was implemented previously but
never used so this CL makes is fully ready for use.

Bug: 1082892
Change-Id: Ifbd477dd3bf32f71a70a742dbf45b0fa5fc3665a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2226416
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779587}

ios/chrome/app/main_controller.mm

@@ -46,6 +46,8 @@
 #include "ios/chrome/browser/chrome_paths.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.h"
+#include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_keyed_service.h"
+#include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_keyed_service_factory.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/features.h"
 #include "ios/chrome/browser/crash_report/breakpad_helper.h"
 #include "ios/chrome/browser/crash_report/crash_keys_helper.h"
@@ -284,6 +286,8 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
 - (void)scheduleAppDistributionPings;
 // Asynchronously schedule the init of the memoryDebuggerManager.
 - (void)scheduleMemoryDebuggingTools;
+// Starts logging breadcrumbs.
+- (void)startLoggingBreadcrumbs;
 // Asynchronously kick off regular free memory checks.
 - (void)startFreeMemoryMonitoring;
 // Asynchronously schedules the reset of the failed startup attempt counter.
@@ -488,9 +492,7 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
   self.mainBrowserState = chromeBrowserState;
 
   if (base::FeatureList::IsEnabled(kLogBreadcrumbs)) {
-    breakpad::MonitorBreadcrumbManagerService(
-        BreadcrumbManagerKeyedServiceFactory::GetForBrowserState(
-            self.mainBrowserState));
+    [self startLoggingBreadcrumbs];
   }
 
   // Force an obvious initialization of the AuthenticationService. This must
@@ -954,6 +956,37 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
   StartFreeMemoryMonitor();
 }
 
+- (void)startLoggingBreadcrumbs {
+  BreadcrumbManagerKeyedService* breadcrumbService =
+      BreadcrumbManagerKeyedServiceFactory::GetForBrowserState(
+          self.mainBrowserState);
+  breakpad::MonitorBreadcrumbManagerService(breadcrumbService);
+
+  __weak __typeof(self) weakSelf = self;
+  BreadcrumbPersistentStorageKeyedService* persistentStorageService =
+      BreadcrumbPersistentStorageKeyedServiceFactory::GetForBrowserState(
+          self.mainBrowserState);
+  // Get stored persistent breadcrumbs from last run and set them on the
+  // breadcrumb manager.
+  persistentStorageService->GetStoredEvents(
+      base::BindOnce(^(std::vector<std::string> events) {
+        __strong __typeof(weakSelf) strongSelf = weakSelf;
+        if (!strongSelf || !strongSelf.mainBrowserState) {
+          return;
+        }
+
+        BreadcrumbManagerKeyedServiceFactory::GetForBrowserState(
+            strongSelf.mainBrowserState)
+            ->SetPreviousEvents(events);
+
+        // Notify persistent breadcrumb service to clear old breadcrumbs and
+        // start storing breadcrumbs for this session.
+        BreadcrumbPersistentStorageKeyedServiceFactory::GetForBrowserState(
+            strongSelf.mainBrowserState)
+            ->StartStoringEvents();
+      }));
+}
+
 - (void)scheduleLowPriorityStartupTasks {
   [_startupTasks initializeOmaha];
   [_startupTasks donateIntents];

ios/chrome/browser/crash_report/BUILD.gn

@@ -25,6 +25,7 @@ source_set("crash_report") {
   configs += [ "//build/config/compiler:enable_arc" ]
 
   deps = [
+    ":constants",
     "//base",
     "//components/crash/core/common",
     "//components/upload_list",
@@ -37,6 +38,10 @@ source_set("crash_report") {
   ]
 }
 
+source_set("constants") {
+  sources = [ "crash_reporter_breadcrumb_constants.h" ]
+}
+
 source_set("crash_report_internal") {
   configs += [ "//build/config/compiler:enable_arc" ]
   sources = [
@@ -82,6 +87,7 @@ source_set("unit_tests") {
     "crash_restore_helper_unittest.mm",
   ]
   deps = [
+    ":constants",
     ":crash_report",
     ":crash_report_internal",
     "//base",

ios/chrome/browser/crash_report/breadcrumbs/BUILD.gn

@@ -21,6 +21,7 @@ source_set("breadcrumbs") {
     "//components/keyed_service/ios",
     "//ios/chrome/browser:chrome_url_constants",
     "//ios/chrome/browser/browser_state",
+    "//ios/chrome/browser/crash_report:constants",
     "//ios/chrome/browser/infobars",
     "//ios/chrome/browser/main:public",
     "//ios/chrome/browser/overlays",
@@ -83,6 +84,8 @@ source_set("unit_tests") {
     "//base/test:test_support",
     "//ios/chrome/browser:chrome_url_constants",
     "//ios/chrome/browser/browser_state:test_support",
+    "//ios/chrome/browser/crash_report",
+    "//ios/chrome/browser/crash_report:constants",
     "//ios/chrome/browser/download",
     "//ios/chrome/browser/infobars",
     "//ios/chrome/browser/infobars/test",

ios/chrome/browser/crash_report/breadcrumbs/application_breadcrumbs_logger_unittest.mm

@@ -8,7 +8,7 @@
 #include "base/metrics/user_metrics_action.h"
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
-#import "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h"
+#include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h"
 #include "testing/platform_test.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.cc

@@ -7,9 +7,20 @@
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer.h"
+#include "ios/chrome/browser/crash_report/crash_reporter_breadcrumb_constants.h"
 
 namespace {
 
+// The maximum number of breadcrumbs which are expected to be useful to store.
+// NOTE: Events are "bucketed" into groups by short time intervals to make it
+// more efficient to manage the continuous dropping of old events. Since events
+// are only dropped at the bucket level, it is expected that the total number of
+// stored breadcrumbs will exceed this value. This value should be close to the
+// upper limit of useful events. (Most events + timestamp breadcrumbs are
+// currently longer than 10 characters.)
+constexpr unsigned long kMaxUsefulBreadcrumbEvents =
+    kMaxBreadcrumbsDataLength / 10;
+
 // The minimum number of event buckets to keep, even if they are expired.
 const int kMinEventsBuckets = 2;
 
@@ -28,9 +39,21 @@ base::Time EventBucket(const base::Time& time) {
 
 }  // namespace
 
-BreadcrumbManager::BreadcrumbManager() = default;
+BreadcrumbManager::BreadcrumbManager() : start_time_(base::Time::Now()) {}
+
 BreadcrumbManager::~BreadcrumbManager() = default;
 
+size_t BreadcrumbManager::GetEventCount() {
+  DropOldEvents();
+
+  size_t count = 0;
+  for (auto it = event_buckets_.rbegin(); it != event_buckets_.rend(); ++it) {
+    std::list<std::string> bucket_events = it->second;
+    count += bucket_events.size();
+  }
+  return count;
+}
+
 const std::list<std::string> BreadcrumbManager::GetEvents(
     size_t event_count_limit) {
   DropOldEvents();
@@ -50,6 +73,29 @@ const std::list<std::string> BreadcrumbManager::GetEvents(
   return events;
 }
 
+void BreadcrumbManager::SetPreviousEvents(
+    const std::vector<std::string>& events) {
+  if (events.empty()) {
+    return;
+  }
+
+  // Create a new bucket with a fake timestamp before the application started.
+  // This ensures that these initial events will be dropped before new events
+  // from the current session.
+  base::TimeDelta time_since_construction = base::Time::Now() - start_time_;
+  base::Time previous_events_bucket_time =
+      base::Time::Now() -
+      base::TimeDelta::FromSeconds(60 + time_since_construction.InSeconds());
+  std::pair<base::Time, std::list<std::string>> bucket(
+      EventBucket(previous_events_bucket_time), std::list<std::string>());
+
+  for (auto event_it = events.rbegin(); event_it != events.rend(); ++event_it) {
+    std::string event = *event_it;
+    bucket.second.push_front(event);
+  }
+  event_buckets_.push_front(bucket);
+}
+
 void BreadcrumbManager::AddEvent(const std::string& event) {
   base::Time time = base::Time::Now();
   base::Time bucket_time = EventBucket(time);
@@ -69,24 +115,47 @@ void BreadcrumbManager::AddEvent(const std::string& event) {
       base::StringPrintf("%s %s", timestamp.c_str(), event.c_str());
   event_buckets_.back().second.push_back(event_log);
 
-  DropOldEvents();
-
   for (auto& observer : observers_) {
     observer.EventAdded(this, event_log);
   }
+
+  DropOldEvents();
 }
 
 void BreadcrumbManager::DropOldEvents() {
   static const base::TimeDelta kMessageExpirationTime =
       base::TimeDelta::FromMinutes(20);
 
+  bool old_buckets_dropped = false;
   base::Time now = base::Time::Now();
+  // Drop buckets which are more than kMessageExpirationTime old.
   while (event_buckets_.size() > kMinEventsBuckets) {
     base::Time oldest_bucket_time = event_buckets_.front().first;
     if (now - oldest_bucket_time < kMessageExpirationTime) {
       break;
     }
     event_buckets_.pop_front();
+    old_buckets_dropped = true;
+  }
+
+  // Drop buckets if the data is unlikely to ever be needed.
+  unsigned long newer_event_count = 0;
+  auto event_bucket_it = event_buckets_.rbegin();
+  while (event_bucket_it != event_buckets_.rend()) {
+    std::list<std::string> bucket_events = event_bucket_it->second;
+    if (newer_event_count > kMaxUsefulBreadcrumbEvents) {
+      event_buckets_.erase(event_buckets_.begin(), event_bucket_it.base());
+      old_buckets_dropped = true;
+      break;
+    }
+    newer_event_count += bucket_events.size();
+    ++event_bucket_it;
+  }
+
+  if (old_buckets_dropped) {
+    for (auto& observer : observers_) {
+      observer.OldEventsRemoved(this);
+    }
   }
 }
 

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h

@@ -10,7 +10,7 @@
 #include <string>
 
 #include "base/observer_list.h"
-#import "base/time/time.h"
+#include "base/time/time.h"
 
 class BreadcrumbManagerObserver;
 
@@ -21,12 +21,22 @@ class BreadcrumbManagerObserver;
 // stale data.
 class BreadcrumbManager {
  public:
+  // Returns the number of collected breadcrumb events which are still relevant.
+  // Note: This method may drop old events so the value can change even when no
+  // new events have been added, but time has passed.
+  size_t GetEventCount();
+
   // Returns a list of the collected breadcrumb events which are still relevant
   // up to |event_count_limit|. Passing zero for |event_count_limit| signifies
   // no limit. Events returned will have a timestamp prepended to the original
   // |event| string representing when |AddEvent| was called.
+  // Note: This method may drop old events so the returned events can change
+  // even if no new events have been added, but time has passed.
   const std::list<std::string> GetEvents(size_t event_count_limit);
 
+  // Sets previous events by inserting them before all existing events.
+  void SetPreviousEvents(const std::vector<std::string>& events);
+
   // Logs a breadcrumb event with message data |event|.
   // NOTE: |event| must not include newline characters as newlines are used by
   // BreadcrumbPersistentStore as a deliminator.
@@ -48,6 +58,9 @@ class BreadcrumbManager {
   // newer events are limited.
   void DropOldEvents();
 
+  // Creation time of the BreadcrumbManager.
+  const base::Time start_time_;
+
   // List of events, paired with the time which they were logged to minute
   // resolution. Newer events are at the end of the list.
   std::list<std::pair<base::Time, std::list<std::string>>> event_buckets_;

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_browser_agent.mm

@@ -8,6 +8,7 @@
 
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.h"
 #import "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_tab_helper.h"

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service.cc

@@ -8,6 +8,11 @@
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h"
 #include "ios/web/public/browser_state.h"
 
+void BreadcrumbManagerKeyedService::SetPreviousEvents(
+    const std::vector<std::string>& events) {
+  breadcrumb_manager_->SetPreviousEvents(events);
+}
+
 void BreadcrumbManagerKeyedService::AddEvent(const std::string& event) {
   std::string event_log =
       base::StringPrintf("%s%s", browsing_mode_.c_str(), event.c_str());
@@ -24,6 +29,10 @@ void BreadcrumbManagerKeyedService::RemoveObserver(
   breadcrumb_manager_->RemoveObserver(observer);
 }
 
+size_t BreadcrumbManagerKeyedService::GetEventCount() {
+  return breadcrumb_manager_->GetEventCount();
+}
+
 const std::list<std::string> BreadcrumbManagerKeyedService::GetEvents(
     size_t event_count_limit) const {
   return breadcrumb_manager_->GetEvents(event_count_limit);

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service.h

@@ -24,6 +24,9 @@ class BreadcrumbManagerKeyedService : public KeyedService {
   explicit BreadcrumbManagerKeyedService(web::BrowserState* browser_state);
   ~BreadcrumbManagerKeyedService() override;
 
+  // Sets previous events by inserting them before all existing events.
+  void SetPreviousEvents(const std::vector<std::string>& events);
+
   // Logs a breadcrumb |event| associated with the BrowserState passed in at
   // initialization of this instance. Prepends the |browsing_mode_| identifier
   // to the event before passing it to the |breadcrumb_manager_|.
@@ -33,6 +36,10 @@ class BreadcrumbManagerKeyedService : public KeyedService {
   void AddObserver(BreadcrumbManagerObserver* observer);
   void RemoveObserver(BreadcrumbManagerObserver* observer);
 
+  // Returns the number of collected breadcrumb events which are still relevant.
+  // See |BreadcrumbManager::GetEventCount| for details.
+  size_t GetEventCount();
+
   // Returns up to |event_count_limit| events from the underlying
   // |breadcrumb_manager|. See |BreadcrumbManager::GetEvents| for returned event
   // details.

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.cc

@@ -5,7 +5,6 @@
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.h"
 
 #include "components/keyed_service/ios/browser_state_dependency_manager.h"
-#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service.h"
 #include "ios/web/public/browser_state.h"
 
@@ -19,7 +18,7 @@ BreadcrumbManagerKeyedServiceFactory::GetInstance() {
 // static
 BreadcrumbManagerKeyedService*
 BreadcrumbManagerKeyedServiceFactory::GetForBrowserState(
-    ChromeBrowserState* browser_state) {
+    web::BrowserState* browser_state) {
   return static_cast<BreadcrumbManagerKeyedService*>(
       GetInstance()->GetServiceForBrowserState(browser_state, /*create=*/true));
 }

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.h

@@ -9,14 +9,17 @@
 #include "components/keyed_service/ios/browser_state_keyed_service_factory.h"
 
 class BreadcrumbManagerKeyedService;
-class ChromeBrowserState;
+
+namespace web {
+class BrowserState;
+}  // namespace web
 
 class BreadcrumbManagerKeyedServiceFactory
     : public BrowserStateKeyedServiceFactory {
  public:
   static BreadcrumbManagerKeyedServiceFactory* GetInstance();
   static BreadcrumbManagerKeyedService* GetForBrowserState(
-      ChromeBrowserState* browser_state);
+      web::BrowserState* browser_state);
 
  private:
   friend class base::NoDestructor<BreadcrumbManagerKeyedServiceFactory>;

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_unittest.mm

@@ -23,8 +23,7 @@ namespace {
 // Creates a new BreadcrumbManagerKeyedService for |browser_state|.
 std::unique_ptr<KeyedService> BuildBreadcrumbManagerKeyedService(
     web::BrowserState* browser_state) {
-  return std::make_unique<BreadcrumbManagerKeyedService>(
-      ChromeBrowserState::FromBrowserState(browser_state));
+  return std::make_unique<BreadcrumbManagerKeyedService>(browser_state);
 }
 }
 

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer.h

@@ -21,6 +21,9 @@ class BreadcrumbManagerObserver : public base::CheckedObserver {
   virtual void EventAdded(BreadcrumbManager* manager,
                           const std::string& event) {}
 
+  // Called when old events have been removed.
+  virtual void OldEventsRemoved(BreadcrumbManager* manager) {}
+
  protected:
   BreadcrumbManagerObserver() = default;
 

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer_bridge.h

@@ -5,15 +5,20 @@
 #ifndef IOS_CHROME_BROWSER_CRASH_REPORT_BREADCRUMBS_BREADCRUMB_MANAGER_OBSERVER_BRIDGE_H_
 #define IOS_CHROME_BROWSER_CRASH_REPORT_BREADCRUMBS_BREADCRUMB_MANAGER_OBSERVER_BRIDGE_H_
 
+#import <Foundation/Foundation.h>
+
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer.h"
 
 class BreadcrumbManager;
 class BreadcrumbManagerKeyedService;
 
 // Protocol mirroring BreadcrumbManagerObserver
-@protocol BreadcrumbManagerObserving
+@protocol BreadcrumbManagerObserving <NSObject>
+@optional
 - (void)breadcrumbManager:(BreadcrumbManager*)manager
               didAddEvent:(NSString*)string;
+
+- (void)breadcrumbManagerDidRemoveOldEvents:(BreadcrumbManager*)manager;
 @end
 
 // A C++ bridge class to handle receiving notifications from the C++ class
@@ -42,6 +47,7 @@ class BreadcrumbManagerObserverBridge : public BreadcrumbManagerObserver {
   // BreadcrumbManagerObserver implementation:
   void EventAdded(BreadcrumbManager* manager,
                   const std::string& event) override;
+  void OldEventsRemoved(BreadcrumbManager* manager) override;
 
   BreadcrumbManager* breadcrumb_manager_ = nullptr;
   BreadcrumbManagerKeyedService* breadcrumb_manager_service_ = nullptr;

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer_bridge.mm

@@ -41,6 +41,17 @@ BreadcrumbManagerObserverBridge::~BreadcrumbManagerObserverBridge() {
 
 void BreadcrumbManagerObserverBridge::EventAdded(BreadcrumbManager* manager,
                                                  const std::string& event) {
+  if ([observer_ respondsToSelector:@selector(breadcrumbManager:
+                                                    didAddEvent:)]) {
     [observer_ breadcrumbManager:manager
                      didAddEvent:base::SysUTF8ToNSString(event)];
+  }
+}
+
+void BreadcrumbManagerObserverBridge::OldEventsRemoved(
+    BreadcrumbManager* manager) {
+  if ([observer_
+          respondsToSelector:@selector(breadcrumbManagerDidRemoveOldEvents:)]) {
+    [observer_ breadcrumbManagerDidRemoveOldEvents:manager];
+  }
 }

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer_bridge_unittest.mm

@@ -5,9 +5,9 @@
 #import "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer_bridge.h"
 
 #import "base/strings/sys_string_conversions.h"
-#include "base/test/task_environment.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h"
+#import "ios/web/public/test/web_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
@@ -27,7 +27,9 @@ class BreadcrumbManagerObserverBridgeTest : public PlatformTest {
         &breadcrumb_manager_, mock_observer_);
   }
 
-  base::test::TaskEnvironment task_environment_;
+  web::WebTaskEnvironment task_env_{
+      web::WebTaskEnvironment::Options::DEFAULT,
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
   BreadcrumbManager breadcrumb_manager_;
   id mock_observer_;
   std::unique_ptr<BreadcrumbManagerObserverBridge> observer_bridge_;
@@ -50,3 +52,21 @@ TEST_F(BreadcrumbManagerObserverBridgeTest, EventAdded) {
 
   [mock_observer_ verify];
 }
+
+// Tests |breadcrumbManagerDidRemoveOldEvents:| forwarding.
+TEST_F(BreadcrumbManagerObserverBridgeTest, OldEventsRemoved) {
+  OCMExpect([mock_observer_ breadcrumbManager:&breadcrumb_manager_
+                                  didAddEvent:OCMOCK_ANY]);
+  OCMExpect([mock_observer_
+      breadcrumbManagerDidRemoveOldEvents:&breadcrumb_manager_]);
+
+  // Oldest event will be dropped becuase it is old enough and there are two
+  // newer buckets.
+  breadcrumb_manager_.AddEvent("event1");
+  task_env_.FastForwardBy(base::TimeDelta::FromHours(1));
+  breadcrumb_manager_.AddEvent("event2");
+  task_env_.FastForwardBy(base::TimeDelta::FromHours(1));
+  breadcrumb_manager_.AddEvent("event3");
+
+  [mock_observer_ verify];
+}

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer_unittest.mm

@@ -5,6 +5,7 @@
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer.h"
 
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h"
+#import "ios/web/public/test/web_task_environment.h"
 #include "testing/platform_test.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -24,12 +25,18 @@ class FakeBreadcrumbManagerObserver : public BreadcrumbManagerObserver {
   // BreadcrumbManagerObserver
   void EventAdded(BreadcrumbManager* manager,
                   const std::string& event) override {
-    last_received_manager_ = manager;
-    last_received_event_ = event;
+    event_added_last_received_manager_ = manager;
+    event_added_last_received_event_ = event;
   }
 
-  BreadcrumbManager* last_received_manager_ = nullptr;
-  std::string last_received_event_;
+  void OldEventsRemoved(BreadcrumbManager* manager) override {
+    old_events_removed_last_received_manager_ = manager;
+  }
+
+  BreadcrumbManager* event_added_last_received_manager_ = nullptr;
+  std::string event_added_last_received_event_;
+
+  BreadcrumbManager* old_events_removed_last_received_manager_ = nullptr;
 };
 
 }
@@ -42,6 +49,10 @@ class BreadcrumbManagerObserverTest : public PlatformTest {
     manager_.RemoveObserver(&observer_);
   }
 
+  web::WebTaskEnvironment task_env_{
+      web::WebTaskEnvironment::Options::DEFAULT,
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+
   BreadcrumbManager manager_;
   FakeBreadcrumbManagerObserver observer_;
 };
@@ -49,13 +60,29 @@ class BreadcrumbManagerObserverTest : public PlatformTest {
 // Tests that |BreadcrumbManagerObserver::EventAdded| is called when an event to
 // added to |manager_|.
 TEST_F(BreadcrumbManagerObserverTest, EventAdded) {
-  ASSERT_FALSE(observer_.last_received_manager_);
-  ASSERT_TRUE(observer_.last_received_event_.empty());
+  ASSERT_FALSE(observer_.event_added_last_received_manager_);
+  ASSERT_TRUE(observer_.event_added_last_received_event_.empty());
 
   std::string event = "event";
   manager_.AddEvent(event);
 
-  EXPECT_EQ(&manager_, observer_.last_received_manager_);
+  EXPECT_EQ(&manager_, observer_.event_added_last_received_manager_);
   // A timestamp will be prepended to the event passed to |AddEvent|.
-  EXPECT_NE(std::string::npos, observer_.last_received_event_.find(event));
+  EXPECT_NE(std::string::npos,
+            observer_.event_added_last_received_event_.find(event));
+}
+
+// Tests that |BreadcumbManager::OldEventsRemoved| is called when old events are
+// dropped from |manager_|.
+TEST_F(BreadcrumbManagerObserverTest, OldEventsRemoved) {
+  ASSERT_FALSE(observer_.old_events_removed_last_received_manager_);
+
+  std::string event = "event";
+  manager_.AddEvent(event);
+  task_env_.FastForwardBy(base::TimeDelta::FromHours(1));
+  manager_.AddEvent(event);
+  task_env_.FastForwardBy(base::TimeDelta::FromHours(1));
+  manager_.AddEvent(event);
+
+  EXPECT_EQ(&manager_, observer_.old_events_removed_last_received_manager_);
 }

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_unittest.mm

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h"
+#include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h"
 
 #import "ios/web/public/test/web_task_environment.h"
 #include "testing/platform_test.h"
@@ -47,7 +47,7 @@ TEST_F(BreadcrumbManagerTest, EventCountLimited) {
   EXPECT_NE(std::string::npos, events.front().find("event4"));
 }
 
-// Tests that old events are dropped.
+// Tests that old event buckets are dropped.
 TEST_F(BreadcrumbManagerTest, OldEventsDropped) {
   // Log an event from one and two hours ago.
   breadcrumb_manager_.AddEvent("event1");
@@ -86,3 +86,19 @@ TEST_F(BreadcrumbManagerTest, MinimumEventsReturned) {
   std::list<std::string> events = breadcrumb_manager_.GetEvents(0);
   EXPECT_EQ(2ul, events.size());
 }
+
+// Tests that previous events are set as expected.
+TEST_F(BreadcrumbManagerTest, SetPreviousEvents) {
+  breadcrumb_manager_.SetPreviousEvents({"event1", "event2"});
+  ASSERT_EQ(2ul, breadcrumb_manager_.GetEvents(0).size());
+
+  task_env_.FastForwardBy(base::TimeDelta::FromMinutes(3));
+  breadcrumb_manager_.AddEvent("event3");
+
+  std::list<std::string> events = breadcrumb_manager_.GetEvents(0);
+  EXPECT_NE(std::string::npos, events.front().find("event1"));
+  events.pop_front();
+  EXPECT_NE(std::string::npos, events.front().find("event2"));
+  events.pop_front();
+  EXPECT_NE(std::string::npos, events.front().find("event3"));
+}

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_keyed_service.h

@@ -9,17 +9,26 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/timer/timer.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer.h"
-
-class BreadcrumbManager;
+#include "ios/chrome/browser/crash_report/crash_reporter_breadcrumb_constants.h"
 
 namespace web {
 class BrowserState;
-}
+}  // namespace web
+
+// The filesize for the file at |breadcrumbs_file_path_|. The file will always
+// be this constant size because it is accessed using a memory mapped file. The
+// file is twice as large as |kMaxBreadcrumbsDataLength| which leaves room for
+// appending breadcrumb events. Once the file is full of events, the contents
+// will be reduced to kMaxBreadcrumbsDataLength.
+constexpr size_t kPersistedFilesizeInBytes = kMaxBreadcrumbsDataLength * 2;
 
 // Saves and retrieves breadcrumb events to and from disk.
 class BreadcrumbPersistentStorageKeyedService
@@ -34,39 +43,58 @@ class BreadcrumbPersistentStorageKeyedService
       web::BrowserState* browser_state);
   ~BreadcrumbPersistentStorageKeyedService() override;
 
-  // Returns the stored breadcrumb events from disk.
-  std::vector<std::string> GetStoredEvents();
+  // Returns the stored breadcrumb events from disk to |callback|. If called
+  // before |StartStoringEvents|, these events (if any) will be from the prior
+  // application session. After |StartStoringEvents| has been called, the
+  // returned events will be from the current application session.
+  void GetStoredEvents(
+      base::OnceCallback<void(std::vector<std::string>)> callback);
 
-  // Sets the |manager| to observe. Old stored breadcrumbs will be removed, even
-  // if null is passed for |manager|. As such, this can be used to ensure stale
-  // data does not take up disk space. If manager is non-null, events will be
-  // written whenever |manager| receives an event.
-  void ObserveBreadcrumbManager(BreadcrumbManager* manager);
+  // Starts persisting breadcrumbs from the BreadcrumbManagerKeyedService
+  // associated with |browser_state_|. This will overwrite any breadcrumbs which
+  // may be stored from a previous application run.
+  void StartStoringEvents();
 
  private:
-  // Writes |observered_manager_|'s events to disk, overwriting any existing
-  // persisted breadcrumbs. Removes all stored events if |observered_manager_|
-  // is null.
-  void WriteExistingBreadcrumbEvents();
+  // Writes events from |observered_manager_| to |breadcrumbs_file_|,
+  // overwriting any existing persisted breadcrumbs.
+  void RewriteAllExistingBreadcrumbs();
 
-  // Appends |event| to |persisted_events_file_|. |persisted_events_file_| must
-  // already exist.
-  void WriteBreadcrumbEvent(const std::string& event);
+  // Writes breadcrumbs stored in |pending_breadcrumbs_| to |breadcrumbs_file_|.
+  void WritePendingBreadcrumbs();
 
   // BreadcrumbManagerObserver
   void EventAdded(BreadcrumbManager* manager,
                   const std::string& event) override;
+  void OldEventsRemoved(BreadcrumbManager* manager) override;
+
+  // KeyedService overrides
+  void Shutdown() override;
+
+  // Individual beadcrumbs which have not yet been written to disk.
+  std::string pending_breadcrumbs_;
 
-  // The breadcrumb manager currently being observed.
-  BreadcrumbManager* observered_manager_ = nullptr;
+  // The last time a breadcrumb was written to |breadcrumbs_file_|. This
+  // timestamp prevents breadcrumbs from being written to disk too often.
+  base::TimeTicks last_written_time_;
 
-  // The path to the breadcrumbs file.
+  // A timer to delay writing to disk too often.
+  base::OneShotTimer write_timer_;
+
+  // The associated browser state.
+  web::BrowserState* browser_state_ = nullptr;
+
+  // The path to the file for storing persisted breadcrumbs.
   base::FilePath breadcrumbs_file_path_;
 
-  // File handle for writing persisted breadcrumbs to |breadcrumbs_file_path_|.
-  // Lazily created after |ObserveBreadcrumbManager| is called with a non-null
-  // manager.
-  std::unique_ptr<base::File> persisted_events_file_;
+  // NOTE: Since this value represents the breadcrumbs written during this
+  // session, it will remain 0 until |StartStoringEvents| is called.
+  size_t current_mapped_file_position_ = 0;
+
+  // The SequencedTaskRunner on which File IO operations are performed.
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  base::WeakPtrFactory<BreadcrumbPersistentStorageKeyedService> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BreadcrumbPersistentStorageKeyedService);
 };

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_keyed_service_factory.cc

@@ -5,7 +5,6 @@
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_keyed_service_factory.h"
 
 #include "components/keyed_service/ios/browser_state_dependency_manager.h"
-#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_keyed_service.h"
 #include "ios/web/public/browser_state.h"
 
@@ -20,7 +19,7 @@ BreadcrumbPersistentStorageKeyedServiceFactory::GetInstance() {
 // static
 BreadcrumbPersistentStorageKeyedService*
 BreadcrumbPersistentStorageKeyedServiceFactory::GetForBrowserState(
-    ChromeBrowserState* browser_state) {
+    web::BrowserState* browser_state) {
   return static_cast<BreadcrumbPersistentStorageKeyedService*>(
       GetInstance()->GetServiceForBrowserState(browser_state, true));
 }

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_keyed_service_factory.h

@@ -9,14 +9,17 @@
 #include "components/keyed_service/ios/browser_state_keyed_service_factory.h"
 
 class BreadcrumbPersistentStorageKeyedService;
-class ChromeBrowserState;
+
+namespace web {
+class BrowserState;
+}  // namespace web
 
 class BreadcrumbPersistentStorageKeyedServiceFactory
     : public BrowserStateKeyedServiceFactory {
  public:
   static BreadcrumbPersistentStorageKeyedServiceFactory* GetInstance();
   static BreadcrumbPersistentStorageKeyedService* GetForBrowserState(
-      ChromeBrowserState* browser_state);
+      web::BrowserState* browser_state);
 
  private:
   friend class base::NoDestructor<

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_util.cc

@@ -12,9 +12,17 @@ namespace breadcrumb_persistent_storage_util {
 const base::FilePath::CharType kBreadcrumbsFile[] =
     FILE_PATH_LITERAL("iOS Breadcrumbs");
 
+const base::FilePath::CharType kBreadcrumbsTempFile[] =
+    FILE_PATH_LITERAL("iOS Breadcrumbs.temp");
+
 base::FilePath GetBreadcrumbPersistentStorageFilePath(
     web::BrowserState* browser_state) {
   return browser_state->GetStatePath().Append(kBreadcrumbsFile);
 }
 
+base::FilePath GetBreadcrumbPersistentStorageTempFilePath(
+    web::BrowserState* browser_state) {
+  return browser_state->GetStatePath().Append(kBreadcrumbsTempFile);
+}
+
 }  // namespace breadcrumb_persistent_storage_util

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_util.h

@@ -18,6 +18,17 @@ namespace breadcrumb_persistent_storage_util {
 base::FilePath GetBreadcrumbPersistentStorageFilePath(
     web::BrowserState* browser_state);
 
+// Returns the path to a file for storing breadcrumbs within |browser_state|'s
+// storage directory. This second file is used to write the new breadcrumbs to
+// so that the primary breadcrumbs file at
+// |GetBreadcrumbPersistentStorageFilePath()| is always in a state correctly
+// describing the application. (If the contents of a single file was instead
+// cleared and re-written, the most recent breadcrumbs would be missing if the
+// application crashed during this timeframe which will happen often whenever
+// old breadcrumbs are removed.)
+base::FilePath GetBreadcrumbPersistentStorageTempFilePath(
+    web::BrowserState* browser_state);
+
 }  // namespace breadcrumb_persistent_storage_util
 
 #endif  // IOS_CHROME_BROWSER_CRASH_REPORT_BREADCRUMBS_BREADCRUMB_PERSISTENT_STORAGE_UTIL_H_

ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_persistent_storage_util_unittest.mm

@@ -15,6 +15,9 @@
 using breadcrumb_persistent_storage_util::
     GetBreadcrumbPersistentStorageFilePath;
 
+using breadcrumb_persistent_storage_util::
+    GetBreadcrumbPersistentStorageTempFilePath;
+
 // Test fixture to test BreadcrumbPersistentStorageUtil.
 typedef ChromeWebTest BreadcrumbPersistentStorageUtilTest;
 
@@ -22,12 +25,20 @@ typedef ChromeWebTest BreadcrumbPersistentStorageUtilTest;
 TEST_F(BreadcrumbPersistentStorageUtilTest, UniqueStorage) {
   base::FilePath path1 =
       GetBreadcrumbPersistentStorageFilePath(GetBrowserState());
+  base::FilePath tempFilePath1 =
+      GetBreadcrumbPersistentStorageTempFilePath(GetBrowserState());
+  // Verify that the temp file path is different.
+  EXPECT_NE(path1, tempFilePath1);
+
   std::unique_ptr<TestChromeBrowserState> local_browser_state =
       TestChromeBrowserState::Builder().Build();
   base::FilePath path2 =
       GetBreadcrumbPersistentStorageFilePath(local_browser_state.get());
+  base::FilePath tempFilePath2 =
+      GetBreadcrumbPersistentStorageTempFilePath(local_browser_state.get());
 
   EXPECT_NE(path1, path2);
+  EXPECT_NE(tempFilePath1, tempFilePath2);
 }
 
 // Tests that the BrowserState returned by
@@ -36,10 +47,16 @@ TEST_F(BreadcrumbPersistentStorageUtilTest, UniqueStorage) {
 TEST_F(BreadcrumbPersistentStorageUtilTest, UniqueIncognitoStorage) {
   base::FilePath path1 =
       GetBreadcrumbPersistentStorageFilePath(GetBrowserState());
+  base::FilePath tempFilePath1 =
+      GetBreadcrumbPersistentStorageTempFilePath(GetBrowserState());
+
   ChromeBrowserState* off_the_record_browser_state =
       chrome_browser_state_->GetOffTheRecordChromeBrowserState();
   base::FilePath path2 =
       GetBreadcrumbPersistentStorageFilePath(off_the_record_browser_state);
+  base::FilePath tempFilePath2 =
+      GetBreadcrumbPersistentStorageTempFilePath(off_the_record_browser_state);
 
   EXPECT_NE(path1, path2);
+  EXPECT_NE(tempFilePath1, tempFilePath2);
 }

ios/chrome/browser/crash_report/breakpad_helper_unittest.mm

@@ -7,6 +7,7 @@
 #include "base/strings/sys_string_conversions.h"
 #import "ios/chrome/browser/crash_report/crash_keys_helper.h"
 #include "ios/chrome/browser/crash_report/crash_report_helper.h"
+#include "ios/chrome/browser/crash_report/crash_reporter_breadcrumb_constants.h"
 #include "ios/chrome/browser/crash_report/crash_reporter_breadcrumb_observer.h"
 #include "ios/chrome/browser/crash_report/main_thread_freeze_detector.h"
 #import "ios/chrome/test/ocmock/OCMockObject+BreakpadControllerTesting.h"

ios/chrome/browser/crash_report/crash_reporter_breadcrumb_constants.h

+// Copyright 2020 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 IOS_CHROME_BROWSER_CRASH_REPORT_CRASH_REPORTER_BREADCRUMB_CONSTANTS_H_
+#define IOS_CHROME_BROWSER_CRASH_REPORT_CRASH_REPORTER_BREADCRUMB_CONSTANTS_H_
+
+// The maximum string length for breadcrumbs data. The breadcrumbs size cannot
+// be larger than the maximum length of a single Breakpad product data value
+// (currently 2550 bytes). This value should be large enough to include enough
+//  events so that they are useful for diagnosing crashes.
+constexpr unsigned long kMaxBreadcrumbsDataLength = 1530;
+
+#endif  // IOS_CHROME_BROWSER_CRASH_REPORT_CRASH_REPORTER_BREADCRUMB_CONSTANTS_H_

ios/chrome/browser/crash_report/crash_reporter_breadcrumb_observer.h

@@ -12,9 +12,6 @@
 
 #import "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer_bridge.h"
 
-// The maximum string length for breadcrumbs data.
-extern const NSUInteger kMaxBreadcrumbsDataLength;
-
 // Combines breadcrumbs from multiple BreadcrumbManagers and sends the merged
 // breadcrumb events to breakpad for attachment to crash reports.
 @interface CrashReporterBreadcrumbObserver

ios/chrome/browser/crash_report/crash_reporter_breadcrumb_observer.mm

@@ -7,15 +7,12 @@
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager.h"
 #import "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_observer_bridge.h"
 #include "ios/chrome/browser/crash_report/crash_keys_helper.h"
+#include "ios/chrome/browser/crash_report/crash_reporter_breadcrumb_constants.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
-// The breadcrumbs size cannot be larger than the maximum length of a single
-// Breakpad product data value (currently 2550 bytes).
-const NSUInteger kMaxBreadcrumbsDataLength = 1530;
-
 @interface CrashReporterBreadcrumbObserver () {
   // Map associating the observed BreadcrumbManager with the corresponding
   // observer bridge instances.

ios/chrome/browser/crash_report/crash_reporter_breadcrumb_observer_unittest.mm

@@ -13,6 +13,7 @@
 #include "ios/chrome/browser/crash_report/breadcrumbs/breadcrumb_manager_keyed_service_factory.h"
 #import "ios/chrome/browser/crash_report/breakpad_helper.h"
 #include "ios/chrome/browser/crash_report/crash_keys_helper.h"
+#include "ios/chrome/browser/crash_report/crash_reporter_breadcrumb_constants.h"
 #import "ios/chrome/test/ocmock/OCMockObject+BreakpadControllerTesting.h"
 #import "ios/testing/scoped_block_swizzler.h"
 #include "testing/gtest/include/gtest/gtest.h"