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

⬆️ UIBackgroundFetchResult ordinal mapping #20

Closed
Kymer opened this issue Aug 8, 2019 · 3 comments
Closed

⬆️ UIBackgroundFetchResult ordinal mapping #20

Kymer opened this issue Aug 8, 2019 · 3 comments
Labels
enhancement New feature or request iOS iOS specific

Comments

@Kymer
Copy link

Kymer commented Aug 8, 2019

Currently in the setMethodCallHandler block of backgroundMethodChannel a UIBackgroundFetchResult is created using the flutter result Int like so:

UIBackgroundFetchResult(rawValue: UInt(fetchResult))

We shouldn't rely on the internal ordering of UIBackgroundFetchResult cases, as they could change with future iOS releases.

@timrijckaert
Copy link
Contributor

Actually this was an idea we already discussed with @jeremie-movify and @petermnt before.
The return of a boolean was a MVP approach.

Ideally we would want to return some sort of enum representing the success state of a background task.

Android already expects either a:

  • Result.failure()
    • Used to indicate that the work completed with a permanent failure. Any work that depends
      on this will also be marked as failed and will not be run. If you need child workers
      to run, you need to return Result.Success() failure indicates a permanent
      stoppage of the chain of work.

  • Result.success()
    • Used to indicate that the work completed successfully. Any work that depends on this
      can be executed as long as all of its other dependencies and constraints are met.

  • Result.Retry()
    • Used to indicate that the work encountered a transient failure and should be retried with
      backoff.

As of now (0.0.12) the plugin will map the returning bool as following:

  • false -> Result.retry()
  • true -> Result.success()

I thought the iOS return code was similarly mapped.

We could then have something like:

void callbackDispatcher() {
  var result = Result.retry
  Workmanager.executeTask((task) async {
    switch (task) {
      case simpleTaskKey:
       print("do some background work");
       result = Result.success
        break;
    }

    return Future.value(result);
  });
}

@petermnt
Copy link
Contributor

petermnt commented Aug 8, 2019

This issue is actually about something else.
The iOS code maps an int (0, 1, 2) to an enum. But it uses the 'index' of the enum for getting the right enum.
At the moment they are in a certain order, but if the order ever changes, the int -> enum mapping would break.
A proper switch case would fix this.

@timrijckaert timrijckaert removed the iOS iOS specific label Aug 8, 2019
@timrijckaert
Copy link
Contributor

Ok I misunderstood I'll open another ticket for the idea from above 👍

@timrijckaert timrijckaert added the iOS iOS specific label Aug 8, 2019
@timrijckaert timrijckaert changed the title iOS: Don't tightly couple UIBackgroundFetchResult with flutter result ⬆️ UIBackgroundFetchResult ordinal mapping Aug 8, 2019
Kymer pushed a commit that referenced this issue Aug 9, 2019
- Now we don't rely on the internal ordering of `UIBackgroundFetchResult`'s cases: fixes #20
- Because `flutterResult` is a bool (for now, see #23), it now calls the completionhandler with a *correct* `UIBackgroundFetchResult`: Fixes #21
Kymer pushed a commit that referenced this issue Aug 12, 2019
* Introduced simple wrapper around os_log for readability

Ideally this should live in a separate little utility framework which this plugin could depend on, but good enough for now.

* Cast flutterResult as bool and return correct UIBackgroundFetchResult

- Now we don't rely on the internal ordering of `UIBackgroundFetchResult`'s cases: fixes #20
- Because `flutterResult` is a bool (for now, see #23), it now calls the completionhandler with a *correct* `UIBackgroundFetchResult`: Fixes #21

* Added some more info logging

* Cleanup `flutterEngine` after work has been done

Fixes #19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request iOS iOS specific
Projects
None yet
Development

No branches or pull requests

3 participants