ios: Attempt to reduce hangs that recover.

There's some speculation that MainThreadFreezeDetector is catching too
many hangs that eventually recover, especially on change from foreground
to background.  If a hang report is generated, but the app recovers,
return early before |reportGenerated| can be set.


Change-Id: Ib4a4f9b1db44eeb42ce15da7f312f4df0033e401
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2536386
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Auto-Submit: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833884}

ios/chrome/browser/crash_report/breakpad_helper.h

@@ -62,13 +62,6 @@ void StartUploadingReportsInRecoveryMode();
 // Resets the Breakpad configuration from the main bundle.
 void RestoreDefaultConfiguration();
 
-// Sets a key indicating that UI thread is frozen (if value is 'true'),
-// otherwise remove the key.
-// Setting the value is synchronous as it is expected to be set just before the
-// report generation.
-// Unsetting the value is asynchronous.
-void SetHangReport(bool value);
-
 }  // namespace breakpad_helper
 
 #endif  // IOS_CHROME_BROWSER_CRASH_REPORT_BREAKPAD_HELPER_H_

ios/chrome/browser/crash_report/breakpad_helper.mm

@@ -46,8 +46,6 @@ NSString* const kCrashReportsUploadingEnabledKey =
 NSString* const kUptimeAtRestoreInMs = @"uptime_at_restore_in_ms";
 NSString* const kUploadedInRecoveryMode = @"uploaded_in_recovery_mode";
 
-NSString* const kHangReport = @"hang-report";
-
 void DeleteAllReportsInDirectory(base::FilePath directory) {
   base::FileEnumerator enumerator(directory, false,
                                   base::FileEnumerator::FILES);
@@ -284,11 +282,4 @@ void RestoreDefaultConfiguration() {
   [[BreakpadController sharedInstance] setUploadingEnabled:NO];
 }
 
-void SetHangReport(bool value) {
-  if (value)
-    AddReportParameter(kHangReport, @"yes", false);
-  else
-    RemoveReportParameter(kHangReport);
-}
-
 }  // namespace breakpad_helper

ios/chrome/browser/crash_report/breakpad_helper_unittest.mm

@@ -63,7 +63,6 @@ TEST_F(BreakpadHelperTest, CrashReportUserApplicationStateAllKeys) {
   // single breakpad record. This test should include all keys for
   // CrashReportUserApplicationState, since the whole dictionary is considered a
   // single breakpad record.
-  breakpad_helper::SetHangReport(true);
   crash_keys::SetCurrentlyInBackground(true);
   crash_keys::SetCurrentlySignedIn(true);
   crash_keys::SetMemoryWarningCount(2);

ios/chrome/browser/crash_report/main_thread_freeze_detector.mm

@@ -26,7 +26,23 @@ void LogRecoveryTime(base::TimeDelta time) {
   UMA_HISTOGRAM_TIMES("IOS.MainThreadFreezeDetection.RecoveredAfter", time);
 }
 
-}
+// Key indicating that UI thread is frozen.
+NSString* const kHangReportKey = @"hang-report";
+
+// Key of the UMA Startup.MobileSessionStartAction histogram.
+const char kUMAMainThreadFreezeDetectionNotRunningAfterReport[] =
+    "IOS.MainThreadFreezeDetection.NotRunningAfterReport";
+
+// Enum actions for the IOS.MainThreadFreezeDetection.NotRunningAfterReport UMA
+// metric. These values are persisted to logs. Entries should not be renumbered
+// and numeric values should never be reused.
+enum class IOSMainThreadFreezeDetectionNotRunningAfterReportBlock {
+  kAfterBreakpadRef = 0,
+  kAfterFileManagerUTEMove = 1,
+  kMaxValue = kAfterFileManagerUTEMove,
+};
+
+}  // namespace
 
 @interface MainThreadFreezeDetector ()
 // The callback that is called regularly on main thread.
@@ -127,24 +143,23 @@ void LogRecoveryTime(base::TimeDelta time) {
 }
 
 - (void)runInMainLoop {
+  NSDate* oldLastSeenMainThread = self.lastSeenMainThread;
+  self.lastSeenMainThread = [NSDate date];
   if (self.reportGenerated) {
     self.reportGenerated = NO;
     // Remove information about the last session info.
     [[NSUserDefaults standardUserDefaults]
         removeObjectForKey:@(kNsUserDefaultKeyLastSessionInfo)];
     LogRecoveryTime(base::TimeDelta::FromSecondsD(
-        [[NSDate date] timeIntervalSinceDate:self.lastSeenMainThread]));
+        [[NSDate date] timeIntervalSinceDate:oldLastSeenMainThread]));
     // Restart the freeze detection.
-    dispatch_after(
-        dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)),
-        _freezeDetectionQueue, ^{
+    dispatch_async(_freezeDetectionQueue, ^{
       [self cleanAndRunInFreezeDetectionQueue];
     });
   }
   if (!self.running) {
     return;
   }
-  self.lastSeenMainThread = [NSDate date];
   dispatch_after(
       dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.1 * NSEC_PER_SEC)),
       dispatch_get_main_queue(), ^{
@@ -168,14 +183,22 @@ void LogRecoveryTime(base::TimeDelta time) {
   }
   if ([[NSDate date] timeIntervalSinceDate:self.lastSeenMainThread] >
       self.delay) {
-    breakpad_helper::SetHangReport(true);
     [[BreakpadController sharedInstance]
         withBreakpadRef:^(BreakpadRef breakpadRef) {
+          if (!self.running) {
+            UMA_HISTOGRAM_ENUMERATION(
+                kUMAMainThreadFreezeDetectionNotRunningAfterReport,
+                IOSMainThreadFreezeDetectionNotRunningAfterReportBlock::
+                    kAfterBreakpadRef);
+            return;
+          }
           if (!breakpadRef) {
             return;
           }
+          BreakpadAddUploadParameter(breakpadRef, kHangReportKey, @"yes");
           NSDictionary* breakpadReportInfo =
               BreakpadGenerateReport(breakpadRef, nil);
+          BreakpadRemoveUploadParameter(breakpadRef, kHangReportKey);
           if (!breakpadReportInfo) {
             return;
           }
@@ -201,6 +224,13 @@ void LogRecoveryTime(base::TimeDelta time) {
                                toPath:UTEConfigFile
                                 error:nil];
           [fileManager moveItemAtPath:dumpFile toPath:UTEDumpFile error:nil];
+          if (!self.running) {
+            UMA_HISTOGRAM_ENUMERATION(
+                kUMAMainThreadFreezeDetectionNotRunningAfterReport,
+                IOSMainThreadFreezeDetectionNotRunningAfterReportBlock::
+                    kAfterFileManagerUTEMove);
+            return;
+          }
           [[NSUserDefaults standardUserDefaults]
               setObject:@{
                 @"dump" : [dumpFile lastPathComponent],
@@ -209,11 +239,11 @@ void LogRecoveryTime(base::TimeDelta time) {
               }
                  forKey:@(kNsUserDefaultKeyLastSessionInfo)];
           self.reportGenerated = YES;
-          breakpad_helper::SetHangReport(false);
         }];
     return;
   }
-  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)),
+  dispatch_after(
+      dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.5 * NSEC_PER_SEC)),
       _freezeDetectionQueue, ^{
         [self runInFreezeDetectionQueue];
       });

tools/metrics/histograms/enums.xml

@@ -39056,6 +39056,16 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
   <int value="4" label="Authorized when Chrome is in use"/>
 </enum>
 
+<enum name="IOSMainThreadFreezeDetectionNotRunningAfterReportBlock">
+  <int value="0" label="After Breakpad Ref">
+    Freeze detection stopped after obtaining the breakpad lock.
+  </int>
+  <int value="1" label="After FileManager UTE move">
+    Freeze detection stopped after moving the hang report out of the Breakpad
+    directory and into the |UTE| directory.
+  </int>
+</enum>
+
 <enum name="IOSMenuAction">
   <int value="0" label="Open in New Tab"/>
   <int value="1" label="Open in New Incognito Tab"/>

tools/metrics/histograms/histograms_xml/ios/histograms.xml

@@ -423,6 +423,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="IOS.MainThreadFreezeDetection.NotRunningAfterReport"
+    enum="IOSMainThreadFreezeDetectionNotRunningAfterReportBlock"
+    expires_after="2021-07-01">
+  <owner>justincohen@chromium.org</owner>
+  <owner>olivierrobin@chromium.org</owner>
+  <summary>
+    Recorded when the main thread recovers immediately after a freeze report was
+    generated.
+  </summary>
+</histogram>
+
 <histogram name="IOS.MainThreadFreezeDetection.RecoveredAfter" units="ms"
     expires_after="2021-11-12">
   <owner>eugenebut@chromium.org</owner>