Forward createValueFromJsonUtf8 in RuntimeDecorator (#58960)

Summary:
Pull Request resolved: #58960

`RuntimeDecorator` doesn't forward `createValueFromJsonUtf8` to the plain runtime, so decorated runtimes use the `JSON.parse`-based default from jsi.cpp instead of the plain runtime's parser. For Hermes that means `makeThreadSafeHermesRuntime` and anything built on `WithRuntimeDecorator` never reach the native parser from a3f615cec.

I think this was missed rather than chosen. `WithRuntimeDecorator` already overrides it (added in c2a0da2ae as `Around` plus `RD::createValueFromJsonUtf8`), which only makes sense if `RuntimeDecorator` forwards it. Today that `RD::` call lands in the default, so it takes one `Around` plus several nested ones for `global`, `getProperty`, `createStringFromUtf8` and `call`. It's also observable: on a thread-safe runtime, after `JSON.parse = () => 'hijacked'`, `Value::createFromJsonUtf8` returns `'hijacked'`, and after `delete globalThis.JSON` it throws. A plain `HermesRuntime` parses normally in both cases.

`TracingRuntime` relies on the current behavior. JSON parsing only shows up in traces because the default splits into traced `global`/`getProperty`/`call` calls. Forwarding alone would hand back a value from the plain runtime with no def, and replay would hit the "has no def" assert in `useObjectID`. So `TracingRuntime` now overrides `createValueFromJsonUtf8` and calls `jsi::Runtime::createValueFromJsonUtf8` explicitly. That runs the same code as before, so traces are unchanged and there's no new record type. If you'd prefer a dedicated SynthTrace record I can do that instead.

A side effect: decorators that watched JSON parsing through `global`/`getProperty`/`call` now need to override `createValueFromJsonUtf8`, like every other forwarded method. For example, `TimedRuntime` no longer counts JSON parsing under its `call` timer. The other methods decorators don't forward yet are the `createError*` family (facebook/hermes#2203) and `String::length` (facebook/hermes#2205). React Native's `decorator.h` has the same gap.

X-link: facebook/hermes#2206

Test Plan:
New tests:
- `JSITest.CreateValueFromJsonUtf8Test` checks that parsing through a `RuntimeDecorator` reaches the wrapped runtime, and keeps coverage of the jsi.cpp default through a decorator that overrides back to it. It calls `outer.createValueFromJsonUtf8` directly to check that the override remains public.
- `HermesRuntimeTest.CreateValueFromJsonUtf8IgnoresGlobalJSON` replaces `JSON.parse` and checks that parsing still works, on the plain and thread-safe runtimes.
- `SynthTraceReplayTest.CreateValueFromJsonUtf8Replay` parses on the tracing runtime, uses the result from native code, and replays with verification.

Without the change the first two fail (the second only on the thread-safe runtime). With only the `decorator.h` part, the SynthTrace test aborts in `useObjectID`. With the full change all pass, and so does the full `APITests` binary (360 tests) in a Debug + ASan build. `APILeanTests` also passes.

The direct call fails to compile with the previous header because `createValueFromJsonUtf8` is protected. With this revision, `cmake --build cmake-build-asan --target check-hermes -j 12` passes in Debug + ASan: 4,126 expected passes, 6 expected failures, and 197 unsupported tests. Node-API checks: 41 passes and 23 skips.

Reviewed By: javache

Differential Revision: D124177263

Pulled By: lavenzg

fbshipit-source-id: 973dc8b3c69002a81e634bb6c992f5f167e13bf2

Author: mrousavy

packages/react-native/ReactCommon/jsi/jsi/decorator.h

@@ -148,6 +148,9 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
   Instrumentation& instrumentation() override {
     return *this;
   }
+  Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override {
+    return plain_.createValueFromJsonUtf8(json, length);
+  }
 
   Value createError(const String& msg) override {
     return plain().createError(msg);

packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp

@@ -2144,6 +2144,54 @@ TEST_P(JSITest, CreateFromUtf16Test) {
   EXPECT_EQ(cp, 55357); // 0xD83D in decimal
 }
 
+TEST_P(JSITest, CreateValueFromJsonUtf8Test) {
+  uint8_t jsonText[] = "{\"a\": 1}";
+
+  // This Runtime Decorator is used to test the default createValueFromJsonUtf8
+  // implementation for VMs that do not provide their own implementation
+  class DefaultRD : public RuntimeDecorator<Runtime, Runtime> {
+   public:
+    explicit DefaultRD(Runtime& rt) : RuntimeDecorator(rt) {}
+
+    Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override {
+      return Runtime::createValueFromJsonUtf8(json, length);
+    }
+  };
+
+  DefaultRD defaultRd(rt);
+  Object obj =
+      Value::createFromJsonUtf8(defaultRd, jsonText, sizeof(jsonText) - 1)
+          .getObject(defaultRd);
+  EXPECT_EQ(obj.getProperty(defaultRd, "a").getNumber(), 1);
+
+  // A RuntimeDecorator must forward createValueFromJsonUtf8 to the runtime it
+  // decorates, like every other method.
+  class CountingRD : public RuntimeDecorator<Runtime, Runtime> {
+   public:
+    explicit CountingRD(Runtime& rt) : RuntimeDecorator(rt) {}
+
+    Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override {
+      ++count;
+      return plain().createValueFromJsonUtf8(json, length);
+    }
+
+    int count = 0;
+  };
+
+  // Overrides nothing, so it should forward everything to CountingRD.
+  class OuterRD : public RuntimeDecorator<Runtime, Runtime> {
+   public:
+    explicit OuterRD(Runtime& rt) : RuntimeDecorator(rt) {}
+  };
+
+  CountingRD counting(rt);
+  OuterRD outer(counting);
+  obj = outer.createValueFromJsonUtf8(jsonText, sizeof(jsonText) - 1)
+            .getObject(outer);
+  EXPECT_EQ(counting.count, 1);
+  EXPECT_EQ(obj.getProperty(outer, "a").getNumber(), 1);
+}
+
 TEST_P(JSITest, GetStringDataTest) {
   // This Runtime Decorator is used to test the default getStringData
   // implementation for VMs that do not provide their own implementation

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

@@ -6198,6 +6198,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
   public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
+  public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
   public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
   public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
   public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api

@@ -10553,6 +10553,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
   public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
+  public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
   public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
   public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
   public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api

@@ -6047,6 +6047,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
   public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
+  public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
   public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
   public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
   public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api

@@ -6047,6 +6047,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
   public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
+  public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
   public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
   public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
   public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api

@@ -8009,6 +8009,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
   public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
+  public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
   public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
   public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
   public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api

@@ -12396,6 +12396,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
   public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
+  public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
   public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
   public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
   public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api

@@ -7860,6 +7860,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
   public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
+  public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
   public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
   public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
   public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api

@@ -7868,6 +7868,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
   public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
   public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
+  public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
   public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
   public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
   public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;