Improve BackgroundObserver

[swiftlyfalling]

Improvements and New Features

Following best-practice recommendations, BackgroundObserver now registers a background task for every attached Procedure (instead of waiting until background events occur and trying to register background tasks then) and ends the registered background task once the Procedure has finished or if background execution time expires.

BackgroundObserver now supports an optional CancellationBehavior for automatically cancelling the attached Procedure when certain background-related events occur. Supported values are:

• .never: do not cancel the attached Procedure (the default)
• .whenBackgroundExecutionTimeExpires: cancel the attached Procedure when the associated background task's expiration handler is called
• .whenAppIsBackgrounded: cancel the attached Procedure when the app is backgrounded (or if it is already in the background)

An internal background manager now observes NotificationCenter background events once, instead of the previous observation inside every BackgroundObserver instance, reducing overhead and improving performance.

Bug Fixes

• Adding a single BackgroundObserver instance to multiple Procedures now works.
• BackgroundObserver properly handles when running in the background is not possible (when attempts to begin background tasks fail).
• BackgroundObserver no longer attempts to access main-thread-only properties of UIApplication on other threads. (Must be called from main thread only #776)

And exhaustive testing improvements, including fixes to the TestableUIApplication and many additional test cases:

• Previously, TestableUIApplication always returned UIBackgroundTaskInvalid for beginBackgroundTask, because it relied on calling UIApplication.shared and background execution is not supported in the test runner (so attempting to actually start background tasks fails). Now, the TestableUIApplication handles mocking up the required APIs and events itself.

[danthorpe]

@swiftlyfalling shall we hold v4.3 for this?

[swiftlyfalling]

@danthorpe: Let's target this for v4.4. (I'd like to run tests on some additional edge cases on real devices, and it'll be a little bit.)

[swiftlyfalling]

I'm debating about the .whenBackgroundExecutionTimeExpires behavior.

As-is, .whenBackgroundExecutionTimeExpires causes the BackgroundObserver to cancel the attached Procedure when the associated background task's expiration handler is called. However, because handling cancellation is asynchronous, there's no guarantee that the Procedure's cancellation handlers will be called prior to the app's background execution time expiring.

Hence, I think the actual effect of .whenBackgroundExecutionTimeExpires is potentially confusing, relies on other factors (when the system actually pauses the app), and it should probably be removed before merging this PR.

Thoughts?

[jshier]

While this would be really nice functionality, I think I have to agree. I don't know that it would be dangerous, but it would certainly be misleading.

On a related note, in the .never behavior, if the observer survives one background suspension, it has cancelled its task, no? If the procedure was still running when the app was suspended again, it would no longer have an associated task, right? That may be problematic, though perhaps I'm not clear on the use case in that scenario.

[swiftlyfalling]

@jshier: Re: .never, I'm also still thinking about that behavior.

The current code registers a background task when the application enters the background - so it ought to register a new task if the app lifecycle is:
Running -> Backgrounded (Registers task 1) -> Running -> Backgrounded (Registers task 2)

This PR registers a background task immediately (as Apple recommends), but doesn't do so a second time (after one cycle of being backgrounded -> active).

The thinking was that the default would actually change from .never (which more closely approximates the current behavior) to .whenAppIsBackgrounded in ProcedureKit v5. The current default behavior (essentially, .never) just steals as much background time as possible every time the app is backgrounded and Procedures with BackgroundObservers are running - this doesn't quite fit best recommendations for iOS.

However, I think modifying this PR to request a new background task if the prior one expired (and the app becomes active again) makes sense. It would more closely match the prior behavior. (Even if it only comes into play when .never is used.)

[danthorpe]

Okay, so following up on this, we've got some outstanding changes to address, I think.

1. Remove .whenBackgroundExecutionTimeExpires cancellation behaviour.
2. Handle an edge case where the observer registers additional Background Tasks when the app comes into the foreground when the attached Procedure is still executing. - Created BackgroundObserver should re-register a background task when app is foregrounded and the attached Procedure is not finished. #820 for this case.

It seems like the 1st point should be done before the PR is merged, but perhaps the 2nd point can come later, as it's "just" additionally handling an edge case.

I'm also looking at the CodeBeat static analysis of this change set - as it introduces quite a few issues. Generally, deeper type nesting (maybe the registry should not be a nested class?) and increased Assignment Branch Condition - which could be minimised with some functions.

@swiftlyfalling I can make these changes, and get this merged.