Fix portability issues to Linux, FreeBSD, and older libc++

[matthargett]

Test Plan:

I have a local Linux build (under Windows 10 WSL running Ubuntu 18.04) where I load an app bundle and ensure it doesn't crash, and run some of the gtest tests. I am currently unable to reproduce the cxxreact build in OSS, but this seems to be a known issue. For the record, my commandline for building and running the tests (after using vcpkg to install double-conversion, glog, gflags, and folly):
clang++-6.0 -fuse-ld=gold -o cxxreact-test_clang6_O3_lto_marchnative_g3 -std=c++14 -O3 -flto -g3 -march=native -L/mnt/c/Users/matt/workspace/vcpkg/installed/x64-linux/lib -I/mnt/c/Users/matt/workspace/vcpkg/installed/x64-linux/include -I/usr/include/webkitgtk-4.0/ -I. -ljavascriptcoregtk-4.0 -lfolly -lglog -ldouble-conversion fabric/events/*.cpp fabric/events/tests/EventsTest.cpp fabric/debug/*.cpp fabric/debug/tests/DebugStringConvertibleTest.cpp cxxreact/*.cpp cxxreact/tests/*.cpp microprofiler/*.cpp jsinspector/*.cpp privatedata/*.cpp jschelpers/JSC*.cpp jschelpers/[UV]*.cpp fabric/core/primitives/*.cpp fabric/core/tests/PrimitivesTest.cpp -lgtest -lgtest_main /mnt/c/Users/matt/workspace/vcpkg/installed/x64-linux/lib/libfolly.a /mnt/c/Users/matt/workspace/vcpkg/installed/x64-linux/lib/libgtest*.a /mnt/c/Users/matt/workspace/vcpkg/installed/x64-linux/lib/libglog.a /mnt/c/Users/matt/workspace/vcpkg/installed/x64-linux/lib/libdouble-conversion.a /mnt/c/Users/matt/workspace/vcpkg/installed/x64-linux/lib/libgflags.a -lpthread

Only two tests fail, which I will investigate and try to fix in a separate PR:

[----------] 2 tests from JSBigFileString
[ RUN      ] JSBigFileString.MapWholeFileTest
unknown file: Failure
C++ exception with description "basic_string::_M_construct null not valid" thrown in the test body.
[  FAILED  ] JSBigFileString.MapWholeFileTest (4 ms)
[ RUN      ] JSBigFileString.MapPartTest
unknown file: Failure
C++ exception with description "basic_string::_M_construct null not valid" thrown in the test body.
[  FAILED  ] JSBigFileString.MapPartTest (3 ms)
[----------] 2 tests from JSBigFileString (20 ms total)

Changelog:

[INTERNAL] [ENHANCEMENT] [ReactCommon/cxxreact] -Fix portability issues on Linux, FreeBSD, and older libc++ to enable fast unit and integration testing

• Fix missing includes that happen to work on some libc++ implementations due to transitive inclusion, like unistd.h
• Don't register hooks that rely on JSGlobalContextEnableDebugger and other things not available on all platforms (e.g. libjavascriptcoregtk-4.0) by guarding with APPLE and ANDROID
• Add nullptr checks for optionally set global functions before using them
• add getGlobalVariable to enable finer-grained unit testing

[shergin]

Oh nice! Thank you!
Could you please,

• Rebase on the fresh master. Seems a bunch of modified files were removed.
• Split this PR into a few logically isolated pieces (e.g. changing includes, using unique_ptr instead of shared_ptr, exceptions and so on.)

[matthargett]

Extracted glog changes to #21915 . Will rebase this one once that is merged.

[hramos]

Just talked to Marc, who will be taking a look at this soon. I’ll wait for his feedback before proceeding.

[mhorowitz]

My feedback is mainly about the API change to Instance::initializeBridge in 933253b.

What platform are you using with Dinkumware? The callers here are in iOS and Android specific code, and I'm surprised you can target either without using the Apple or Google libraries. The code in cxxreact is intended to be more portable, though.

I'm not excited about changing the API to support a flawed standard library. But, it should be possible to fix this without changing the API. Keep the argument typed as a unique_ptr, and instead of using std::make_unique or std::shared_ptr, avoid the implicit conversion by writing out the construction, like

std::unique_ptr<InstanceCallback>(new JInstanceCallback(...))

for example. make_unique is convenient for avoiding verbose code, but making this change should not change any semantics or behavior, so I think it's a better change if you need to hack things to deal with the toolchain. If the usage is in some code which isn't in the repo, then you can avoid making this diff at all, and just use the above style in your own code.

Finally, you should remove the android/log.h header in that commit, since it's likely to break Android builds.

[matthargett]

Thanks for taking the time to review!

The explicit cast didn't work for me, and I had tried that in September when I first ran into the issue. Something about what you said gave me a fresh idea, and what I ended up doing was wrapping the unique_ptr<> arg in a std::move() so the destructor wouldn't get triggered in the callsite context. That worked! I changed the shared_ptr<> back to unique_ptr<> in the CxxReact, iOS, and Android code.

Not sure how that log.h ended up there -- some weird auto-merge fail? I rm'd it.

If there's any more feedback, please let me know.

[facebook-github-bot]

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

[react-native-bot]

@matthargett merged commit 36916ee into facebook:master.