[Android] [Fixed] fix indexed RAM bundle

[dratwas]

Co-Authored: @zamotany

Summary

With React Native 0.59.8 the app keeps crashing with indexed RAM bundle on Android with the following error:

2019-05-09 11:58:06.684 2793-2856/? E/AndroidRuntime: FATAL EXCEPTION: mqt_js
    Process: com.ramtestapp, PID: 2793
    com.facebook.jni.CppException: getPropertyAsObject: property '__fbRequireBatchedBridge' is not an Object
    
    no stack
        at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:29)
        at android.os.Looper.loop(Looper.java:193)
        at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:232)
        at java.lang.Thread.run(Thread.java:764)

After investigation we found that when using any bundle, let it be non-ram, FIle RAM bundle or Index RAM bundle, the CatalystInstanceImpl.java is always using loadScriptsFromAsset, which is calling CatalystInstanceImpl::jniLoadScriptFromAssets in C++. This method when checking if bundle is a RAM bundle, uses JniJSModulesUnbundle::isUnbundle which only check for js-modules/UNBUNDLE - file generated when building File RAM bundle. There is no other logic to handle Indexed RAM bundle, so it figures that the bundle is not RAM, cause there is no js-modules/UNBUNDLE file and tries to load as regular bundle and fails.

In this PR we added check if it is indexed RAM bundle in jniLoadScriptFromAssets and handle it if it is.

Changelog

[Android] [Fixed] fix indexed RAM bundle

Solves #21282

Test Plan

Add this setup to app/build.gradle file

project.ext.react = [
    bundleCommand: "ram-bundle",
    extraPackagerArgs: ["--indexed-ram-bundle"],
]

and run application in release mode.

Note

Please include it in the next 0.59 patch release

[pull-bot]

	Warnings
⚠️	❔ Base Branch - The base branch for this PR is something other than master. Are you sure you want to merge these changes into a stable release? If you are interested in backporting updates to an older release, the suggested approach is to land those changes on master first and then cherry-pick the commits into the branch for that release. The Releases Guide has more information.

	Messages
📖	📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.
📖	📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

Generated by 🚫 dangerJS against 779db36

[grabbou]

CC: @matthargett @fkgozali @cpojer

[matthargett]

The integration CI failure is a network problem that only occurred during package fetching in the node10 container. It would be nice to add a building and loading of indexed RAM bundle into the e2e suite, but otherwise this looks good.

[matthargett]

@hramos were you waiting to get the CI in shape before merging this?

[cpojer]

👍

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[cpojer]

I rebased this branch on master because we can't merge into the stable branch, it needs to land on master first.

Here are the build errors I'm getting at FB:

Summary: 
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/unique_ptr.h:765:34: error: call to implicitly-deleted copy constructor of 'std::basic_ifstream<char>'
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                                 ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/unique_ptr.h:765:34: error: call to implicitly-deleted copy constructor of 'std::basic_stringstream<char>'
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                                 ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/unique_ptr.h:765:34: error: call to implicitly-deleted copy constructor of 'std::basic_ifstream<char>'
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                                 ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/unique_ptr.h:765:34: error: call to implicitly-deleted copy constructor of 'std::basic_stringstream<char>'
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                                 ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ** Summary of failures encountered during the build **
stderr: In file included from xplat/js/react-native-github/ReactCommon/cxxreact/JSIndexedRAMBundle.cpp:6:
In file included from xplat/js/react-native-github/ReactCommon/cxxreact/JSIndexedRAMBundle.h:9:
In file included from /opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/memory:81:
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/unique_ptr.h:765:34: error: call to implicitly-deleted copy constructor of 'std::basic_ifstream<char>'
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                                 ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
xplat/js/react-native-github/ReactCommon/cxxreact/JSIndexedRAMBundle.cpp:23:19: note: in instantiation of function template specialization 'std::make_unique<std::basic_ifstream<char>, std::basic_ifstream<char> >' requested here
  m_bundle = std::make_unique<std::ifstream>(
                  ^
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/fstream:430:28: note: copy constructor of 'basic_ifstream<char, std::char_traits<char> >' is implicitly deleted because base class 'basic_istream<char, std::char_traits<char> >' has a deleted copy constructor
    class basic_ifstream : public basic_istream<_CharT, _Traits>
                           ^
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/istream:58:27: note: copy constructor of 'basic_istream<char, std::char_traits<char> >' is implicitly deleted because base class 'basic_ios<char, std::char_traits<char> >' has a deleted copy constructor
    class basic_istream : virtual public basic_ios<_CharT, _Traits>
                          ^
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/basic_ios.h:66:23: note: copy constructor of 'basic_ios<char, std::char_traits<char> >' is implicitly deleted because base class 'std::ios_base' has an inaccessible copy constructor
    class basic_ios : public ios_base
                      ^
In file included from xplat/js/react-native-github/ReactCommon/cxxreact/JSIndexedRAMBundle.cpp:6:
In file included from xplat/js/react-native-github/ReactCommon/cxxreact/JSIndexedRAMBundle.h:9:
In file included from /opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/memory:81:
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/unique_ptr.h:765:34: error: call to implicitly-deleted copy constructor of 'std::basic_stringstream<char>'
    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
                                 ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
xplat/js/react-native-github/ReactCommon/cxxreact/JSIndexedRAMBundle.cpp:36:55: note: in instantiation of function template specialization 'std::make_unique<std::basic_stringstream<char>, std::basic_stringstream<char> >' requested here
  std::unique_ptr<std::stringstream> tmpStream = std::make_unique<std::stringstream>(
                                                      ^
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/sstream:502:32: note: copy constructor of 'basic_stringstream<char, std::char_traits<char>, std::allocator<char> >' is implicitly deleted because base class 'basic_iostream<char, std::char_traits<char> >' has a deleted copy constructor
    class basic_stringstream : public basic_iostream<_CharT, _Traits>
                               ^
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/istream:796:7: note: copy constructor of 'basic_iostream<char, std::char_traits<char> >' is implicitly deleted because base class 'basic_istream<char, std::char_traits<char> >' has a deleted copy constructor
    : public basic_istream<_CharT, _Traits>,
      ^
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/istream:58:27: note: copy constructor of 'basic_istream<char, std::char_traits<char> >' is implicitly deleted because base class 'basic_ios<char, std::char_traits<char> >' has a deleted copy constructor
    class basic_istream : virtual public basic_ios<_CharT, _Traits>
                          ^
/opt/android_ndk/r15fb2/sources/cxx-stl/gnu-libstdc++/4.9/include/bits/basic_ios.h:66:23: note: copy constructor of 'basic_ios<char, std::char_traits<char> >' is implicitly deleted because base class 'std::ios_base' has an inaccessible copy constructor
    class basic_ios : public ios_base
                      ^
2 errors generated.
    When running <c++ preprocess_and_compile>.
    When building rule fbsource//xplat/js/react-native-github/ReactCommon/cxxreact:bridgeAndroid#android-armv7,compile-pic-JSIndexedRAMBundle.cpp.ocf2cf3cd.
Not all rules succeeded.

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @dratwas in d8fa120.

^{When will my fix make it into a release? | Upcoming Releases}

[dratwas]

Hey @cpojer i fixed compilation issues you mentioned. However there is a different issue on master branch with indexedRAMbundle app crashes with error -

com.facebook.react.common.JavascriptException: Exception in HostObject::get(propName:SourceCode): java.lang.AssertionError, stack:
    o@16.js:1:289
    getEnforcing@16.js:1:147
    <unknown>@176.js:1:151
    h@2:1670
    <unknown>@372.js:1:59
    h@2:1670
    <unknown>@371.js:1:49
    h@2:1670
    <unknown>@370.js:1:154
    h@2:1670
    <unknown>@366.js:1:573
    h@2:1670
    <unknown>@365.js:1:139
    h@2:1670
    <unknown>@1.js:1:58
    h@2:1670
    d@2:868
    global code@7:4
    
        at com.facebook.react.modules.core.ExceptionsManagerModule.showOrThrowError(ExceptionsManagerModule.java:54)
        at com.facebook.react.modules.core.ExceptionsManagerModule.reportFatalException(ExceptionsManagerModule.java:38)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
        at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:158)
        at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:29)
        at android.os.Looper.loop(Looper.java:193)
        at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:232)
        at java.lang.Thread.run(Thread.java:764)

I spend a moment on debugging and i think it is related to TurboModules since it crashes here

https://github.com/facebook/react-native/blob/master/Libraries/TurboModule/TurboModuleRegistry.js#L35

when tries to load module with name SourceCode, so the JS bundle is loaded properly.

It still works perfectly on 59.8