Skip to content

EXT-229 Defer ATT request until app becomes active - #93

Merged
JakubMrozek merged 2 commits into
mainfrom
duc/EXT-229-add-delay-for-att-prompt
Mar 31, 2026
Merged

EXT-229 Defer ATT request until app becomes active#93
JakubMrozek merged 2 commits into
mainfrom
duc/EXT-229-add-delay-for-att-prompt

Conversation

@phiviet01

Copy link
Copy Markdown
Contributor

No description provided.

@phiviet01
phiviet01 force-pushed the duc/EXT-229-add-delay-for-att-prompt branch from 21f0c92 to db9fa3c Compare March 9, 2026 08:57
@phiviet01 phiviet01 changed the title Defer ATT request until app becomes active EXT-229 Defer ATT request until app becomes active Mar 9, 2026
@phiviet01
phiviet01 marked this pull request as ready for review March 16, 2026 10:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Defers the iOS App Tracking Transparency (ATT) authorization request until the application is active, avoiding calling ATTrackingManager.requestTrackingAuthorization during inactive/background states.

Changes:

  • Routes requestTrackingAuthorization through a new requestTrackingAuthorizationWhenActive helper on iOS 14+.
  • Adds an applicationState check and defers the request via a didBecomeActive observer when the app isn’t active.
  • Ensures the observer is removed before completing with a final status.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +51 to +65
if UIApplication.shared.applicationState != .active {
addObserver(result: result)
return
}

removeObserver()
ATTrackingManager.requestTrackingAuthorization { [weak self] status in
if status == .denied && ATTrackingManager.trackingAuthorizationStatus == .notDetermined {
self?.addObserver(result: result)
return
}

self?.removeObserver()
result(Int(status.rawValue))
}
Comment on lines +51 to +53
if UIApplication.shared.applicationState != .active {
addObserver(result: result)
return
@JakubMrozek

Copy link
Copy Markdown
Member

@claude

@claude

claude Bot commented Mar 16, 2026

Copy link
Copy Markdown

Claude finished @JakubMrozek's task in 1m 15s —— View job


Code Review

The approach is sound — deferring ATT until the app is active is the correct fix. The logic flows well, but there are a few issues to address before merging:


1. Thread safety (echoing Copilot's comment)

UIApplication.shared.applicationState and ATTrackingManager.requestTrackingAuthorization must run on the main thread. Flutter method channel calls arrive on the main thread by default, but that's not guaranteed. Explicitly dispatching removes any ambiguity:

@available(iOS 14, *)
private func requestTrackingAuthorizationWhenActive(result: @escaping FlutterResult) {
    DispatchQueue.main.async {
        if UIApplication.shared.applicationState != .active {
            self.addObserver(result: result)
            return
        }
        // ... rest of the logic
    }
}

Fix this →


2. Pending result is silently dropped on concurrent calls

When a second requestTrackingAuthorization call arrives while the first is still deferred (app not yet active), addObserver calls removeObserver() first — which discards the first observer without ever calling its result closure. The Flutter Completer on the Dart side will hang indefinitely.

Two reasonable options:

  • Return an error if a request is already pending:
    private func addObserver(result: @escaping FlutterResult) {
        if observer != nil {
            result(FlutterError(code: "ALREADY_PENDING", message: "ATT request already pending", details: nil))
            return
        }
        // ...
    }
  • Queue the second result and fulfill both when the app becomes active (more complex but user-friendly).

Fix this →


3. Active-path removeObserver() also drops a pending deferred result

At requestTrackingAuthorizationWhenActive line ~56, if the app is now active and a previous call registered an observer (deferred path), removeObserver() is called before proceeding with the new request. This has the same silent-drop problem as #2.

The fix for #2 handles this implicitly if you guard against concurrent requests, but worth noting the code path.


4. Minor: redundant removeObserver() inside ATT callback

At line 63, self?.removeObserver() is called before result(...). In the active-app code path, no observer was added (it was removed at line 56), so this call is a no-op. It's harmless but slightly misleading — a comment clarifying intent would help, or restructure so it's only called when needed.


Summary

Severity
Thread safety (#1) Medium — safe in practice today, but fragile
Silent result drop on concurrent calls (#2, #3) High — causes Flutter-side hangs
Redundant removeObserver in callback (#4) Low — cosmetic

Overall the deferral logic is correct. The concurrent-call handling is the main issue to fix before shipping.

@phiviet01

Copy link
Copy Markdown
Contributor Author

Claude finished @JakubMrozek's task in 1m 15s —— View job

Code Review

The approach is sound — deferring ATT until the app is active is the correct fix. The logic flows well, but there are a few issues to address before merging:

1. Thread safety (echoing Copilot's comment)

UIApplication.shared.applicationState and ATTrackingManager.requestTrackingAuthorization must run on the main thread. Flutter method channel calls arrive on the main thread by default, but that's not guaranteed. Explicitly dispatching removes any ambiguity:

@available(iOS 14, *)
private func requestTrackingAuthorizationWhenActive(result: @escaping FlutterResult) {
    DispatchQueue.main.async {
        if UIApplication.shared.applicationState != .active {
            self.addObserver(result: result)
            return
        }
        // ... rest of the logic
    }
}

Fix this →

2. Pending result is silently dropped on concurrent calls

When a second requestTrackingAuthorization call arrives while the first is still deferred (app not yet active), addObserver calls removeObserver() first — which discards the first observer without ever calling its result closure. The Flutter Completer on the Dart side will hang indefinitely.

Two reasonable options:

* **Return an error** if a request is already pending:
  ```swift
  private func addObserver(result: @escaping FlutterResult) {
      if observer != nil {
          result(FlutterError(code: "ALREADY_PENDING", message: "ATT request already pending", details: nil))
          return
      }
      // ...
  }
  ```

* **Queue the second result** and fulfill both when the app becomes active (more complex but user-friendly).

Fix this →

3. Active-path removeObserver() also drops a pending deferred result

At requestTrackingAuthorizationWhenActive line ~56, if the app is now active and a previous call registered an observer (deferred path), removeObserver() is called before proceeding with the new request. This has the same silent-drop problem as #2.

The fix for #2 handles this implicitly if you guard against concurrent requests, but worth noting the code path.

4. Minor: redundant removeObserver() inside ATT callback

At line 63, self?.removeObserver() is called before result(...). In the active-app code path, no observer was added (it was removed at line 56), so this call is a no-op. It's harmless but slightly misleading — a comment clarifying intent would help, or restructure so it's only called when needed.

Summary

Severity
Thread safety (#1) Medium — safe in practice today, but fragile
Silent result drop on concurrent calls (#2, #3) High — causes Flutter-side hangs
Redundant removeObserver in callback (#4) Low — cosmetic

Overall the deferral logic is correct. The concurrent-call handling is the main issue to fix before shipping.

#1 - I would not treat this as a required fix. The plugin uses the default FlutterMethodChannel registration, and Flutter’s docs say handlers run on the platform main thread unless you opt into a background task queue. Sources: Flutter platform channels and Flutter iOS embedder docs. Wrapping this in DispatchQueue.main.async is defensive, not necessary for current behavior.
#2 and #3 - in this repo that code path is not normally reachable because ATT requests are already guarded to run only once
#4 - is cosmetic

@JakubMrozek
JakubMrozek merged commit ad02cb8 into main Mar 31, 2026
2 checks passed
@phiviet01
phiviet01 deleted the duc/EXT-229-add-delay-for-att-prompt branch April 1, 2026 11:52
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.

3 participants