Skip to content

Commit ff67900

Browse files
mrousavyfacebook-github-bot
authored andcommitted
Forward String::length in RuntimeDecorator and WithRuntimeDecorator
Summary: `IRuntime::length(const String&)` came in with "Add `String::length` API" (590396bd4 on static_h, D96030262), and that commit didn't touch `decorator.h`. Neither `RuntimeDecorator` nor `WithRuntimeDecorator` overrides it, so on a decorated runtime `length()` falls back to the default `utf16(str).size()` and never reaches the wrapped runtime. It looks like an oversight. The same stack added `push`, `tryGetMutableBuffer`, `detached` and the TypedArray methods, and each of those commits added decorator overrides, including one for this method's overload `length(const TypedArray&)`. The `createError*` commit (facebook/hermes#2203) and this one didn't. For Hermes-backed decorators the value is already right, because Hermes' `utf16()` is exact, but every call copies the whole string instead of reading the length. Runtimes with a native `length()` and the default `utf16()` get wrong answers. React Native's `JSCRuntime` is one: built against the system JavaScriptCore and wrapped in a `RuntimeDecorator`, `'a\uD800b'` has length 1 instead of 3 and `'x\uDC00yz'` has 1 instead of 4. With this change both match the undecorated runtime. That setup lives outside this repo, so it isn't a test here. `WithRuntimeDecorator` needs the override too. Otherwise it would inherit the plain forward and `length()` would skip `Around`, which it currently goes through via `utf16()`. `TracingRuntime` no longer records an incidental `Utf16Record` when `length()` is called. That record only came from the `utf16()` fallback. I didn't add a record for `length()`: strings are immutable, and `TracingRuntime` already leaves pure queries like `size(Array)` untraced. X-link: facebook/hermes#2205 Reviewed By: avp Differential Revision: D123943189 Pulled By: lavenzg
1 parent 2bd0106 commit ff67900

2 files changed

Lines changed: 64 additions & 0 deletions

File tree

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,10 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
243243
return plain_.utf16(sym);
244244
}
245245

246+
size_t length(const String& str) override {
247+
return plain_.length(str);
248+
}
249+
246250
void getStringData(
247251
const jsi::String& str,
248252
void* ctx,
@@ -822,6 +826,11 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
822826
return RD::utf16(sym);
823827
}
824828

829+
size_t length(const String& str) override {
830+
Around around{with_};
831+
return RD::length(str);
832+
}
833+
825834
void getStringData(
826835
const jsi::String& str,
827836
void* ctx,

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,61 @@ TEST_P(JSITest, StringLengthTest) {
138138
EXPECT_EQ(invalid.length(rt), 2);
139139
}
140140

141+
TEST_P(JSITest, DecoratorForwardsStringLength) {
142+
// Decorators should forward length(const String&) to the decorated runtime
143+
// instead of falling back to the default Runtime::length, which copies the
144+
// string via utf16() and bypasses the runtime's own implementation.
145+
class Inner : public RuntimeDecorator<Runtime, Runtime> {
146+
public:
147+
explicit Inner(Runtime& rt) : RuntimeDecorator(rt) {}
148+
149+
size_t length(const String&) override {
150+
++lengthCalls;
151+
return 42;
152+
}
153+
154+
std::u16string utf16(const String& str) override {
155+
++utf16Calls;
156+
return RuntimeDecorator::utf16(str);
157+
}
158+
159+
int lengthCalls = 0;
160+
int utf16Calls = 0;
161+
};
162+
163+
class Outer : public RuntimeDecorator<Runtime, Runtime> {
164+
public:
165+
explicit Outer(Runtime& rt) : RuntimeDecorator(rt) {}
166+
};
167+
168+
struct Count {
169+
void before() {
170+
++beforeCalls;
171+
}
172+
void after() {
173+
++afterCalls;
174+
}
175+
int beforeCalls = 0;
176+
int afterCalls = 0;
177+
};
178+
179+
Inner inner(rt);
180+
String str = String::createFromAscii(inner, "hello");
181+
182+
Outer outer(inner);
183+
EXPECT_EQ(str.length(outer), 42);
184+
EXPECT_EQ(inner.lengthCalls, 1);
185+
EXPECT_EQ(inner.utf16Calls, 0);
186+
187+
Count count;
188+
WithRuntimeDecorator<Count> with(inner, count);
189+
EXPECT_EQ(str.length(with), 42);
190+
EXPECT_EQ(inner.lengthCalls, 2);
191+
EXPECT_EQ(inner.utf16Calls, 0);
192+
EXPECT_EQ(count.beforeCalls, 1);
193+
EXPECT_EQ(count.afterCalls, 1);
194+
}
195+
141196
TEST_P(JSITest, ObjectTest) {
142197
eval("x = {1:2, '3':4, 5:'six', 'seven':['eight', 'nine']}");
143198
Object x = rt.global().getPropertyAsObject(rt, "x");

0 commit comments

Comments
 (0)