[native][android] Move database and MMKV initialization to CommCoreJSIModulePackage
ClosedPublic
Actions

Authored by ashoat on Sat, Apr 13, 4:56 PM.

Details

Reviewers

marcin
tomek

Commits

rCOMMc7ea09fab8ba: [native][android] Move database and MMKV initialization to…

Summary

This diff prevents the Android app from getting into a state that causes a crash loop, described in ENG-7696. It however does not resolve the crash loop if it already occurring.

Test Plan

Deleted the app off the simulator and redeployed, and the problem was gone (app stopped crash looping after being closed/reopened)
Deployed this fix to production and confirmed it fixed the issue for all 3 users that were affected

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Sat, Apr 13, 4:56 PM

ashoat added a child revision: D11660: [native] Recreate CommMMKV keys if they can't be loaded.Sat, Apr 13, 4:58 PM

Harbormaster completed remote builds in B28208: Diff 39098.Sat, Apr 13, 5:13 PM

ashoat requested review of this revision.Sat, Apr 13, 5:13 PM

Why moving this code to CommHybrid fixes the issue?

In D11659#334885, @tomek wrote:

Why moving this code to CommHybrid fixes the issue?

I believe because it executes later. More details here. My thinking was that it's safe to call from CommHybrid, since it's called from CommCoreJSIModulePackage.getJSIModules.

Since by that point other JSI libraries (presuamably including Expo libraries) seem ready for setup, I figured it was safe to call SecureStore from that location.

In D11659#334906, @ashoat wrote:

In D11659#334885, @tomek wrote:

Why moving this code to CommHybrid fixes the issue?

I believe because it executes later. More details here. My thinking was that it's safe to call from CommHybrid, since it's called from CommCoreJSIModulePackage.getJSIModules.

Since by that point other JSI libraries (presuamably including Expo libraries) seem ready for setup, I figured it was safe to call SecureStore from that location.

Maybe we can call it from getJSIModules? We're calling CommSecureStore.getInstance().initialize(secureStoreModuleSupplier); there and it is somehow connected.

This revision is now accepted and ready to land.Mon, Apr 15, 4:16 AM

Maybe we can call it from getJSIModules? We're calling CommSecureStore.getInstance().initialize(secureStoreModuleSupplier); there and it is somehow connected.

Probably a good idea. I'll test and confirm that this still works before landing.

Move to CommCoreJSIModulePackage

ashoat retitled this revision from [native][android] Move database and MMKV initialization to CommHybrid to [native][android] Move database and MMKV initialization to CommCoreJSIModulePackage.Mon, Apr 15, 4:39 AM

ashoat edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B28215: Diff 39111.Mon, Apr 15, 5:00 AM

Closed by commit rCOMMc7ea09fab8ba: [native][android] Move database and MMKV initialization to… (authored by ashoat). · Explain WhyMon, Apr 15, 5:12 AM

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMMc7ea09fab8ba: [native][android] Move database and MMKV initialization to….