This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
[iOS] Generate and send a turnstile event when map view event is queued #3713
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boundsj
force-pushed
the
boundsj-send-turnstyle-on-map-load
branch
from
3f44d36 to
2d408d9
Compare
| - (void) pushTurnstileEvent { | ||
|
|
||
| __weak MGLMapboxEvents *weakSelf = self; | ||
| // Turnstile events only need to be sent up once per application launch | ||
| if (turnstileEventSent) { return; } |
There was a problem hiding this comment.
We should make a call on if () return style in MGLMapboxEvents and harmonize the entire thing.
|
🚧:+1: |
|
👍 |
boundsj
force-pushed
the
boundsj-send-turnstyle-on-map-load
branch
2 times, most recently
from
99a0766 to
07755b6
Compare
This change adds logic to push a turnstile event whenever a map view event is queued. It guards against sending the turnstile event more than once per session. Note that `pushTurnstileEvent` is actually a "force push" in that it flushes the queue and, since it enqueues a turnstile event, the flush's call to `postEvents` will attempt to send any queued events even if telemetry is paused. This has the side effect of defeating the other queue / timing / flush logic in place whenever map load (and now implicitly turnstile) events are pushed.
boundsj
force-pushed
the
boundsj-send-turnstyle-on-map-load
branch
from
07755b6 to
9891d0e
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds logic to push a turnstile event whenever a map view
event is queued. It guards against sending the turnstile event more
than once per session.
Note that
pushTurnstileEventis actually a "force push" in that itflushes the queue and, since it enqueues a turnstile event, the
flush's call to
postEventswill attempt to send any queued eventseven if telemetry is paused. This has the side effect of defeating
the other queue / timing / flush logic in place whenever map load
(and now implicitly turnstile) events are pushed.
This change does take care to continue to ensure that, in
pushEventeven if theevents instance is paused, the turnstile event is still pushed.
This change drops the use of the priority background queue that used
to be used to dispatch async
pushTurnstileEvent. I could not see how thepriority queue was required for that. The subsequently called implementations of
pushTurnstileEventandflushstill use the custom serial queueand the
postEventsmethod dispatches using a priority backgroundqueue.
cc @1ec5 @friedbunny