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

Do not null mBillingClient (#315) #359

Merged
merged 1 commit into from
Jan 7, 2019
Merged

Do not null mBillingClient (#315) #359

merged 1 commit into from
Jan 7, 2019

Conversation

mitchellmcm27
Copy link
Contributor

This PR tries to solve issues brought up in #315 , specifically that methods are invoked on mBillingClient when it is null. See my comment there for a discussion of the problem. Copy-pasted here, the variable clientReady is being used to track if the billing client setup has been completed, by setting it to false in the onBillingServiceDisconnected() callback. But mBillingClient itself is set to null immediately when endConnection() is called. This is a mismatch. It's possible for clientReady to remain set to true, even if mBillingClient is null. In that case, methods will be invoked by the on the null object instance. It seems that something is preventing onBillingServiceDisconnected() from being called.

Solution:

  • avoid setting mBillingClient to null. It is still not initialized in the class constructor, so will be null before ensureConnection() is called for the first time. Thus the null checks are still necessary.

  • Remove the clientReady property in favor of relying on mBillingClient.isReady() (again checking for null before calling).

The root cause of this issue is that when endConnection() is called as a React Native component is unmounting, it seems that the onBillingServiceDisconnected() callback is never invoked. I don't know whether this is intended by the API or not, so I did not attempt to change this. I suggest someone with more experience in Android In App Billing review this PR before it is merged.

@hyochan
Copy link
Owner

hyochan commented Jan 7, 2019

Thanks for the PR and the description. Looks like it has been rollbacked. However, since I agree that this isn't the solution. I will merge this now. Thank you.

@hyochan hyochan merged commit 0dda7f4 into hyochan:master Jan 7, 2019
@hyochan hyochan added the 🛠 bugfix All kinds of bug fixes label Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠 bugfix All kinds of bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants