-
Notifications
You must be signed in to change notification settings - Fork 231
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
Universal ink cold launch fix #1421
Merged
Merged
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
-> Moved check isLinkingRelatedRequest to postRequest func -> Fixed isLinkingRelatedRequest to read identifiers from request post params. -> Temporarily Commented out code for retrieving archived queues - will revert later
…rl get successful.
…sion re-initialization irrespective of current SDK initializationStatus
Sources/BranchSDK/Branch.m
Outdated
@@ -1971,8 +1981,9 @@ - (void)initUserSessionAndCallCallback:(BOOL)callCallback sceneIdentifier:(NSStr | |||
|
|||
dispatch_async(self.isolationQueue, ^(){ | |||
|
|||
|
|||
// If the session is not yet initialized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment on this line that reset means we've got a new link we need to process
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or whatever reset
specifically indicates
gdeluna-branch
approved these changes
Aug 2, 2024
nsingh-branch
approved these changes
Aug 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fix for Universal Link Cold Launch
Summary
Issues -
-- If tracking is disabled and app launch is cold, Universal Links were not resolving intermittently because of race condition happening between
branch.initSession
and[BranchScene scene:continueUserActivity:]
for accessing preference variables. It happens only if both functions are called at the very same time.Following is the sequence of events which happens
DidFinishLaunching
usingbranch.initSession
. It has nouniversal_link
in request. First Open Request is sent to server.[BranchScene scene:continueUserActivity:]
is called and it hasuniversal_link
url . It sets this value in prefs. So when First open request is going out - it checks if its a linking request (by checking url in prefs, which is just set by second request), function returns - YES.universal_link_url
but its not anymore in prefs - its considered a non-linking request.Fix :
isLinkingRelatedRequest
function will read values of identifiers from post params of request instead of preferences.-- SDK keeps sending archived opens from the previous launch even after link is resolved
Fix : A property is added in
BranchOpenRequest
class to find out if the event is from the archived queue. Another propertyprocessArchivedOpens
is added inBNCServerRequestQueue
to signal if archived opens should be propecessed.After SDK gets
referringURL
inBranchOpenRequest
response, it turns of this flag.processNextQueueItem
function uses these properties to find out if the next request is from archive and it should not be processed. If YES, it removes it from the queue and processes next one.-- SDK resets session using enum variable
initializationStatus
. It sets its value toBNCInitStatusUninitialized
. Its a global variable and its value is sometimes changed by other parallel session initializations happening, if any, toBNCInitStatusInitializing
orBNCInitStatusInitialized
. So functioninitUserSessionAndCallCallback
, which checks this value for session initialization, simply returns wihtout resetting/initializing session.Fix : Added param in function
initUserSessionAndCallCallback
for forced session re-initialization irrespective of current SDK initializationStatusTesting Instructions
Use a test app with Universal Link Enabled and tracking disabled.
--> If Test App is running kill it. Cold Launch is required.
--> Copy universal links for testing in iMessage.
--> Click on Univesal Links in iMessage.
--> Test App should be launched.
--> Check logs to see if Open request sent with param "universal_link_url" is processed and other opens without this params are dropped.
Repeat this test multiple times, by killing app and re-launching app using links from iMessage.
I have used this app https://github.com/BranchMetrics/SpotifyTest
Links used for testing -
Repeat test by enabling tracking as well as delaying function calls
initSession
andBranchScene.shared().scene
(...)Type Of Change
cc @BranchMetrics/saas-sdk-devs for visibility.