-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: support typed arrays in indexeddb #34949
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,8 @@ | |
* limitations under the License. | ||
*/ | ||
|
||
type TypedArrayKind = 'i8' | 'ui8' | 'ui8c' | 'i16' | 'ui16' | 'i32' | 'ui32' | 'f32' | 'f64' | 'bi64' | 'bui64'; | ||
|
||
export type SerializedValue = | ||
undefined | boolean | number | string | | ||
{ v: 'null' | 'undefined' | 'NaN' | 'Infinity' | '-Infinity' | '-0' } | | ||
|
@@ -25,7 +27,8 @@ export type SerializedValue = | |
{ a: SerializedValue[], id: number } | | ||
{ o: { k: string, v: SerializedValue }[], id: number } | | ||
{ ref: number } | | ||
{ h: number }; | ||
{ h: number } | | ||
{ ta: { b: string, k: TypedArrayKind } }; | ||
|
||
export type HandleOrValue = { h: number } | { fallThrough: any }; | ||
|
||
|
@@ -68,6 +71,48 @@ export function source() { | |
} | ||
} | ||
|
||
function isTypedArray(obj: any, constructor: Function): boolean { | ||
try { | ||
return obj instanceof constructor || Object.prototype.toString.call(obj) === `[object ${constructor.name}]`; | ||
} catch (error) { | ||
return false; | ||
} | ||
} | ||
|
||
const typedArrayConstructors: Record<TypedArrayKind, Function> = { | ||
i8: Int8Array, | ||
ui8: Uint8Array, | ||
ui8c: Uint8ClampedArray, | ||
i16: Int16Array, | ||
ui16: Uint16Array, | ||
i32: Int32Array, | ||
ui32: Uint32Array, | ||
// TODO: add Float16Array once it's in baseline | ||
f32: Float32Array, | ||
f64: Float64Array, | ||
bi64: BigInt64Array, | ||
bui64: BigUint64Array, | ||
}; | ||
|
||
function typedArrayToBase64(array: any) { | ||
/** | ||
* Firefox does not support iterating over typed arrays, so we use `.toBase64`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps instead of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tried that, but doesn't work. It looks like any kind of iteration is forbidden: https://searchfox.org/mozilla-central/source/js/xpconnect/wrappers/XrayWrapper.cpp#576 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What exactly does this mean? You should be able to iterate arrays and use Array.from on them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But not inside the utility world, apparently 🤷 |
||
* Error: 'Accessing TypedArray data over Xrays is slow, and forbidden in order to encourage performant code. To copy TypedArrays across origin boundaries, consider using Components.utils.cloneInto().' | ||
*/ | ||
if ('toBase64' in array) | ||
return array.toBase64(); | ||
const binary = Array.from(new Uint8Array(array.buffer, array.byteOffset, array.byteLength)).map(b => String.fromCharCode(b)).join(''); | ||
return btoa(binary); | ||
} | ||
|
||
function base64ToTypedArray(base64: string, TypedArrayConstructor: any) { | ||
const binary = atob(base64); | ||
const bytes = new Uint8Array(binary.length); | ||
for (let i = 0; i < binary.length; i++) | ||
bytes[i] = binary.charCodeAt(i); | ||
return new TypedArrayConstructor(bytes.buffer); | ||
} | ||
|
||
function parseEvaluationResultValue(value: SerializedValue, handles: any[] = [], refs: Map<number, object> = new Map()): any { | ||
if (Object.is(value, undefined)) | ||
return undefined; | ||
|
@@ -119,6 +164,8 @@ export function source() { | |
} | ||
if ('h' in value) | ||
return handles[value.h]; | ||
if ('ta' in value) | ||
return base64ToTypedArray(value.ta.b, typedArrayConstructors[value.ta.k]); | ||
} | ||
return value; | ||
} | ||
|
@@ -186,6 +233,10 @@ export function source() { | |
return { u: value.toJSON() }; | ||
if (isRegExp(value)) | ||
return { r: { p: value.source, f: value.flags } }; | ||
for (const [k, ctor] of Object.entries(typedArrayConstructors) as [TypedArrayKind, Function][]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd start with looking up constructor.name in a set instead of iterating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that won't work on Firefox, see impl of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does work for me, are you saying this is also due to utility worlds? Something is fishy here. Do we have a bug in the way we initialize utility context? It needs to be proper JS context. |
||
if (isTypedArray(value, ctor)) | ||
return { ta: { b: typedArrayToBase64(value), k } }; | ||
} | ||
|
||
const id = visitorInfo.visited.get(value); | ||
if (id) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like we need a bunch of page.evaluate tests for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests in
page-evaluate.spec.ts
all go through the protocol serializer, which also doesn't yet have support. Do we test the server-side ofpage.evaluate
in isolation?I was planning to add support to the protocol serializer in a follow-up, happy to add page.evaluate tests in there.