Bug 202394

Summary: ImageBitmap should be serializable
Product: WebKit Reporter: Chris Lord <clord>
Component: BindingsAssignee: Chris Lord <clord>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alecflett, beidson, cdumez, commit-queue, ews-watchlist, ggaren, jsbell, mark.lam, sabouhallawa, simon.fraser, webkit-bug-importer, ysuzuki, zan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 183720    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Chris Lord
Reported 2019-10-01 02:16:18 PDT
Currently an ImageBitmap can be sent in sendMessage if it's transferred, but it can't be copied. As it is serializable in the spec[1], this should work. [1] https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html#imagebitmap
Attachments
Patch (9.19 KB, patch)
2019-10-01 03:45 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Patch (11.12 KB, patch)
2019-10-04 03:00 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Patch (11.28 KB, patch)
2019-10-04 04:44 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Patch (11.57 KB, patch)
2019-10-04 07:04 PDT, Chris Lord 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Chris Lord
Comment 1 2019-10-01 03:45:44 PDT
Created attachment 379887 [details] Patch
Chris Lord
Comment 2 2019-10-04 03:00:11 PDT
Created attachment 380208 [details] Patch
Zan Dobersek
Comment 3 2019-10-04 04:08:17 PDT
Comment on attachment 380208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380208&action=review > Source/WebCore/bindings/js/SerializedScriptValue.cpp:915 > + auto buffer = imageBitmap.buffer(); It's usually preferable to use `auto*` to explicitly denote that a pointer of some sorts is being returned. Given the pointer, what should happen if it is null? It can be, at least theoretically. Should a validation error be reported in this case. or should serialization still succeed? > Source/WebCore/bindings/js/SerializedScriptValue.cpp:927 > + write(static_cast<int32_t>(imageBitmap.originClean())); origin-clean being a boolean, I think using uint8_t type here and in the deserialization makes more sense, if possible. > Source/WebCore/bindings/js/SerializedScriptValue.cpp:932 > + write(arrayBuffer->byteLength()); Nit: perhaps this should be explicitly casted to uint32_t to make the intended type clear, even when `unsigned` as returned by byteLength() is an equivalent. > Source/WebCore/bindings/js/SerializedScriptValue.cpp:2769 > + read(originClean); > + read(logicalWidth); > + read(logicalHeight); > + read(resolutionScale); These returns should also probable be checked, perhaps in the same if-statement to keep things compact.
Chris Lord
Comment 4 2019-10-04 04:44:45 PDT
Created attachment 380211 [details] Patch
Zan Dobersek
Comment 5 2019-10-04 06:24:49 PDT
Comment on attachment 380211 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380211&action=review Patch looks good, just some additional checks required to cover the odd cases. > Source/WebCore/bindings/js/SerializedScriptValue.cpp:923 > + auto imageData = buffer->getPremultipliedImageData(IntRect(0, 0, logicalSize.width(), logicalSize.height())); Looked it up only now -- getPremultipliedImageData() can possibly return null, so we have to null-check around that as well. > Source/WebCore/bindings/js/SerializedScriptValue.cpp:2776 > + auto imageData = Uint8ClampedArray::tryCreate(WTFMove(arrayBuffer), 0, arrayBuffer->byteLength()); tryCreate() possibly returns null, so we have to null-check it. > Source/WebCore/bindings/js/SerializedScriptValue.cpp:2777 > + auto buffer = ImageBuffer::create(FloatSize(logicalWidth, logicalHeight), Unaccelerated, resolutionScale); Similar here -- ImageBuffer::create() can return null if internally the initialization doesn't succeed (e.g. memory allocation or some other step fails).
Chris Lord
Comment 6 2019-10-04 07:04:56 PDT
Created attachment 380218 [details] Patch
WebKit Commit Bot
Comment 7 2019-10-04 07:53:24 PDT
Comment on attachment 380218 [details] Patch Clearing flags on attachment: 380218 Committed r250721: <https://trac.webkit.org/changeset/250721>
WebKit Commit Bot
Comment 8 2019-10-04 07:53:26 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-10-04 07:54:14 PDT
<rdar://problem/55982648>
Note You need to log in before you can comment on or make changes to this bug.