Commit e3523ee3 authored by Eugene But's avatar Eugene But Committed by Commit Bot

Fix Stability.iOS.UTE.HasPossibleExplanation logging condition

Previously Stability.iOS.UTE.HasPossibleExplanation would always be true
if OSRestartedAfterPreviousSession is true. But according to M78 UMA
OSRestartedAfterPreviousSession is true in more than 50% of UTEs, which
suggests that most restarts don't actually cause UTE.

Relax the logic to take into account OSRestartedAfterPreviousSession
only if battery level at 1%.

Also remove deviceWasInLowPowerMode from logging condition as it seem
irrelevant.

Bug: 1006928
Change-Id: I583f0aaeff842ea058a54315dd27d79537242296
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1819884Reviewed-by: default avatarMike Dougherty <michaeldo@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Auto-Submit: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699377}
parent b2f9941d
...@@ -27,6 +27,11 @@ enum MobileSessionShutdownType { ...@@ -27,6 +27,11 @@ enum MobileSessionShutdownType {
MOBILE_SESSION_SHUTDOWN_TYPE_COUNT, MOBILE_SESSION_SHUTDOWN_TYPE_COUNT,
}; };
// Percentage of battery level which is assumed low enough to have possibly
// been the reason for the previous session ending in an unclean shutdown.
// Percent rpresented by a value between 0 and 1.
extern const float kCriticallyLowBatteryLevel;
class MobileSessionShutdownMetricsProvider : public metrics::MetricsProvider { class MobileSessionShutdownMetricsProvider : public metrics::MetricsProvider {
public: public:
explicit MobileSessionShutdownMetricsProvider( explicit MobileSessionShutdownMetricsProvider(
......
...@@ -110,6 +110,8 @@ void LogDeviceThermalState(DeviceThermalState thermal_state) { ...@@ -110,6 +110,8 @@ void LogDeviceThermalState(DeviceThermalState thermal_state) {
} }
} // namespace } // namespace
const float kCriticallyLowBatteryLevel = 0.01;
MobileSessionShutdownMetricsProvider::MobileSessionShutdownMetricsProvider( MobileSessionShutdownMetricsProvider::MobileSessionShutdownMetricsProvider(
metrics::MetricsService* metrics_service) metrics::MetricsService* metrics_service)
: metrics_service_(metrics_service) { : metrics_service_(metrics_service) {
...@@ -172,15 +174,15 @@ void MobileSessionShutdownMetricsProvider::ProvidePreviousSessionData( ...@@ -172,15 +174,15 @@ void MobileSessionShutdownMetricsProvider::ProvidePreviousSessionData(
bool possible_explanation = bool possible_explanation =
// Log any of the following cases as a possible explanation for the // Log any of the following cases as a possible explanation for the
// crash: // crash:
// - device restarted while the battery was critically low
(session_info.deviceBatteryState == DeviceBatteryState::kUnplugged &&
session_info.deviceBatteryLevel <= kCriticallyLowBatteryLevel &&
session_info.OSRestartedAfterPreviousSession) ||
// - storage was critically low // - storage was critically low
(session_info.availableDeviceStorage >= 0 && (session_info.availableDeviceStorage >= 0 &&
session_info.availableDeviceStorage <= kCriticallyLowDeviceStorage) || session_info.availableDeviceStorage <= kCriticallyLowDeviceStorage) ||
// - OS version changed // - OS version changed
session_info.isFirstSessionAfterOSUpgrade || session_info.isFirstSessionAfterOSUpgrade ||
// - OS was restarted
session_info.OSRestartedAfterPreviousSession ||
// - low power mode enabled
session_info.deviceWasInLowPowerMode ||
// - device in abnormal thermal state // - device in abnormal thermal state
session_info.deviceThermalState == DeviceThermalState::kCritical || session_info.deviceThermalState == DeviceThermalState::kCritical ||
session_info.deviceThermalState == DeviceThermalState::kSerious; session_info.deviceThermalState == DeviceThermalState::kSerious;
......
...@@ -206,7 +206,10 @@ TEST_F(MobileSessionShutdownMetricsProviderTest, ...@@ -206,7 +206,10 @@ TEST_F(MobileSessionShutdownMetricsProviderTest,
"Stability.iOS.UTE.HasPossibleExplanation", false, 1); "Stability.iOS.UTE.HasPossibleExplanation", false, 1);
// Test UTE with low battery when OS did not restart. // Test UTE with low battery when OS did not restart.
[PreviousSessionInfo sharedInstance].deviceBatteryLevel = 0.01; [PreviousSessionInfo sharedInstance].deviceBatteryLevel =
kCriticallyLowBatteryLevel;
[PreviousSessionInfo sharedInstance].deviceBatteryState =
previous_session_info_constants::DeviceBatteryState::kUnplugged;
histogram_tester = std::make_unique<base::HistogramTester>(); histogram_tester = std::make_unique<base::HistogramTester>();
metrics_provider_->ProvidePreviousSessionData(nullptr); metrics_provider_->ProvidePreviousSessionData(nullptr);
histogram_tester->ExpectUniqueSample( histogram_tester->ExpectUniqueSample(
...@@ -222,6 +225,15 @@ TEST_F(MobileSessionShutdownMetricsProviderTest, ...@@ -222,6 +225,15 @@ TEST_F(MobileSessionShutdownMetricsProviderTest,
"Stability.iOS.UTE.OSRestartedAfterPreviousSession", true, 1); "Stability.iOS.UTE.OSRestartedAfterPreviousSession", true, 1);
histogram_tester->ExpectUniqueSample( histogram_tester->ExpectUniqueSample(
"Stability.iOS.UTE.HasPossibleExplanation", true, 1); "Stability.iOS.UTE.HasPossibleExplanation", true, 1);
// Test UTE with normal battery when OS restarted after previous session.
[PreviousSessionInfo sharedInstance].deviceBatteryLevel = 50;
histogram_tester = std::make_unique<base::HistogramTester>();
metrics_provider_->ProvidePreviousSessionData(nullptr);
histogram_tester->ExpectUniqueSample(
"Stability.iOS.UTE.OSRestartedAfterPreviousSession", true, 1);
histogram_tester->ExpectUniqueSample(
"Stability.iOS.UTE.HasPossibleExplanation", false, 1);
} }
INSTANTIATE_TEST_SUITE_P(/* No InstantiationName */, INSTANTIATE_TEST_SUITE_P(/* No InstantiationName */,
......
...@@ -11,6 +11,8 @@ ...@@ -11,6 +11,8 @@
@property(nonatomic, assign) BOOL didSeeMemoryWarningShortlyBeforeTerminating; @property(nonatomic, assign) BOOL didSeeMemoryWarningShortlyBeforeTerminating;
@property(nonatomic, assign) BOOL isFirstSessionAfterUpgrade; @property(nonatomic, assign) BOOL isFirstSessionAfterUpgrade;
@property(nonatomic, assign) float deviceBatteryLevel; @property(nonatomic, assign) float deviceBatteryLevel;
@property(nonatomic, assign)
previous_session_info_constants::DeviceBatteryState deviceBatteryState;
@property(nonatomic, assign) BOOL OSRestartedAfterPreviousSession; @property(nonatomic, assign) BOOL OSRestartedAfterPreviousSession;
+ (void)resetSharedInstanceForTesting; + (void)resetSharedInstanceForTesting;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment