-
Notifications
You must be signed in to change notification settings - Fork 371
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
Crash on bindService with broadcast receiver context #1935
Crash on bindService with broadcast receiver context #1935
Conversation
30d56d2
to
bf255f1
Compare
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.
Changes themselves look good!, but there are some commits in this PR that just add and then later remove code that we should clean up before merging.
bf255f1
to
9e3c8b8
Compare
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.
Ok I just cleaned them up and place them into one commit
Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on @brismithers and @emawby)
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.
Forgot to bring this up before, but could you add a comment to TrackGooglePurchases
of why applicationContext
is used? This way if this code is refactored in the future they will know applicationContext
is something that can't be removed.
9e3c8b8
to
8e6fb1f
Compare
@jkasten2 Good point, I have added the comment and explain the usage of applicationContext |
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.
Thanks for all the fixups, looks good now 👍
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.
Reviewed 3 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @emawby)
…ast-Context Crash on bindService with broadcast receiver context
…ast-Context Crash on bindService with broadcast receiver context
…ast-Context Crash on bindService with broadcast receiver context
Description
One Line Summary
Change broadcast receivers that calls initWithContext with the context from onReceive.
Details
The context carried by BroadcastReceiver.onReceive on certain older Android devices may be ReceiverRestrictedContext, disallowed from invoking bindService. However, this context type no longer exists in newer Android API levels, in which the method now carries the application context. Therefore, it explained why the observed crash affects some, but not all, devices. Accessing context.applicationContext retrieves the actual application context that registered the receiver and is permitted to call bindService.
Motivation
Fixing a critical bug that causes the client app to crash.
Scope
Customers that were not affected will continue remain unaffected because context.applicationContext is the same as context in devices with newer Android versions
Testing
Unit testing
Unable to reproduce the issue, since ReceiverRestrictedContext is no longer accessible
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is