Skip to content

Commit

Permalink
Update URLSearchParams to use jsg::Function
Browse files Browse the repository at this point in the history
  • Loading branch information
jasnell committed Aug 18, 2023
1 parent 973c478 commit 7d8521c
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 38 deletions.
23 changes: 11 additions & 12 deletions src/workerd/api/url-standard.c++
Original file line number Diff line number Diff line change
Expand Up @@ -2079,25 +2079,24 @@ jsg::Ref<URLSearchParams::ValueIterator> URLSearchParams::values(jsg::Lock&) {

void URLSearchParams::forEach(
jsg::Lock& js,
jsg::V8Ref<v8::Function> callback,
jsg::Function<void(jsg::UsvStringPtr, jsg::UsvStringPtr, jsg::Ref<URLSearchParams>)> callback,
jsg::Optional<jsg::Value> thisArg) {
auto cb = callback.getHandle(js);
auto this_ = thisArg.map([&js](jsg::Value& v) { return v.getHandle(js); })
.orDefault(js.v8Undefined());
auto query = KJ_ASSERT_NONNULL(JSG_THIS.tryGetHandle(js.v8Isolate));
// On each iteration of the for loop, a JavaScript callback is invokved. If a new
auto receiver = js.v8Undefined();
KJ_IF_MAYBE(arg, thisArg) {
auto handle = arg->getHandle(js);
if (!handle->IsNullOrUndefined()) {
receiver = handle;
}
}
callback.setReceiver(js.v8Ref(receiver));
// On each iteration of the for loop, a JavaScript callback is invoked. If a new
// item is appended to the URLSearchParams within that function, the loop must pick
// it up. Using the classic for (;;) syntax here allows for that. However, this does
// mean that it's possible for a user to trigger an infinite loop here if new items
// are added to the search params unconditionally on each iteration.
for (size_t i = 0; i < list.size(); i++) {
auto& entry = list[i];
v8::Local<v8::Value> args[3] = {
jsg::v8Str(js.v8Isolate, entry.value),
jsg::v8Str(js.v8Isolate, entry.name),
query,
};
jsg::check(cb->Call(js.v8Context(), this_, 3, args));
callback(js, entry.value, entry.name, JSG_THIS);
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/workerd/api/url-standard.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,9 @@ class URLSearchParams: public jsg::Object {
IteratorState,
valueIteratorNext)

void forEach(
jsg::Lock& js,
jsg::V8Ref<v8::Function> callback,
jsg::Optional<jsg::Value> thisArg);
void forEach(jsg::Lock&,
jsg::Function<void(jsg::UsvStringPtr, jsg::UsvStringPtr, jsg::Ref<URLSearchParams>)>,
jsg::Optional<jsg::Value>);

jsg::UsvString toString();

Expand Down
30 changes: 12 additions & 18 deletions src/workerd/api/url.c++
Original file line number Diff line number Diff line change
Expand Up @@ -587,31 +587,25 @@ void URLSearchParams::sort() {

void URLSearchParams::forEach(
jsg::Lock& js,
jsg::V8Ref<v8::Function> callback,
jsg::Function<void(kj::StringPtr, kj::StringPtr, jsg::Ref<URLSearchParams>)> callback,
jsg::Optional<jsg::Value> thisArg) {
auto localCallback = callback.getHandle(js);
auto localThisArg = thisArg.map([&js](jsg::Value& v) { return v.getHandle(js); })
.orDefault(js.v8Undefined());
// JSG_THIS.getHandle() is guaranteed safe because `forEach()` is only called
// from JavaScript, which means a Headers JS wrapper object must already exist.
auto localParams = KJ_ASSERT_NONNULL(JSG_THIS.tryGetHandle(js.v8Isolate));

// On each iteration of the for loop, a JavaScript callback is invokved. If a new
auto receiver = js.v8Undefined();
KJ_IF_MAYBE(arg, thisArg) {
auto handle = arg->getHandle(js);
if (!handle->IsNullOrUndefined()) {
receiver = handle;
}
}
callback.setReceiver(js.v8Ref(receiver));

// On each iteration of the for loop, a JavaScript callback is invoked. If a new
// item is appended to the this->url->query within that function, the loop must pick
// it up. Using the classic for (;;) syntax here allows for that. However, this does
// mean that it's possible for a user to trigger an infinite loop here if new items
// are added to the search params unconditionally on each iteration.
for (size_t i = 0; i < this->url->query.size(); i++) {
auto& [key, value] = this->url->query[i];
static constexpr auto ARG_COUNT = 3;
v8::Local<v8::Value> args[ARG_COUNT] = {
jsg::v8Str(js.v8Isolate, value),
jsg::v8Str(js.v8Isolate, key),
localParams,
};
// Call jsg::check() to propagate exceptions, but we don't expect any
// particular return value.
jsg::check(localCallback->Call(js.v8Context(), localThisArg, ARG_COUNT, args));
callback(js, value, key, JSG_THIS);
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/workerd/api/url.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,9 @@ class URLSearchParams: public jsg::Object {
IteratorState,
valueIteratorNext)

void forEach(
jsg::Lock& js,
jsg::V8Ref<v8::Function> callback,
jsg::Optional<jsg::Value> thisArg);
void forEach(jsg::Lock&,
jsg::Function<void(kj::StringPtr, kj::StringPtr, jsg::Ref<URLSearchParams>)>,
jsg::Optional<jsg::Value>);

kj::String toString();

Expand Down

0 comments on commit 7d8521c

Please sign in to comment.