Skip to content

Introduce StructuredSerializeInternal forStorage parameter - #5576

Merged
webkit-early-warning-system merged 1 commit into
WebKit:mainfrom
youennf:eng/Introduce-StructuredSerializeInternal-forStorage-parameter
Oct 20, 2022
Merged

Introduce StructuredSerializeInternal forStorage parameter#5576
webkit-early-warning-system merged 1 commit into
WebKit:mainfrom
youennf:eng/Introduce-StructuredSerializeInternal-forStorage-parameter

Conversation

@youennf

@youennf youennf commented Oct 20, 2022

Copy link
Copy Markdown
Contributor

664fe94

Introduce StructuredSerializeInternal forStorage parameter
https://bugs.webkit.org/show_bug.cgi?id=246744
rdar://problem/101367447

Reviewed by Alex Christensen.

We introduce a SerializationForStorage (No/Yes) parameter to SerializedScriptValue::create.
This allows to throw in case we are serializing objects that are serializable but are throwing for forStorage=yes cases.
Make use of SerializationForStorage parameter for WebCodecsEncodedVideoChunk as per https://w3c.github.io/webcodecs/#encodedvideochunk-serialization.

The default is SerializationForStorage = No.
We set SerializationForStorage = Yes for IDB and IDLSerializedScriptValue, which is used for History pushState.
Both IDB and History.pushState are using https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializeforstorage.

Covered by added tests.

* LayoutTests/imported/w3c/web-platform-tests/webcodecs/chunk-serialization.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/chunk-serialization.any.js:
(test):
* Source/WebCore/Modules/indexeddb/IDBCursor.cpp:
(WebCore::IDBCursor::update):
* Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:
(WebCore::IDBObjectStore::putOrAdd):
* Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp:
(WebCore::AudioWorkletNode::create):
* Source/WebCore/bindings/js/JSDOMConvertSerializedScriptValue.h:
(WebCore::Converter<IDLSerializedScriptValue<T>>::convert):
* Source/WebCore/bindings/js/JSDOMWrapper.cpp:
(WebCore::cloneAcrossWorlds):
* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneSerializer::serialize):
(WebCore::CloneSerializer::CloneSerializer):
(WebCore::CloneSerializer::dumpIfTerminal):
(WebCore::SerializedScriptValue::create):
* Source/WebCore/bindings/js/SerializedScriptValue.h:
(WebCore::SerializedScriptValue::convert):
* Source/WebCore/dom/ErrorEvent.cpp:
(WebCore::ErrorEvent::trySerializeError):
* Source/WebCore/dom/PopStateEvent.cpp:
(WebCore::PopStateEvent::trySerializeState):
* Source/WebCore/page/DOMWindow.cpp:
(WebCore::DOMWindow::postMessage):
* Source/WebCore/page/WindowOrWorkerGlobalScope.cpp:
(WebCore::WindowOrWorkerGlobalScope::structuredClone):
* Source/WebCore/workers/DedicatedWorkerGlobalScope.cpp:
(WebCore::DedicatedWorkerGlobalScope::postMessage):
* Source/WebCore/workers/Worker.cpp:
(WebCore::Worker::postMessage):
* Source/WebCore/workers/service/ExtendableMessageEvent.cpp:
(WebCore::ExtendableMessageEvent::ExtendableMessageEvent):
* Source/WebCore/workers/service/ServiceWorker.cpp:
(WebCore::ServiceWorker::postMessage):
* Source/WebCore/workers/service/ServiceWorkerClient.cpp:
(WebCore::ServiceWorkerClient::postMessage):

Canonical link: https://commits.webkit.org/255788@main

5806333

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🛠 mac-AS-debug 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 api-mac 🧪 api-gtk
✅ 🛠 tv 🧪 mac-wk1
✅ 🛠 tv-sim ✅ 🧪 mac-wk2
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress

@youennf
youennf requested review from cdumez and rniwa as code owners October 20, 2022 05:59
@youennf youennf self-assigned this Oct 20, 2022
@youennf youennf added WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). WebKit Nightly Build labels Oct 20, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

: bool

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will change

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec says a TypeError.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are changing this, see w3c/webcodecs#589

@youennf
youennf force-pushed the eng/Introduce-StructuredSerializeInternal-forStorage-parameter branch from 4c01a49 to 5806333 Compare October 20, 2022 15:08
@youennf youennf added the merge-queue Applied to send a pull request to merge-queue label Oct 20, 2022
https://bugs.webkit.org/show_bug.cgi?id=246744
rdar://problem/101367447

Reviewed by Alex Christensen.

We introduce a SerializationForStorage (No/Yes) parameter to SerializedScriptValue::create.
This allows to throw in case we are serializing objects that are serializable but are throwing for forStorage=yes cases.
Make use of SerializationForStorage parameter for WebCodecsEncodedVideoChunk as per https://w3c.github.io/webcodecs/#encodedvideochunk-serialization.

The default is SerializationForStorage = No.
We set SerializationForStorage = Yes for IDB and IDLSerializedScriptValue, which is used for History pushState.
Both IDB and History.pushState are using https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializeforstorage.

Covered by added tests.

* LayoutTests/imported/w3c/web-platform-tests/webcodecs/chunk-serialization.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/chunk-serialization.any.js:
(test):
* Source/WebCore/Modules/indexeddb/IDBCursor.cpp:
(WebCore::IDBCursor::update):
* Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:
(WebCore::IDBObjectStore::putOrAdd):
* Source/WebCore/Modules/webaudio/AudioWorkletNode.cpp:
(WebCore::AudioWorkletNode::create):
* Source/WebCore/bindings/js/JSDOMConvertSerializedScriptValue.h:
(WebCore::Converter<IDLSerializedScriptValue<T>>::convert):
* Source/WebCore/bindings/js/JSDOMWrapper.cpp:
(WebCore::cloneAcrossWorlds):
* Source/WebCore/bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneSerializer::serialize):
(WebCore::CloneSerializer::CloneSerializer):
(WebCore::CloneSerializer::dumpIfTerminal):
(WebCore::SerializedScriptValue::create):
* Source/WebCore/bindings/js/SerializedScriptValue.h:
(WebCore::SerializedScriptValue::convert):
* Source/WebCore/dom/ErrorEvent.cpp:
(WebCore::ErrorEvent::trySerializeError):
* Source/WebCore/dom/PopStateEvent.cpp:
(WebCore::PopStateEvent::trySerializeState):
* Source/WebCore/page/DOMWindow.cpp:
(WebCore::DOMWindow::postMessage):
* Source/WebCore/page/WindowOrWorkerGlobalScope.cpp:
(WebCore::WindowOrWorkerGlobalScope::structuredClone):
* Source/WebCore/workers/DedicatedWorkerGlobalScope.cpp:
(WebCore::DedicatedWorkerGlobalScope::postMessage):
* Source/WebCore/workers/Worker.cpp:
(WebCore::Worker::postMessage):
* Source/WebCore/workers/service/ExtendableMessageEvent.cpp:
(WebCore::ExtendableMessageEvent::ExtendableMessageEvent):
* Source/WebCore/workers/service/ServiceWorker.cpp:
(WebCore::ServiceWorker::postMessage):
* Source/WebCore/workers/service/ServiceWorkerClient.cpp:
(WebCore::ServiceWorkerClient::postMessage):

Canonical link: https://commits.webkit.org/255788@main
@webkit-early-warning-system
webkit-early-warning-system force-pushed the eng/Introduce-StructuredSerializeInternal-forStorage-parameter branch from 5806333 to 664fe94 Compare October 20, 2022 16:38
@webkit-commit-queue

Copy link
Copy Markdown
Collaborator

Committed 255788@main (664fe94): https://commits.webkit.org/255788@main

Reviewed commits have been landed. Closing PR #5576 and removing active labels.

@webkit-early-warning-system
webkit-early-warning-system merged commit 664fe94 into WebKit:main Oct 20, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants