Skip to content

Commit 1904f58

Browse files
mrousavyfacebook-github-bot
authored andcommitted
Forward createValueFromJsonUtf8 in RuntimeDecorator
Summary: `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 Differential Revision: D124177263 Pulled By: lavenzg
1 parent 82def6e commit 1904f58

14 files changed

Lines changed: 63 additions & 0 deletions

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
148148
Instrumentation& instrumentation() override {
149149
return *this;
150150
}
151+
Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override {
152+
return plain_.createValueFromJsonUtf8(json, length);
153+
}
151154

152155
protected:
153156
// plain is generally going to be a reference to an object managed

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1981,6 +1981,54 @@ TEST_P(JSITest, CreateFromUtf16Test) {
19811981
EXPECT_EQ(cp, 55357); // 0xD83D in decimal
19821982
}
19831983

1984+
TEST_P(JSITest, CreateValueFromJsonUtf8Test) {
1985+
uint8_t jsonText[] = "{\"a\": 1}";
1986+
1987+
// This Runtime Decorator is used to test the default createValueFromJsonUtf8
1988+
// implementation for VMs that do not provide their own implementation
1989+
class DefaultRD : public RuntimeDecorator<Runtime, Runtime> {
1990+
public:
1991+
explicit DefaultRD(Runtime& rt) : RuntimeDecorator(rt) {}
1992+
1993+
Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override {
1994+
return Runtime::createValueFromJsonUtf8(json, length);
1995+
}
1996+
};
1997+
1998+
DefaultRD defaultRd(rt);
1999+
Object obj =
2000+
Value::createFromJsonUtf8(defaultRd, jsonText, sizeof(jsonText) - 1)
2001+
.getObject(defaultRd);
2002+
EXPECT_EQ(obj.getProperty(defaultRd, "a").getNumber(), 1);
2003+
2004+
// A RuntimeDecorator must forward createValueFromJsonUtf8 to the runtime it
2005+
// decorates, like every other method.
2006+
class CountingRD : public RuntimeDecorator<Runtime, Runtime> {
2007+
public:
2008+
explicit CountingRD(Runtime& rt) : RuntimeDecorator(rt) {}
2009+
2010+
Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override {
2011+
++count;
2012+
return plain().createValueFromJsonUtf8(json, length);
2013+
}
2014+
2015+
int count = 0;
2016+
};
2017+
2018+
// Overrides nothing, so it should forward everything to CountingRD.
2019+
class OuterRD : public RuntimeDecorator<Runtime, Runtime> {
2020+
public:
2021+
explicit OuterRD(Runtime& rt) : RuntimeDecorator(rt) {}
2022+
};
2023+
2024+
CountingRD counting(rt);
2025+
OuterRD outer(counting);
2026+
obj = outer.createValueFromJsonUtf8(jsonText, sizeof(jsonText) - 1)
2027+
.getObject(outer);
2028+
EXPECT_EQ(counting.count, 1);
2029+
EXPECT_EQ(obj.getProperty(outer, "a").getNumber(), 1);
2030+
}
2031+
19842032
TEST_P(JSITest, GetStringDataTest) {
19852033
// This Runtime Decorator is used to test the default getStringData
19862034
// implementation for VMs that do not provide their own implementation

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6264,6 +6264,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
62646264
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
62656265
public virtual facebook::jsi::Instrumentation& instrumentation() override;
62666266
public virtual facebook::jsi::Object global() override;
6267+
public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
62676268
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
62686269
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
62696270
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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10880,6 +10880,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
1088010880
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
1088110881
public virtual facebook::jsi::Instrumentation& instrumentation() override;
1088210882
public virtual facebook::jsi::Object global() override;
10883+
public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
1088310884
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
1088410885
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
1088510886
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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6113,6 +6113,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
61136113
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
61146114
public virtual facebook::jsi::Instrumentation& instrumentation() override;
61156115
public virtual facebook::jsi::Object global() override;
6116+
public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
61166117
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
61176118
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
61186119
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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6113,6 +6113,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
61136113
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
61146114
public virtual facebook::jsi::Instrumentation& instrumentation() override;
61156115
public virtual facebook::jsi::Object global() override;
6116+
public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
61166117
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
61176118
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
61186119
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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8108,6 +8108,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
81088108
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
81098109
public virtual facebook::jsi::Instrumentation& instrumentation() override;
81108110
public virtual facebook::jsi::Object global() override;
8111+
public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
81118112
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
81128113
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
81138114
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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12739,6 +12739,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
1273912739
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
1274012740
public virtual facebook::jsi::Instrumentation& instrumentation() override;
1274112741
public virtual facebook::jsi::Object global() override;
12742+
public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
1274212743
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
1274312744
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
1274412745
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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7959,6 +7959,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
79597959
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
79607960
public virtual facebook::jsi::Instrumentation& instrumentation() override;
79617961
public virtual facebook::jsi::Object global() override;
7962+
public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
79627963
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
79637964
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
79647965
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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7967,6 +7967,7 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
79677967
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
79687968
public virtual facebook::jsi::Instrumentation& instrumentation() override;
79697969
public virtual facebook::jsi::Object global() override;
7970+
public virtual facebook::jsi::Value createValueFromJsonUtf8(const uint8_t* json, size_t length) override;
79707971
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
79717972
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
79727973
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

0 commit comments

Comments
 (0)