[ios] Add metrics to understand if more tabs cause more crashes

Add metric which record number of tabs after clean shutdown and after the crash. If users have more tabs in metrics recorded after the crash than there is a correlation between number of tabs and crashiness. These metrics will help us understand if reducing memory footprint from multiple tabs may have positive impact on stability. Bug: 1140990 Change-Id: I07026ac9671e72d9367faf01b5fa71d8d990bdc5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2516240 Commit-Queue: Eugene But <eugenebut@chromium.org> Reviewed-by: David Jean <djean@chromium.org> Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Cr-Commit-Position: refs/heads/master@{#824170}
marcosholgado · Nov 4, 2020 · 59c0dda · 59c0dda
1 parent 48cf43e
commit 59c0dda
Show file tree

Hide file tree

Showing 8 changed files with 291 additions and 1 deletion.
diff --git a/components/previous_session_info/previous_session_info.h b/components/previous_session_info/previous_session_info.h
@@ -28,6 +28,10 @@ extern NSString* const kPreviousSessionInfoConnectedSceneSessionIDs;
 extern NSString* const kPreviousSessionInfoParams;
 // Key in the UserDefaults for the memory footprint of the browser process.
 extern NSString* const kPreviousSessionInfoMemoryFootprint;
+// Key in the UserDefaults for the number of open tabs.
+extern NSString* const kPreviousSessionInfoTabCount;
+// Key in the UserDefaults for the number of open "off the record" tabs.
+extern NSString* const kPreviousSessionInfoOTRTabCount;
 
 // The values of this enum are persisted (both to NSUserDefaults and logs) and
 // represent the state of the last session (which may have been running a
@@ -143,6 +147,12 @@ enum class DeviceBatteryState {
 // session.
 @property(nonatomic, readonly) BOOL applicationWillTerminateWasReceived;
 
+// Number of open tabs in the previous session.
+@property(nonatomic, readonly) NSInteger tabCount;
+
+// Number of open "off the record" tabs in the previous session.
+@property(nonatomic, readonly) NSInteger OTRTabCount;
+
 // Singleton PreviousSessionInfo. During the lifetime of the app, the returned
 // object is the same, and describes the previous session, even after a new
 // session has started (by calling beginRecordingCurrentSession).
@@ -208,6 +218,11 @@ enum class DeviceBatteryState {
 // gets destructed.
 - (void)resetSessionRestorationFlag;
 
+// Records number of regular (non off the record) tabs.
+- (void)updateCurrentSessionTabCount:(NSInteger)count;
+// Records number of off the record tabs.
+- (void)updateCurrentSessionOTRTabCount:(NSInteger)count;
+
 // Records information crash report parameters.
 - (void)setReportParameterValue:(NSString*)value forKey:(NSString*)key;
 - (void)setReportParameterURL:(const GURL&)URL forKey:(NSString*)key;

diff --git a/components/previous_session_info/previous_session_info.mm b/components/previous_session_info/previous_session_info.mm
@@ -115,6 +115,9 @@ DeviceThermalState GetThermalStateFromNSProcessInfoThermalState(
 NSString* const kPreviousSessionInfoParams = @"PreviousSessionInfoParams";
 NSString* const kPreviousSessionInfoMemoryFootprint =
     @"PreviousSessionInfoMemoryFootprint";
+NSString* const kPreviousSessionInfoTabCount = @"PreviousSessionInfoTabCount";
+NSString* const kPreviousSessionInfoOTRTabCount =
+    @"PreviousSessionInfoOTRTabCount";
 }  // namespace previous_session_info_constants
 
 @interface PreviousSessionInfo ()
@@ -148,6 +151,9 @@ @interface PreviousSessionInfo ()
 @property(nonatomic, copy) NSDictionary<NSString*, NSString*>* reportParameters;
 @property(nonatomic, assign) NSInteger memoryFootprint;
 @property(nonatomic, assign) BOOL applicationWillTerminateWasReceived;
+@property(nonatomic, assign) NSInteger tabCount;
+@property(nonatomic, assign) NSInteger OTRTabCount;
+
 @end
 
 @implementation PreviousSessionInfo {
@@ -245,6 +251,12 @@ + (instancetype)sharedInstance {
 
     gSharedInstance.applicationWillTerminateWasReceived =
         [defaults boolForKey:kPreviousSessionInfoAppWillTerminate];
+    gSharedInstance.tabCount =
+        [defaults integerForKey:previous_session_info_constants::
+                                    kPreviousSessionInfoTabCount];
+    gSharedInstance.OTRTabCount =
+        [defaults integerForKey:previous_session_info_constants::
+                                    kPreviousSessionInfoOTRTabCount];
   }
   return gSharedInstance;
 }
@@ -586,6 +598,21 @@ - (void)resetSessionRestorationFlag {
   [NSUserDefaults.standardUserDefaults synchronize];
 }
 
+- (void)updateCurrentSessionTabCount:(NSInteger)count {
+  [NSUserDefaults.standardUserDefaults
+      setInteger:count
+          forKey:previous_session_info_constants::kPreviousSessionInfoTabCount];
+  [NSUserDefaults.standardUserDefaults synchronize];
+}
+
+- (void)updateCurrentSessionOTRTabCount:(NSInteger)count {
+  [NSUserDefaults.standardUserDefaults
+      setInteger:count
+          forKey:previous_session_info_constants::
+                     kPreviousSessionInfoOTRTabCount];
+  [NSUserDefaults.standardUserDefaults synchronize];
+}
+
 - (void)setReportParameterValue:(NSString*)value forKey:(NSString*)key {
   NSMutableDictionary* params = [[NSUserDefaults.standardUserDefaults
       dictionaryForKey:previous_session_info_constants::

diff --git a/components/previous_session_info/previous_session_info_private.h b/components/previous_session_info/previous_session_info_private.h
@@ -24,6 +24,8 @@
 @property(nonatomic, strong) NSMutableSet<NSString*>* connectedSceneSessionsIDs;
 @property(nonatomic, copy) NSDictionary<NSString*, NSString*>* reportParameters;
 @property(nonatomic, assign) NSInteger memoryFootprint;
+@property(nonatomic, assign) NSInteger tabCount;
+@property(nonatomic, assign) NSInteger OTRTabCount;
 
 + (void)resetSharedInstanceForTesting;
 

diff --git a/components/previous_session_info/previous_session_info_unittest.mm b/components/previous_session_info/previous_session_info_unittest.mm
@@ -21,9 +21,13 @@
 using previous_session_info_constants::kPreviousSessionInfoRestoringSession;
 using previous_session_info_constants::
     kPreviousSessionInfoConnectedSceneSessionIDs;
+using previous_session_info_constants::kPreviousSessionInfoTabCount;
+using previous_session_info_constants::kPreviousSessionInfoOTRTabCount;
 
 namespace {
 
+const NSInteger kTabCount = 15;
+
 // Key in the UserDefaults for a boolean value keeping track of memory warnings.
 NSString* const kDidSeeMemoryWarningShortlyBeforeTerminating =
     previous_session_info_constants::
@@ -579,6 +583,54 @@
   }));
 }
 
+// Tests tabCount property.
+TEST_F(PreviousSessionInfoTest, TabCount) {
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  [NSUserDefaults.standardUserDefaults setInteger:kTabCount
+                                           forKey:kPreviousSessionInfoTabCount];
+
+  [[PreviousSessionInfo sharedInstance] beginRecordingCurrentSession];
+  EXPECT_EQ(kTabCount, [PreviousSessionInfo sharedInstance].tabCount);
+}
+
+// Tests tab count gets written to NSUserDefaults.
+TEST_F(PreviousSessionInfoTest, TabCountRecording) {
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  [NSUserDefaults.standardUserDefaults
+      removeObjectForKey:kPreviousSessionInfoTabCount];
+
+  [[PreviousSessionInfo sharedInstance] beginRecordingCurrentSession];
+  [[PreviousSessionInfo sharedInstance] updateCurrentSessionTabCount:kTabCount];
+
+  EXPECT_NSEQ(@(kTabCount), [NSUserDefaults.standardUserDefaults
+                                objectForKey:kPreviousSessionInfoTabCount]);
+}
+
+// Tests OTRTabCount property.
+TEST_F(PreviousSessionInfoTest, OtrTabCount) {
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  [NSUserDefaults.standardUserDefaults
+      setInteger:kTabCount
+          forKey:kPreviousSessionInfoOTRTabCount];
+
+  [[PreviousSessionInfo sharedInstance] beginRecordingCurrentSession];
+  EXPECT_EQ(kTabCount, [PreviousSessionInfo sharedInstance].OTRTabCount);
+}
+
+// Tests OTR tab count gets written to NSUserDefaults.
+TEST_F(PreviousSessionInfoTest, OtrTabCountRecording) {
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+  [NSUserDefaults.standardUserDefaults
+      removeObjectForKey:kPreviousSessionInfoOTRTabCount];
+
+  [[PreviousSessionInfo sharedInstance] beginRecordingCurrentSession];
+  [[PreviousSessionInfo sharedInstance]
+      updateCurrentSessionOTRTabCount:kTabCount];
+
+  EXPECT_NSEQ(@(kTabCount), [NSUserDefaults.standardUserDefaults
+                                objectForKey:kPreviousSessionInfoOTRTabCount]);
+}
+
 // Tests memoryFootprint property.
 TEST_F(PreviousSessionInfoTest, MemoryFootprint) {
   [PreviousSessionInfo resetSharedInstanceForTesting];

diff --git a/ios/chrome/browser/crash_report/crash_keys_helper.mm b/ios/chrome/browser/crash_report/crash_keys_helper.mm
@@ -5,6 +5,7 @@
 #import "ios/chrome/browser/crash_report/crash_keys_helper.h"
 
 #include "base/check.h"
+#import "components/previous_session_info/previous_session_info.h"
 #import "ios/chrome/browser/crash_report/breakpad_helper.h"
 #import "ios/chrome/browser/crash_report/crash_report_user_application_state.h"
 #import "ios/chrome/browser/crash_report/main_thread_freeze_detector.h"
@@ -123,11 +124,14 @@ void SetCurrentlySignedIn(bool signedIn) {
 void SetRegularTabCount(int tabCount) {
   [[CrashReportUserApplicationState sharedInstance] setValue:kRegularTabCount
                                                    withValue:tabCount];
+  [[PreviousSessionInfo sharedInstance] updateCurrentSessionTabCount:tabCount];
 }
 
 void SetIncognitoTabCount(int tabCount) {
   [[CrashReportUserApplicationState sharedInstance] setValue:kIncognitoTabCount
                                                    withValue:tabCount];
+  [[PreviousSessionInfo sharedInstance]
+      updateCurrentSessionOTRTabCount:tabCount];
 }
 
 void SetDestroyingAndRebuildingIncognitoBrowserState(bool in_progress) {

diff --git a/ios/chrome/browser/metrics/mobile_session_shutdown_metrics_provider.mm b/ios/chrome/browser/metrics/mobile_session_shutdown_metrics_provider.mm
@@ -285,12 +285,18 @@ void CreateSyntheticCrashReportWithBreadcrumbs(
     return;
   }
 
+  PreviousSessionInfo* session_info = [PreviousSessionInfo sharedInstance];
+  NSInteger allTabCount = session_info.tabCount + session_info.OTRTabCount;
   // Do not log UTE metrics if the application terminated cleanly.
   if (shutdown_type == SHUTDOWN_IN_BACKGROUND) {
+    UMA_STABILITY_HISTOGRAM_COUNTS_100(
+        "Stability.iOS.TabCountBeforeCleanShutdown", allTabCount);
     return;
   }
 
-  PreviousSessionInfo* session_info = [PreviousSessionInfo sharedInstance];
+  UMA_STABILITY_HISTOGRAM_COUNTS_100("Stability.iOS.TabCountBeforeCrash",
+                                     allTabCount);
+
   // Log metrics to improve categorization of crashes.
   LogApplicationBackgroundedTime(session_info.sessionEndTime);
 
@@ -315,6 +321,9 @@ void CreateSyntheticCrashReportWithBreadcrumbs(
         "Stability.iOS.UTE.OSRestartedAfterPreviousSession",
         session_info.OSRestartedAfterPreviousSession);
 
+    UMA_STABILITY_HISTOGRAM_COUNTS_100("Stability.iOS.TabCountBeforeUTE",
+                                       allTabCount);
+
     bool possible_explanation =
         // Log any of the following cases as a possible explanation for the
         // crash:
@@ -356,6 +365,15 @@ void CreateSyntheticCrashReportWithBreadcrumbs(
           ->GetStoredEvents(
               base::BindOnce(CreateSyntheticCrashReportWithBreadcrumbs));
     }
+  } else if (shutdown_type ==
+                 SHUTDOWN_IN_FOREGROUND_WITH_CRASH_LOG_NO_MEMORY_WARNING ||
+             shutdown_type ==
+                 SHUTDOWN_IN_FOREGROUND_WITH_CRASH_LOG_WITH_MEMORY_WARNING) {
+    UMA_STABILITY_HISTOGRAM_COUNTS_100(
+        "Stability.iOS.TabCountBeforeSignalCrash", allTabCount);
+  } else if (shutdown_type == SHUTDOWN_IN_FOREGROUND_WITH_MAIN_THREAD_FROZEN) {
+    UMA_STABILITY_HISTOGRAM_COUNTS_100("Stability.iOS.TabCountBeforeFreeze",
+                                       allTabCount);
   }
   [session_info resetSessionRestorationFlag];
 }

diff --git a/ios/chrome/browser/metrics/mobile_session_shutdown_metrics_provider_unittest.mm b/ios/chrome/browser/metrics/mobile_session_shutdown_metrics_provider_unittest.mm
@@ -180,6 +180,155 @@ bool ReceivedMemoryWarningBeforeLastShutdown() override {
   [PreviousSessionInfo resetSharedInstanceForTesting];
 }
 
+// Tests Stability.iOS.TabCountBefore* metrics recording after clean shutdown.
+TEST_F(MobileSessionShutdownMetricsProviderTest, TabCountMetricCleanShutdown) {
+  // Setup the MetricsService.
+  [PreviousSessionInfo sharedInstance].tabCount = 2;
+  [PreviousSessionInfo sharedInstance].OTRTabCount = 3;
+  local_state_.SetBoolean(metrics::prefs::kStabilityExitedCleanly, true);
+  metrics_state_ = metrics::MetricsStateManager::Create(
+      &local_state_, new metrics::TestEnabledStateProvider(false, false),
+      base::string16(), metrics::MetricsStateManager::StoreClientInfoCallback(),
+      metrics::MetricsStateManager::LoadClientInfoCallback());
+  metrics_service_.reset(new metrics::MetricsService(
+      metrics_state_.get(), &metrics_client_, &local_state_));
+
+  // Create the metrics provider to test.
+  metrics_provider_.reset(new MobileSessionShutdownMetricsProviderForTesting(
+      metrics_service_.get()));
+
+  // Create a histogram tester for verifying samples written to the shutdown
+  // type histogram.
+  base::HistogramTester histogram_tester;
+
+  // Now call the method under test and verify exactly one sample is written to
+  // the expected bucket.
+  metrics_provider_->ProvidePreviousSessionData(nullptr);
+  histogram_tester.ExpectUniqueSample(
+      "Stability.iOS.TabCountBeforeCleanShutdown", 5, 1);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeCrash", 0);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeUTE", 0);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeSignalCrash",
+                                    0);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeFreeze", 0);
+
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+}
+
+// Tests Stability.iOS.TabCountBefore* metrics recording after
+// Unexplained Termination Event or Explained Termination Event.
+TEST_F(MobileSessionShutdownMetricsProviderTest, TabCountMetricUte) {
+  // Setup the MetricsService.
+  [PreviousSessionInfo sharedInstance].tabCount = 2;
+  [PreviousSessionInfo sharedInstance].OTRTabCount = 3;
+  local_state_.SetBoolean(metrics::prefs::kStabilityExitedCleanly, false);
+  metrics_state_ = metrics::MetricsStateManager::Create(
+      &local_state_, new metrics::TestEnabledStateProvider(false, false),
+      base::string16(), metrics::MetricsStateManager::StoreClientInfoCallback(),
+      metrics::MetricsStateManager::LoadClientInfoCallback());
+  metrics_service_.reset(new metrics::MetricsService(
+      metrics_state_.get(), &metrics_client_, &local_state_));
+
+  // Create the metrics provider to test.
+  metrics_provider_.reset(new MobileSessionShutdownMetricsProviderForTesting(
+      metrics_service_.get()));
+
+  // Create a histogram tester for verifying samples written to the shutdown
+  // type histogram.
+  base::HistogramTester histogram_tester;
+
+  // Now call the method under test and verify exactly one sample is written to
+  // the expected bucket.
+  metrics_provider_->ProvidePreviousSessionData(nullptr);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeCleanShutdown",
+                                    0);
+  histogram_tester.ExpectUniqueSample("Stability.iOS.TabCountBeforeCrash", 5,
+                                      1);
+  histogram_tester.ExpectUniqueSample("Stability.iOS.TabCountBeforeUTE", 5, 1);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeSignalCrash",
+                                    0);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeFreeze", 0);
+
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+}
+
+// Tests Stability.iOS.TabCountBefore* metrics recording after crash with log.
+TEST_F(MobileSessionShutdownMetricsProviderTest, TabCountMetricCrashWithLog) {
+  // Setup the MetricsService.
+  [PreviousSessionInfo sharedInstance].tabCount = 2;
+  [PreviousSessionInfo sharedInstance].OTRTabCount = 3;
+  local_state_.SetBoolean(metrics::prefs::kStabilityExitedCleanly, false);
+  metrics_state_ = metrics::MetricsStateManager::Create(
+      &local_state_, new metrics::TestEnabledStateProvider(false, false),
+      base::string16(), metrics::MetricsStateManager::StoreClientInfoCallback(),
+      metrics::MetricsStateManager::LoadClientInfoCallback());
+  metrics_service_.reset(new metrics::MetricsService(
+      metrics_state_.get(), &metrics_client_, &local_state_));
+
+  // Create the metrics provider to test.
+  metrics_provider_.reset(new MobileSessionShutdownMetricsProviderForTesting(
+      metrics_service_.get()));
+
+  metrics_provider_->set_has_crash_logs(true);
+
+  // Create a histogram tester for verifying samples written to the shutdown
+  // type histogram.
+  base::HistogramTester histogram_tester;
+
+  // Now call the method under test and verify exactly one sample is written to
+  // the expected bucket.
+  metrics_provider_->ProvidePreviousSessionData(nullptr);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeCleanShutdown",
+                                    0);
+  histogram_tester.ExpectUniqueSample("Stability.iOS.TabCountBeforeCrash", 5,
+                                      1);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeUTE", 0);
+  histogram_tester.ExpectUniqueSample("Stability.iOS.TabCountBeforeSignalCrash",
+                                      5, 1);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeFreeze", 0);
+
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+}
+
+// Tests Stability.iOS.TabCountBefore* metrics recording after UI Freeze.
+TEST_F(MobileSessionShutdownMetricsProviderTest, TabCountMetricFreeze) {
+  // Setup the MetricsService.
+  [PreviousSessionInfo sharedInstance].tabCount = 2;
+  [PreviousSessionInfo sharedInstance].OTRTabCount = 3;
+  local_state_.SetBoolean(metrics::prefs::kStabilityExitedCleanly, false);
+  metrics_state_ = metrics::MetricsStateManager::Create(
+      &local_state_, new metrics::TestEnabledStateProvider(false, false),
+      base::string16(), metrics::MetricsStateManager::StoreClientInfoCallback(),
+      metrics::MetricsStateManager::LoadClientInfoCallback());
+  metrics_service_.reset(new metrics::MetricsService(
+      metrics_state_.get(), &metrics_client_, &local_state_));
+
+  // Create the metrics provider to test.
+  metrics_provider_.reset(new MobileSessionShutdownMetricsProviderForTesting(
+      metrics_service_.get()));
+
+  metrics_provider_->set_was_last_shutdown_frozen(true);
+
+  // Create a histogram tester for verifying samples written to the shutdown
+  // type histogram.
+  base::HistogramTester histogram_tester;
+
+  // Now call the method under test and verify exactly one sample is written to
+  // the expected bucket.
+  metrics_provider_->ProvidePreviousSessionData(nullptr);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeCleanShutdown",
+                                    0);
+  histogram_tester.ExpectUniqueSample("Stability.iOS.TabCountBeforeCrash", 5,
+                                      1);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeUTE", 0);
+  histogram_tester.ExpectTotalCount("Stability.iOS.TabCountBeforeSignalCrash",
+                                    0);
+  histogram_tester.ExpectUniqueSample("Stability.iOS.TabCountBeforeFreeze", 5,
+                                      1);
+
+  [PreviousSessionInfo resetSharedInstanceForTesting];
+}
+
 // Tests logging the following metrics:
 //   - Stability.iOS.UTE.HasPossibleExplanation
 //   - Stability.iOS.UTE.OSRestartedAfterPreviousSession