Page MenuHomePhabricator

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

Authored by ashoat on Apr 13 2024, 4:56 PM.
Tags
None
Referenced Files
F3392270: D11659.id39098.diff
Sat, Nov 30, 7:51 AM
Unknown Object (File)
Sat, Nov 23, 12:30 AM
Unknown Object (File)
Sun, Nov 3, 10:45 AM
Unknown Object (File)
Oct 13 2024, 1:27 AM
Unknown Object (File)
Oct 13 2024, 1:27 AM
Unknown Object (File)
Oct 13 2024, 1:27 AM
Unknown Object (File)
Oct 13 2024, 1:27 AM
Unknown Object (File)
Oct 13 2024, 1:26 AM
Subscribers
None

Details

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
  1. Deleted the app off the simulator and redeployed, and the problem was gone (app stopped crash looping after being closed/reopened)
  2. 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

Why moving this code to CommHybrid fixes the issue?

ashoat edited the test plan for this revision. (Show Details)EditedApr 15 2024, 3:13 AM

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.

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.Apr 15 2024, 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.Apr 15 2024, 4:39 AM
ashoat edited the summary of this revision. (Show Details)