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

Make initial launch clearing safer #24

Merged
merged 2 commits into from
Oct 8, 2018

Conversation

byencho
Copy link

@byencho byencho commented Oct 7, 2018

This PR updates the logic used to decide when to clear all saved data on a "fresh" launch to address issues discussed in #22. The existing logic assumed that Bridge.saveInstanceState / Bridge.restoreInstanceState would be used on all Activities in an app by placing those calls in a base class. A failure to do so could result in data being cleared at the wrong time. Since that is neither discussed in the README nor technically enforceable anyway, I've decided to remove that assumption.

The updates made here take advantage of the ActivityLifecycleCallbacks already being used to hook into Activity.onCreate events. I've simply shifted the existing logic from restoreInstanceState to the onActivityCreated callback of the ActivityLifecycleCallbacks. In order for this to work in all cases, however, I've needed to bump the min SDK from 12 to 14 to ensure that all users of Bridge get the same behavior. I highly doubt anyone was using this for < 14 so I don't see this as much of a problem.

@byencho byencho requested a review from weisers October 7, 2018 19:21
@byencho byencho mentioned this pull request Oct 7, 2018
@@ -95,12 +94,6 @@ private Bundle readFromDisk(@NonNull String uuid) {

@SuppressLint("NewApi")
private void registerForLifecycleEvents(@NonNull Context context) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version bump probably makes sense anyway, so 👍

Copy link

@weisers weisers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense

@byencho byencho merged commit e04ddba into master Oct 8, 2018
@byencho byencho deleted the make-initial-launch-clearing-safer branch October 8, 2018 13:42
@byencho
Copy link
Author

byencho commented Oct 8, 2018

Thanks @weisers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants