Skip to content

Commit

Permalink
ios: Cleanup launched Crashpad flags and tests remove Breakpad from ios/
Browse files Browse the repository at this point in the history
There will be more changes to remove Breakpad outside of ios/ for iOS
clients.

Bug: 1412773, 1412774
Change-Id: I0363e74d8f92e62ab4007d4862f9610a204efbb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4103725
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Auto-Submit: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1101191}
  • Loading branch information
Justin Cohen authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent a812ee2 commit adcdd14
Show file tree
Hide file tree
Showing 50 changed files with 191 additions and 902 deletions.
8 changes: 2 additions & 6 deletions ios/build/chrome_build.gni
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,11 @@ if (target_environment == "catalyst") {
ios_enable_screen_time = false
}

# Configure whether breakpad support is enabled.
breakpad_enabled = is_official_build && is_chrome_branded

if (breakpad_enabled) {
breakpad_enabled_as_int = 1
# Configure is_official_release.
if (is_official_build && is_chrome_branded) {
is_official_release = enable_dsyms && target_environment == "device" &&
current_toolchain == default_toolchain
} else {
breakpad_enabled_as_int = 0
is_official_release = false
}

Expand Down
1 change: 0 additions & 1 deletion ios/chrome/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ if (!is_fat_secondary_toolchain) {
deps += ios_chrome_info_plist_addition_targets
}
args = [
"--breakpad=$breakpad_enabled_as_int",
"--branding=$chromium_short_name",
"--add-gtm-metadata=1",
]
Expand Down
2 changes: 0 additions & 2 deletions ios/chrome/app/application_delegate/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ source_set("unit_tests") {
"//ios/chrome/common/crash_report",
"//ios/chrome/common/intents",
"//ios/chrome/test:test_support",
"//ios/chrome/test/ocmock",
"//ios/chrome/test/providers/app_distribution",
"//ios/public/provider/chrome/browser/app_distribution:app_distribution_api",
"//ios/testing:block_swizzler",
Expand All @@ -91,7 +90,6 @@ source_set("unit_tests") {
"//ios/web/public/test/fakes",
"//net:test_support",
"//testing/gtest",
"//third_party/breakpad:client",
"//third_party/crashpad/crashpad/client:common",
"//third_party/ocmock",
"//ui/base",
Expand Down
4 changes: 0 additions & 4 deletions ios/chrome/app/application_delegate/app_state.mm
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,6 @@ - (void)applicationDidEnterBackground:(UIApplication*)application
[[PreviousSessionInfo sharedInstance] resetMemoryWarningFlag];
[[PreviousSessionInfo sharedInstance] stopRecordingMemoryFootprint];

// Turn off uploading of crash reports. This prevents failed uploads that will
// not be retried in Breakpad.
crash_helper::PauseBreakpadUploads();

GetApplicationContext()->OnAppEnterBackground();
}

Expand Down
70 changes: 0 additions & 70 deletions ios/chrome/app/application_delegate/app_state_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
#import "ios/testing/scoped_block_swizzler.h"
#import "ios/web/public/test/web_task_environment.h"
#import "ios/web/public/thread/web_task_traits.h"
#import "third_party/breakpad/breakpad/src/client/ios/BreakpadController.h"
#import "third_party/ocmock/OCMock/OCMock.h"
#import "third_party/ocmock/gtest_support.h"

Expand Down Expand Up @@ -275,19 +274,6 @@ void swizzleSafeModeShouldStart(BOOL shouldStart) {
safe_mode_swizzle_block_));
}

void swizzleBreakpadUploadingDisabled() {
breakpad_disabled_called_ = NO;

breakpad_swizzle_block_ = ^{
breakpad_disabled_called_ = YES;
};

breakpad_swizzler_.reset(
new ScopedBlockSwizzler([BreakpadController class],
NSSelectorFromString(@"setUploadingEnabled:"),
breakpad_swizzle_block_));
}

void swizzleHandleStartupParameters(
id<TabOpening> expectedTabOpener,
ChromeBrowserState* expectedBrowserState) {
Expand Down Expand Up @@ -445,8 +431,6 @@ ScopedBlockSwizzler swizzler(
}
ChromeBrowserState* getBrowserState() { return browser_state_.get(); }

BOOL breakpadUploadingHasBeenDisabled() { return breakpad_disabled_called_; }

private:
web::WebTaskEnvironment task_environment_;
TestAppState* app_state_;
Expand All @@ -465,12 +449,9 @@ ScopedBlockSwizzler swizzler(
ScenesBlock connected_scenes_swizzle_block_;
DecisionBlock safe_mode_swizzle_block_;
HandleStartupParam handle_startup_swizzle_block_;
ProceduralBlock breakpad_swizzle_block_;
std::unique_ptr<ScopedBlockSwizzler> safe_mode_swizzler_;
std::unique_ptr<ScopedBlockSwizzler> connected_scenes_swizzler_;
std::unique_ptr<ScopedBlockSwizzler> handle_startup_swizzler_;
std::unique_ptr<ScopedBlockSwizzler> breakpad_swizzler_;
__block BOOL breakpad_disabled_called_;
std::unique_ptr<TestChromeBrowserState> browser_state_;
};

Expand Down Expand Up @@ -797,57 +778,6 @@ ScopedBlockSwizzler swizzler(
EXPECT_OCMOCK_VERIFY(getBrowserLauncherMock());
}

// Tests that -applicationDidEnterBackground calls the metrics mediator.
TEST_F(AppStateTest, applicationDidEnterBackgroundIncognito) {
// This is a breakpad only test and can be deprecated once Breakpad is
// removed.
if (crash_helper::common::CanUseCrashpad())
return;
swizzleSafeModeShouldStart(NO);
[[getStartupInformationMock() stub] setIsFirstRun:YES];
[[[getStartupInformationMock() stub] andReturnValue:@YES] isFirstRun];

// Setup.
ScopedKeyWindow scopedKeyWindow;
id application = [OCMockObject niceMockForClass:[UIApplication class]];
id memoryHelper = [OCMockObject mockForClass:[MemoryWarningHelper class]];
StubBrowserInterfaceProvider* interfaceProvider = getInterfaceProvider();
crash_helper::SetEnabled(true);

std::unique_ptr<Browser> browser =
std::make_unique<TestBrowser>(getBrowserState());
id startupInformation = getStartupInformationMock();
id browserLauncher = getBrowserLauncherMock();

AppState* appState = getAppStateWithRealWindow(scopedKeyWindow.Get());
id appStateMock = OCMPartialMock(appState);
[[appStateMock expect] completeUIInitialization];

[[startupInformation expect] expireFirstUserActionRecorder];
[[[memoryHelper stub] andReturnValue:@0] foregroundMemoryWarningCount];
interfaceProvider.incognitoInterface.browser = browser.get();
[[[browserLauncher stub] andReturn:interfaceProvider] interfaceProvider];

swizzleBreakpadUploadingDisabled();

// Simulate launching the app before going to background. This is to start
// initialization process.
NSDictionary* launchOptions = @{};
id browserLauncherMock = getBrowserLauncherMock();
[[browserLauncherMock expect] setLaunchOptions:launchOptions];
[appState queueTransitionToFirstInitStage];
[appState queueTransitionToNextInitStage];

// Action.
[appState applicationDidEnterBackground:application
memoryHelper:memoryHelper];

// Tests.
EXPECT_OCMOCK_VERIFY(startupInformation);
EXPECT_TRUE(breakpadUploadingHasBeenDisabled());
crash_helper::SetEnabled(false);
}

// Tests that -applicationDidEnterBackground do nothing if the application has
// never been in a Foreground stage.
TEST_F(AppStateTest, applicationDidEnterBackgroundStageBackground) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
- (void)handleMemoryPressure;

// Resets the foregroundMemoryWarningCount property and the memoryWarningCount
// of the breakpad helper, setting their value to 0.
// crash key, setting their value to 0.
- (void)resetForegroundMemoryWarningCount;

@end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ - (void)handleMemoryPressure {
_outOfMemoryResetTime =
CFAbsoluteTimeGetCurrent() + kOutOfMemoryResetTimeInterval;

// Add information to breakpad.
// Add information to crash keys.
crash_keys::SetMemoryWarningCount(_foregroundMemoryWarningCount);
crash_keys::SetMemoryWarningInProgress(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#import "ios/chrome/browser/web_state_list/web_state_list.h"
#import "ios/chrome/browser/web_state_list/web_state_opener.h"
#import "ios/chrome/common/app_group/app_group_metrics.h"
#import "ios/chrome/test/ocmock/OCMockObject+BreakpadControllerTesting.h"
#import "ios/testing/scoped_block_swizzler.h"
#import "ios/web/public/test/web_task_environment.h"
#import "net/base/network_change_notifier.h"
Expand Down
2 changes: 1 addition & 1 deletion ios/chrome/app/chrome_overlay_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#import <MaterialComponents/MaterialOverlayWindow.h>

// Tracks size classes changes then reports to SizeClassRecorder and Breakpad.
// Tracks size classes changes then reports to SizeClassRecorder and Crash keys.
@interface ChromeOverlayWindow : MDCOverlayWindow
@end

Expand Down
14 changes: 7 additions & 7 deletions ios/chrome/app/chrome_overlay_window.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ @interface ChromeOverlayWindow ()
UserInterfaceStyleRecorder* userInterfaceStyleRecorder API_AVAILABLE(
ios(13.0));

// Updates the Breakpad report with the current size class.
- (void)updateBreakpad;
// Updates the crash keys with the current size class.
- (void)updateCrashKeys;

@end

Expand All @@ -29,7 +29,7 @@ - (instancetype)initWithFrame:(CGRect)frame {
self = [super initWithFrame:frame];
if (self) {
// When not created via a nib, create the recorders immediately.
[self updateBreakpad];
[self updateCrashKeys];
_userInterfaceStyleRecorder = [[UserInterfaceStyleRecorder alloc]
initWithUserInterfaceStyle:self.traitCollection.userInterfaceStyle];
}
Expand All @@ -38,10 +38,10 @@ - (instancetype)initWithFrame:(CGRect)frame {

- (void)awakeFromNib {
[super awakeFromNib];
[self updateBreakpad];
[self updateCrashKeys];
}

- (void)updateBreakpad {
- (void)updateCrashKeys {
crash_keys::SetCurrentHorizontalSizeClass(
self.traitCollection.horizontalSizeClass);
crash_keys::SetCurrentUserInterfaceStyle(
Expand All @@ -64,15 +64,15 @@ - (void)traitCollectionDidChange:(UITraitCollection*)previousTraitCollection {
[super traitCollectionDidChange:previousTraitCollection];
if (previousTraitCollection.horizontalSizeClass !=
self.traitCollection.horizontalSizeClass) {
[self updateBreakpad];
[self updateCrashKeys];
}
if ([self.traitCollection
hasDifferentColorAppearanceComparedToTraitCollection:
previousTraitCollection]) {
[self.userInterfaceStyleRecorder
userInterfaceStyleDidChange:self.traitCollection.userInterfaceStyle];
}
[self updateBreakpad];
[self updateCrashKeys];
}

@end
5 changes: 2 additions & 3 deletions ios/chrome/app/main_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,7 @@ - (void)cleanupDiscardedSessions;
- (void)sendQueuedFeedback;
// Called whenever an orientation change is received.
- (void)orientationDidChange:(NSNotification*)notification;
// Register to receive orientation change notification to update breakpad
// report.
// Register to receive orientation change notification to update crash keys.
- (void)registerForOrientationChangeNotifications;
// Asynchronously creates the pref observers.
- (void)schedulePrefObserverInitialization;
Expand Down Expand Up @@ -1158,7 +1157,7 @@ - (void)startLoggingBreadcrumbs {
DCHECK(persistentStorageManager);
persistentStorageManager->GetStoredEvents(
base::BindOnce(^(std::vector<std::string> events) {
breakpad::SetPreviousSessionEvents(events);
crash_report_helper::SetPreviousSessionEvents(events);
}));
}

Expand Down
2 changes: 1 addition & 1 deletion ios/chrome/app/memory_monitor.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
// Delay between each invocations of `UpdateMemoryValues`.
constexpr base::TimeDelta kMemoryMonitorDelay = base::Seconds(30);

// Checks the values of free RAM and free disk space and updates breakpad with
// Checks the values of free RAM and free disk space and updates crash keys with
// these values. Also updates available free disk space for PreviousSessionInfo.
void UpdateMemoryValues() {
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
Expand Down
4 changes: 2 additions & 2 deletions ios/chrome/app/startup/ios_chrome_main.mm
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
}
main_params.argv = argv;

// Chrome registers an AtExitManager in main in order to initialize breakpad
// early, so prevent a second registration by WebMainRunner.
// Chrome registers an AtExitManager in main in order to initialize the crash
// handler early, so prevent a second registration by WebMainRunner.
main_params.register_exit_manager = false;
web_main_runner_->Initialize(std::move(main_params));
}
Expand Down
3 changes: 0 additions & 3 deletions ios/chrome/browser/crash_report/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ source_set("crash_report") {
"//ios/chrome/common/crash_report",
"//ios/web",
"//third_party/abseil-cpp:absl",
"//third_party/breakpad:client",
]
}

Expand Down Expand Up @@ -117,12 +116,10 @@ source_set("unit_tests") {
"//ios/chrome/browser/web_state_list:test_support",
"//ios/chrome/browser/web_state_list:web_state_list",
"//ios/chrome/common/crash_report",
"//ios/chrome/test/ocmock",
"//ios/testing:block_swizzler",
"//ios/web/public/test",
"//testing/gmock",
"//testing/gtest",
"//third_party/breakpad:client",
"//third_party/crashpad/crashpad/client:common",
"//third_party/ocmock",
]
Expand Down
12 changes: 1 addition & 11 deletions ios/chrome/browser/crash_report/crash_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@

namespace crash_helper {

// Sync the kCrashpadIOS feature to kCrashpadStartOnNextRun NSUserDefault.
void SyncCrashpadEnabledOnNextRun();

// Starts the crash handlers. This must be run as soon as possible to catch
// early crashes.
void Start();
Expand All @@ -23,13 +20,9 @@ void SetEnabled(bool enabled);
// Process and begin uploading pending crash reports if application is active.
// If application state is inactive or backgrounded, this is a no-op. Can be
// called multiple times, but will only take effect the first time (when app
// state is active) for Crashpad. For Breakpad, this can be called to start
// uploads and restart uploads after -PauseBreakpadUploads() is called.
// state is active).
void UploadCrashReports();

// For breakpad, it is necessary to pause uploads when entering the background.
void PauseBreakpadUploads();

// Process any pending crashpad reports, and mark them as
// 'uploaded_in_recovery_mode'.
void ProcessIntermediateReportsForSafeMode();
Expand All @@ -54,9 +47,6 @@ void WillStartCrashRestoration();
// in recovery mode.
void StartUploadingReportsInRecoveryMode();

// Resets the Breakpad configuration from the main bundle.
void RestoreDefaultConfiguration();

// Deletes any reports that were recorded or uploaded within the time range.
void ClearReportsBetween(base::Time delete_begin, base::Time delete_end);

Expand Down

0 comments on commit adcdd14

Please sign in to comment.