[firebase_messaging] changing getToken to rely on platform's getToken (2) #869
Conversation
|
I think the error in the checks is flutter/flutter#23095 |
ffeu
changed the title
|
Hi @Hixie |
| @@ -74,6 +74,8 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result | |||
| NSString *topic = call.arguments; | |||
| [[FIRMessaging messaging] unsubscribeFromTopic:topic]; | |||
| result(nil); | |||
| } else if ([@"getToken" isEqualToString:method]) { | |||
| result([[FIRMessaging messaging] FCMToken]); | |||
There was a problem hiding this comment.
Could you use the callback to retrieve the token from InstanceID in the same way it is done here:
https://github.com/firebase/quickstart-ios/blob/master/messaging/MessagingExample/ViewController.m#L39
There was a problem hiding this comment.
No problem, but let me make a question first.
On Android, that was the only way I managed to make it work - I couldn't find a way to get it without a callback.
On iOS, in the example you sent, it seems the local token is accessed using the same way I'm currently using. And then it does what seems to be getting the remote token from Instance ID.
Wouldn't they have to be same ? Why would they differ?
I ask because I've made other changes (which I'm holding waiting for this PR) to allow token unregistration (deleteInstanceID, setAutoInitEnabled, etc), and if getting a remote token requests for a new one, it would interfere in the way I was thinking it works.
There was a problem hiding this comment.
The token is automatically generated by the native SDKs however if you directly call getToken without the callback there is the possibility that it could be null. Using the callback would mean that getToken from the dart side either returns the cached token or returns the token when it becomes available but never null.
There was a problem hiding this comment.
HHmm, in this PR I'm removing the dart cached token value - I would rather use the Android and iOS cached values. If we managed to do that, I think getting a null would be understandable (if not previously configured), specially considering that it would be way faster.
However, if we can't do that on both (in Android it seems that currently there isn't a cached value - deprecated), then I agree it's better to have both obtaining it 'remotely'.
I'll make the change, but it will take some time to test the issues again!
There was a problem hiding this comment.
The Instance ID does not always go remote for the token. If the token is available then the callback returns immediately so should be just as fast as if you called it directly. Also it would be good to have the same behaviour on both Android and iOS. Thanks a lot for looking into this.
There was a problem hiding this comment.
Hi @kroikie,
Please take a look in the iOS an Android implementations. It's WIP as I didn't properly test them.
I changed the Android implementation from OnSuccessListener to OnCompleteListener to uniform implementations, logging the error if found.
Let me know if that's ok (then I'll test it) :)
There was a problem hiding this comment.
Yes that looks good to me! Thanks!
ffeu
changed the title
|
Tested on:
|
ffeu
changed the title
|
Hey @kroikie , I finished testing. |
|
Why is this closed? It's not working for me on IOS real device. |
… (2) (flutter#869) * changing plugin getToken to call platforms getTokens (2nd PR) * switching to get the token using callback and uniforming calls
…getToken (2) (flutter#869)" This reverts commit 7909d80.
… (2) (flutter#869) * changing plugin getToken to call platforms getTokens (2nd PR) * switching to get the token using callback and uniforming calls
intends to fix flutter/flutter#17699 and fix flutter/flutter#20378
Removing local cache of getToken() in the dart part of the plugin. Now getToken() calls directly its counterparts in the iOS and Android implementations. This enables obtaining its value without calling configure() or having to wait for a new token refresh.
replaces PR #789
@Hixie
@philipgiuliani
@calebisstupid
@beardo01
@muirandy