From 04c328562507e034daed237ec8c2b1b90c9727f3 Mon Sep 17 00:00:00 2001 From: Robin Macharg Date: Fri, 20 Mar 2020 18:31:30 +0000 Subject: [PATCH] orientation change breadcrumb PR feedback --- Source/BugsnagClient.h | 7 ++ Source/BugsnagClient.m | 149 +++++++++++++++++++++++++---------------- Tests/BugsnagTests.m | 25 +++++++ 3 files changed, 122 insertions(+), 59 deletions(-) diff --git a/Source/BugsnagClient.h b/Source/BugsnagClient.h index f76abdfe6..e576e7454 100644 --- a/Source/BugsnagClient.h +++ b/Source/BugsnagClient.h @@ -28,6 +28,13 @@ #import "BugsnagConfiguration.h" #import "BugsnagMetadata.h" +#if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE +#import +#elif TARGET_OS_MAC +#import +#endif + +NSString * _Nullable BSGOrientationNameFromEnum(UIDeviceOrientation deviceOrientation); @class BugsnagSessionTracker; diff --git a/Source/BugsnagClient.m b/Source/BugsnagClient.m index f156fe6f2..b373838a6 100644 --- a/Source/BugsnagClient.m +++ b/Source/BugsnagClient.m @@ -42,12 +42,6 @@ #import "BSG_KSSystemInfo.h" #import "BSG_KSMach.h" -#if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE -#import -#elif TARGET_OS_MAC -#import -#endif - NSString *const NOTIFIER_VERSION = @"5.23.0"; NSString *const NOTIFIER_URL = @"https://github.com/bugsnag/bugsnag-cocoa"; NSString *const BSTabCrash = @"crash"; @@ -84,10 +78,6 @@ static NSUInteger handledCount; static NSUInteger unhandledCount; static bool hasRecordedSessions; -#if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE -// The previous device orientation - iOS only -static NSString *lastOrientation = NULL; -#endif /** * Handler executed when the application crashes. Writes information about the @@ -147,6 +137,40 @@ void BSSerializeDataCrashHandler(const BSG_KSCrashReportWriter *writer, int type } } +/** + * Convert a device orientation into its Bugsnag string representation + * + * @param deviceOrientation The platform device orientation + * + * @returns A string representing the device orientation or nil if there's no equivalent + */ +NSString *BSGOrientationNameFromEnum(UIDeviceOrientation deviceOrientation) +{ + NSString *orientation; + switch (deviceOrientation) { + case UIDeviceOrientationPortraitUpsideDown: + orientation = @"portraitupsidedown"; + break; + case UIDeviceOrientationPortrait: + orientation = @"portrait"; + break; + case UIDeviceOrientationLandscapeRight: + orientation = @"landscaperight"; + break; + case UIDeviceOrientationLandscapeLeft: + orientation = @"landscapeleft"; + break; + case UIDeviceOrientationFaceUp: + orientation = @"faceup"; + break; + case UIDeviceOrientationFaceDown: + orientation = @"facedown"; + break; + default: + return nil; // always ignore unknown breadcrumbs + } + return orientation; +} /** * Writes a dictionary to a destination using the BSG_KSCrash JSON encoding * @@ -213,6 +237,10 @@ @interface BugsnagClient () @property (nonatomic, strong) BSGOutOfMemoryWatchdog *oomWatchdog; @property (nonatomic, strong) BugsnagPluginClient *pluginClient; @property (nonatomic) BOOL appCrashedLastLaunch; +#if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE +// The previous device orientation - iOS only +@property (class, nonatomic, strong) NSString *lastOrientation; +#endif @end @interface BugsnagConfiguration () @@ -221,6 +249,23 @@ @interface BugsnagConfiguration () @implementation BugsnagClient +#if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE +/** + * Storage for the device orientation. It is "last" whenever an orientation change is received + */ +static NSString *_lastOrientation = nil; + ++ (NSString *)lastOrientation +{ + return _lastOrientation; +} + ++ (void)setLastOrientation:(NSString *)lastOrientation +{ + _lastOrientation = lastOrientation; +} +#endif + @synthesize configuration; - (id)initWithConfiguration:(BugsnagConfiguration *)initConfiguration { @@ -280,6 +325,10 @@ - (id)initWithConfiguration:(BugsnagConfiguration *)initConfiguration { [self metadataChanged:self.configuration.config]; [self metadataChanged:self.state]; self.pluginClient = [[BugsnagPluginClient alloc] initWithPlugins:self.configuration.plugins]; + +#if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE + _lastOrientation = BSGOrientationNameFromEnum([UIDevice currentDevice].orientation); +#endif } return self; } @@ -390,7 +439,6 @@ - (void)start { [[UIDevice currentDevice] beginGeneratingDeviceOrientationNotifications]; [self batteryChanged:nil]; - [self orientationChanged:nil]; [self addTerminationObserver:UIApplicationWillTerminateNotification]; #elif TARGET_OS_MAC @@ -778,63 +826,46 @@ - (void)batteryChanged:(NSNotification *)notif { toTabWithName:BSGKeyDeviceState]; } -- (void)orientationChanged:(NSNotification *)notif { - NSString *orientation; - +/** + * Called when an orientation change notification is received to record an + * equivalent breadcrumb. + * + * @param notification The orientation-change notification + */ +- (void)orientationChanged:(NSNotification *)notification { UIDeviceOrientation currentDeviceOrientation = [UIDevice currentDevice].orientation; - - switch (currentDeviceOrientation) { - case UIDeviceOrientationPortraitUpsideDown: - orientation = @"portraitupsidedown"; - break; - case UIDeviceOrientationPortrait: - orientation = @"portrait"; - break; - case UIDeviceOrientationLandscapeRight: - orientation = @"landscaperight"; - break; - case UIDeviceOrientationLandscapeLeft: - orientation = @"landscapeleft"; - break; - case UIDeviceOrientationFaceUp: - orientation = @"faceup"; - break; - case UIDeviceOrientationFaceDown: - orientation = @"facedown"; - break; - default: - return; // always ignore unknown breadcrumbs + NSString *orientation = BSGOrientationNameFromEnum(currentDeviceOrientation); + + // Short-circuit the exit if we don't have enough info to record a full breadcrumb + // or the orientation hasn't changed (false positive). + if (!orientation) { + return; + } + else if (!_lastOrientation || [orientation isEqualToString:_lastOrientation]) { + _lastOrientation = orientation; + return; } - NSDictionary *lastBreadcrumb = - [[self.configuration.breadcrumbs arrayValue] lastObject]; - NSString *orientationNotifName = - BSGBreadcrumbNameForNotificationName(notif.name); + NSDictionary *lastBreadcrumb = [[self.configuration.breadcrumbs arrayValue] lastObject]; + NSString *orientationNotifName = BSGBreadcrumbNameForNotificationName(notification.name); - if (lastBreadcrumb && - [orientationNotifName isEqualToString:lastBreadcrumb[BSGKeyName]]) { + if (lastBreadcrumb && [orientationNotifName isEqualToString:lastBreadcrumb[BSGKeyName]]) + { NSDictionary *metadata = lastBreadcrumb[BSGKeyMetadata]; - if ([orientation isEqualToString:metadata[BSGKeyOrientation]]) + if ([orientation isEqualToString:metadata[BSGKeyOrientation]]) { return; // ignore duplicate orientation event + } } - - // It's not a change - if ([orientation isEqualToString:lastOrientation]) - return; - - // We previously had an orientation - if (lastOrientation) { - [[self state] addAttribute:BSGKeyOrientationChange - withValue:@{@"from" : lastOrientation, - @"to" : orientation} - toTabWithName:BSGKeyDeviceState]; - } - // We shouldn't get here without orientation being set, but to be on the safe side: - if (orientation) - // Preserve the orientation - lastOrientation = orientation; + // Record the breadcrumb + [[self state] addAttribute:BSGKeyOrientationChange + withValue:@{@"from" : _lastOrientation, + @"to" : orientation} + toTabWithName:BSGKeyDeviceState]; + + // Preserve the orientation + _lastOrientation = orientation; } - (void)lowMemoryWarning:(NSNotification *)notif { diff --git a/Tests/BugsnagTests.m b/Tests/BugsnagTests.m index b5f79b009..46d67af2b 100644 --- a/Tests/BugsnagTests.m +++ b/Tests/BugsnagTests.m @@ -8,6 +8,7 @@ // Unit tests of global Bugsnag behaviour #import "Bugsnag.h" +#import "BugsnagClient.h" #import "BugsnagTestConstants.h" #import @@ -21,6 +22,11 @@ @interface BugsnagConfiguration () @property(nonatomic, readwrite, strong) NSMutableArray *onSendBlocks; @end +@interface BugsnagClient () ++ (NSString *)lastOrientation; ++ (void)setLastOrientation:(NSString *)lastOrientation; +@end + @interface BugsnagTests : XCTestCase @end @@ -406,4 +412,23 @@ - (void) testOnSendBlocks { [self waitForExpectations:@[expectation5, expectation6] timeout:1.0]; } +/** + * Test that the Orientation -> string mapping is as expected + * NOTE: should be moved to BugsnagClientTests when that file exists + */ +- (void)testBSGOrientationNameFromEnum { + XCTAssertEqualObjects(BSGOrientationNameFromEnum(UIDeviceOrientationPortraitUpsideDown), @"portraitupsidedown"); + XCTAssertEqualObjects(BSGOrientationNameFromEnum(UIDeviceOrientationPortrait), @"portrait"); + XCTAssertEqualObjects(BSGOrientationNameFromEnum(UIDeviceOrientationLandscapeRight), @"landscaperight"); + XCTAssertEqualObjects(BSGOrientationNameFromEnum(UIDeviceOrientationLandscapeLeft), @"landscapeleft"); + XCTAssertEqualObjects(BSGOrientationNameFromEnum(UIDeviceOrientationFaceUp), @"faceup"); + XCTAssertEqualObjects(BSGOrientationNameFromEnum(UIDeviceOrientationFaceDown), @"facedown"); + + XCTAssertNil(BSGOrientationNameFromEnum(-1)); + XCTAssertNil(BSGOrientationNameFromEnum(99)); + + [BugsnagClient setLastOrientation:@"testOrientation"]; + XCTAssertEqualObjects([BugsnagClient lastOrientation], @"testOrientation"); +} + @end