From 16318d9c700eab9161a617cae09db1b627e600ae Mon Sep 17 00:00:00 2001 From: Felix Hanau Date: Mon, 12 Aug 2024 23:59:52 -0400 Subject: [PATCH] [JSG] Use SetAccessorProperty() in registerReadonlyPrototypeProperty() This mirrors the approach used for registerPrototypeProperty(). --- src/workerd/api/http-test.js | 62 ++++++++++++------------- src/workerd/api/streams/streams-test.js | 48 +++++++++---------- src/workerd/api/tests/blob-test.js | 4 +- src/workerd/jsg/resource.h | 16 +++---- 4 files changed, 64 insertions(+), 66 deletions(-) diff --git a/src/workerd/api/http-test.js b/src/workerd/api/http-test.js index 9d647fd65e7..85f456c13a0 100644 --- a/src/workerd/api/http-test.js +++ b/src/workerd/api/http-test.js @@ -89,7 +89,6 @@ export const inspect = { const url = new URL("http://user:pass@placeholder:8787/path?a=1&a=2&b=3"); assert.strictEqual(util.inspect(url), `URL { - searchParams: URLSearchParams(3) { 'a' => '1', 'a' => '2', 'b' => '3' }, origin: 'http://placeholder:8787', href: 'http://user:pass@placeholder:8787/path?a=1&a=2&b=3', protocol: 'http:', @@ -100,7 +99,8 @@ export const inspect = { port: '8787', pathname: '/path', search: '?a=1&a=2&b=3', - hash: '' + hash: '', + searchParams: URLSearchParams(3) { 'a' => '1', 'a' => '2', 'b' => '3' } }` ); @@ -126,22 +126,22 @@ export const inspect = { }); assert.strictEqual(util.inspect(request), `Request { - keepalive: false, - integrity: '', - cf: undefined, - signal: AbortSignal { reason: undefined, aborted: false, onabort: null }, - fetcher: null, - redirect: 'follow', - headers: Headers(1) { 'content-type' => 'text/plain', [immutable]: false }, - url: 'http://placeholder', method: 'POST', - bodyUsed: false, + url: 'http://placeholder', + headers: Headers(1) { 'content-type' => 'text/plain', [immutable]: false }, + redirect: 'follow', + fetcher: null, + signal: AbortSignal { aborted: false, reason: undefined, onabort: null }, + cf: undefined, + integrity: '', + keepalive: false, body: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 7n - } + }, + bodyUsed: false }` ); @@ -149,21 +149,21 @@ export const inspect = { const response = await env.SERVICE.fetch("http://placeholder/not-found"); assert.strictEqual(util.inspect(response), `Response { - cf: undefined, - webSocket: null, - url: 'http://placeholder/not-found', - redirected: false, - ok: false, - headers: Headers(0) { [immutable]: true }, - statusText: 'Not Found', status: 404, - bodyUsed: false, + statusText: 'Not Found', + headers: Headers(0) { [immutable]: true }, + ok: false, + redirected: false, + url: 'http://placeholder/not-found', + webSocket: null, + cf: undefined, body: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 0n - } + }, + bodyUsed: false }` ); @@ -178,17 +178,17 @@ export const inspect = { assert.strictEqual(event.data, `MessageEvent { data: 'data', - isTrusted: true, - timeStamp: 0, - srcElement: WebSocket { extensions: '', protocol: '', url: null, readyState: 1 }, - currentTarget: WebSocket { extensions: '', protocol: '', url: null, readyState: 1 }, - returnValue: true, - defaultPrevented: false, - cancelable: false, - bubbles: false, - composed: false, - eventPhase: 2, type: 'message', + eventPhase: 2, + composed: false, + bubbles: false, + cancelable: false, + defaultPrevented: false, + returnValue: true, + currentTarget: WebSocket { readyState: 1, url: null, protocol: '', extensions: '' }, + srcElement: WebSocket { readyState: 1, url: null, protocol: '', extensions: '' }, + timeStamp: 0, + isTrusted: true, cancelBubble: false, NONE: 0, CAPTURING_PHASE: 1, diff --git a/src/workerd/api/streams/streams-test.js b/src/workerd/api/streams/streams-test.js index c4be0590573..7b81f779594 100644 --- a/src/workerd/api/streams/streams-test.js +++ b/src/workerd/api/streams/streams-test.js @@ -171,8 +171,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `FixedLengthStream { - writable: WritableStream { locked: false, [state]: 'writable', [expectsBytes]: true }, - readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 5n } + readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 5n }, + writable: WritableStream { locked: false, [state]: 'writable', [expectsBytes]: true } }` ); @@ -181,8 +181,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `FixedLengthStream { - writable: WritableStream { locked: true, [state]: 'writable', [expectsBytes]: true }, - readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 5n } + readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 5n }, + writable: WritableStream { locked: true, [state]: 'writable', [expectsBytes]: true } }` ); @@ -191,8 +191,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `FixedLengthStream { - writable: WritableStream { locked: true, [state]: 'writable', [expectsBytes]: true }, - readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 5n } + readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 5n }, + writable: WritableStream { locked: true, [state]: 'writable', [expectsBytes]: true } }` ); @@ -200,8 +200,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `FixedLengthStream { - writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true }, - readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 5n } + readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: 5n }, + writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true } }` ); @@ -209,8 +209,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `FixedLengthStream { - writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true }, - readable: ReadableStream { locked: true, [state]: 'readable', [supportsBYOB]: true, [length]: 5n } + readable: ReadableStream { locked: true, [state]: 'readable', [supportsBYOB]: true, [length]: 5n }, + writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true } }` ); @@ -218,8 +218,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `FixedLengthStream { - writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true }, - readable: ReadableStream { locked: true, [state]: 'readable', [supportsBYOB]: true, [length]: 2n } + readable: ReadableStream { locked: true, [state]: 'readable', [supportsBYOB]: true, [length]: 2n }, + writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true } }` ); @@ -227,8 +227,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `FixedLengthStream { - writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true }, - readable: ReadableStream { locked: true, [state]: 'readable', [supportsBYOB]: true, [length]: 0n } + readable: ReadableStream { locked: true, [state]: 'readable', [supportsBYOB]: true, [length]: 0n }, + writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true } }` ); @@ -236,8 +236,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `FixedLengthStream { - writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true }, - readable: ReadableStream { locked: true, [state]: 'closed', [supportsBYOB]: true, [length]: 0n } + readable: ReadableStream { locked: true, [state]: 'closed', [supportsBYOB]: true, [length]: 0n }, + writable: WritableStream { locked: true, [state]: 'closed', [expectsBytes]: true } }` ); } @@ -249,8 +249,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `IdentityTransformStream { - writable: WritableStream { locked: false, [state]: 'writable', [expectsBytes]: true }, - readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: undefined } + readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: undefined }, + writable: WritableStream { locked: false, [state]: 'writable', [expectsBytes]: true } }` ); @@ -260,8 +260,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `IdentityTransformStream { - writable: WritableStream { locked: true, [state]: 'errored', [expectsBytes]: true }, - readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: undefined } + readable: ReadableStream { locked: false, [state]: 'readable', [supportsBYOB]: true, [length]: undefined }, + writable: WritableStream { locked: true, [state]: 'errored', [expectsBytes]: true } }` ); @@ -269,8 +269,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `IdentityTransformStream { - writable: WritableStream { locked: true, [state]: 'errored', [expectsBytes]: true }, - readable: ReadableStream { locked: true, [state]: 'readable', [supportsBYOB]: true, [length]: undefined } + readable: ReadableStream { locked: true, [state]: 'readable', [supportsBYOB]: true, [length]: undefined }, + writable: WritableStream { locked: true, [state]: 'errored', [expectsBytes]: true } }` ); @@ -278,8 +278,8 @@ export const inspect = { assert.strictEqual( util.inspect(transformStream, inspectOpts), `IdentityTransformStream { - writable: WritableStream { locked: true, [state]: 'errored', [expectsBytes]: true }, - readable: ReadableStream { locked: true, [state]: 'errored', [supportsBYOB]: true, [length]: undefined } + readable: ReadableStream { locked: true, [state]: 'errored', [supportsBYOB]: true, [length]: undefined }, + writable: WritableStream { locked: true, [state]: 'errored', [expectsBytes]: true } }` ); } diff --git a/src/workerd/api/tests/blob-test.js b/src/workerd/api/tests/blob-test.js index 0d3ec0564a0..2c4ed8439e8 100644 --- a/src/workerd/api/tests/blob-test.js +++ b/src/workerd/api/tests/blob-test.js @@ -122,10 +122,10 @@ export default { export const testInspect = { async test(ctrl, env, ctx) { const blob = new Blob(["abc"], { type: "text/plain" }); - strictEqual(inspect(blob), "Blob { type: 'text/plain', size: 3 }"); + strictEqual(inspect(blob), "Blob { size: 3, type: 'text/plain' }"); const file = new File(["1"], "file.txt", { type: "text/plain", lastModified: 1000 }); - strictEqual(inspect(file), "File { lastModified: 1000, name: 'file.txt', type: 'text/plain', size: 1 }"); + strictEqual(inspect(file), "File { name: 'file.txt', lastModified: 1000, size: 1, type: 'text/plain' }"); } }; diff --git a/src/workerd/jsg/resource.h b/src/workerd/jsg/resource.h index ab9ff816a06..869efee6362 100644 --- a/src/workerd/jsg/resource.h +++ b/src/workerd/jsg/resource.h @@ -926,19 +926,17 @@ struct ResourceTypeBuilder { inline void registerReadonlyPrototypeProperty() { auto v8Name = v8StrIntern(isolate, name); - using Gcb = GetterCallback; + using Gcb = PropertyGetterCallback; if (!Gcb::enumerable) { inspectProperties->Set(v8Name, v8::False(isolate), v8::PropertyAttribute::ReadOnly); } + auto getterFn = v8::FunctionTemplate::New(isolate, Gcb::callback); - prototype->SetNativeDataProperty( - v8Name, - &Gcb::callback, - nullptr, - v8::Local(), - Gcb::enumerable ? v8::PropertyAttribute::ReadOnly - : static_cast( - v8::PropertyAttribute::ReadOnly | v8::PropertyAttribute::DontEnum)); + prototype->SetAccessorProperty(v8Name, getterFn, {}, + Gcb::enumerable ? + v8::PropertyAttribute::ReadOnly : + static_cast( + v8::PropertyAttribute::DontEnum | v8::PropertyAttribute::ReadOnly)); }