Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: application state check on app extensions #303

Merged
merged 8 commits into from
Aug 4, 2020

Conversation

aboedo
Copy link
Member

@aboedo aboedo commented Jul 31, 2020

UIApplication.sharedApplication isn't available in iOS app extensions. They won't compile if the method is used.

Unfortunately, there's no pre-processor macro to check if the code is running within an app extension. There's also no direct API.
Most of the solutions I found online seem to involve adding a custom flag to the app extension target's build settings... which is fine, if you're the app developer. If you're developing an SDK, however, this isn't an option.

I realized that MParticle used to ask developers to create this custom flag, but afterwards removed that and instead they now check if the app's bundle has the filename extension .appex.
This only works at runtime, so I had to refactor the related code a bit: I use KVC to call the method by name, if and only if the code is not running inside an app extension.

That should solve the app extension issue.

On another note, since the code is calling UIApplication but may be running in a background thread, this might raise warnings from the Main Thread Checker (see #300).
To address this, I moved the call to UIApplication.sharedApplication to the main thread. However, the rest of the method (fetching purchaser info and offerings from the backend) runs in a background operation thread.

@aboedo aboedo self-assigned this Jul 31, 2020
@aboedo aboedo requested a review from vegaro July 31, 2020 21:57
@@ -20,7 +20,7 @@ NS_ASSUME_NONNULL_BEGIN
@property(nonatomic, copy, readonly) NSString *platformFlavorVersion;


- (BOOL)isApplicationBackgrounded;
- (void)isApplicationBackgroundedWithCompletion:(void(^)(BOOL))completion; // calls completion on the main thread
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there might be a better way to document that this returns on the main thread?

Comment on lines +96 to +101
#elif TARGET_OS_TV
return UIApplication.sharedApplication.applicationState == UIApplicationStateBackground;
#elif TARGET_OS_OSX
return NO;
#elif TARGET_OS_WATCH
return WKExtension.sharedExtension.applicationState == WKApplicationStateBackground;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other targets should be safe I believe

}

- (BOOL)isAppExtension {
return [NSBundle.mainBundle.bundlePath hasSuffix:@".appex"];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was taken from MParticle's solution

Purchases/Misc/RCSystemInfo.m Show resolved Hide resolved
Comment on lines +299 to +308
[self.systemInfo isApplicationBackgroundedWithCompletion:^(BOOL isBackgrounded) {
if (!isBackgrounded) {
dispatch_queue_t backgroundQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
dispatch_async(backgroundQueue, ^{
[self updateAllCachesWithCompletionBlock:callDelegate];
});
} else {
[self sendCachedPurchaserInfoIfAvailable];
}
}];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the reason for a lot of the changes in the tests - since we're calling the updateAllCaches method from a background queue, it will take longer to return for tests.

Comment on lines +209 to +213
if let responseNSError = responseError as? NSError {
successFailed = (status >= 500
&& data == nil
&& error.domain == responseNSError.domain
&& error.code == responseNSError.code)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this had been bugging me for a while - apparently on iOS 14, error != responseError as NSError?.
So this would fail. I'm just manually checking the domain and code, which should suffice for the purposes of the test.

Comment on lines +15 to +16
requestFetcher = MockRequestFetcher()
systemInfo = MockSystemInfo(platformFlavor: nil, platformFlavorVersion: nil, finishTransactions: true)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be doing this for all mocks - otherwise they just get created once and keep state in between tests.
I'm just doing it here for these two since I had issues, but I'm not 100% it's necessary. I'll check before merging.

Copy link
Contributor

@vegaro vegaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! just a minor comment. I can approve once it's not a draft

Purchases/Misc/RCSystemInfo.m Show resolved Hide resolved
@aboedo aboedo marked this pull request as ready for review August 3, 2020 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants