[prev in list] [next in list] [prev in thread] [next in thread]
List: webkit-dev
Subject: Re: [webkit-dev] Implementing OffscreenCanvas
From: Ryosuke Niwa <rniwa () webkit ! org>
Date: 2019-10-10 19:57:36
Message-ID: CABNRm60jYWMhJ26=jQefaGCe5j=W8vkxO-ukTqjdhqPjJ7jrvQ () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hi Chris,
I'm excited that you're working on OffscreenCanvas because I think it would
be a valuable feature, and I'm confident we can come up with a strategy to
limit its privacy & security risk as we see fit.
However, many of your patches seem to ignore the fact most of WebCore
objects aren't thread safe. For example, CSS parser and the entire CSS
object model aren't designed to used in non-main thread. Regardless of how
ready Linux ports might be, we can't land or enable this feature in WebKit
until all thread safety issues are sorted out.
Unfortunately, I can't make a time commitment to review & find every thread
safety issue in your patches. Please work with relevant experts and go over
your code changes.
For example, it's never safe to an object that's RefCounted in multiple
threads because RefCounted isn't thread safe. One would have to use
ThreadSafeRefCounted. It's never safe to use AtomString from one another in
another because AtomString has a pool of strings per thread. For that
matter, it's never safe to use a single String object from two or more
threads because String itself is RefCounted and isn't thread safe. It's not
not okay to do readonly access to basic container types like Vector,
HashMap, etc... because they don't guarantee atomic update of internal data
structures and doing so can result in UAF.
I think the hardest part of this project is validating that enabling this
feature wouldn't introduce dozens of new thread safety issues and thereby
security vulnerabilities.
- R. Niwa
On Thu, Oct 10, 2019 at 4:23 AM Chris Lord <clord@igalia.com> wrote:
>
> I've spent the last month or so 'finishing' the implementation of
> OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].
> OffscreenCanvas is an API for being able to use canvas drawing without a
> visible canvas, and from within Workers. It's supported by Blink and has
> partial support in Gecko.
>
> It's at the point now where I'd consider it a finished draft - it is
> almost fully implemented and passes the majority of relevant tests in a
> debug build without crashing, but has some areas that need completion on
> other platforms (async drawing on non-Linux) and some missing parts (Web
> Inspector, ImageBitmapRenderingContext). It almost certainly needs
> reworking in places.
>
> My work is on GitHub[3] - I'd like to solicit reviews and comment. Some
> of the bugs hanging off [2] have patches that need review and I think
> are near ready to being landable as the foundation of this work. It is
> broadly split up like so:
>
> - Refactor to move functionality from HTMLCanvasElement to CanvasBase
> - Refactor to not unnecessarily require HTMLCanvasElement in places
> - Implement OffscreenCanvas functionality
> - Make font loading/styling usable from a Worker and without a Document
> - Implement AnimationFrameProvider on DedicatedWorkerGlobalScope
> - Implement asynchronous drawing updates on placeholder canvases
>
> I expect the font-related stuff to be the most contentious, and my
> AnimationFrameProvider implementation may be too trivial (but might be
> ok for a first go?)
>
> All feedback appreciated. Best regards,
>
> Chris
>
> [1]
>
> https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface
> [2] https://bugs.webkit.org/show_bug.cgi?id=183720
> [3] https://github.com/Cwiiis/webkit/tree/offscreen-canvas
> _______________________________________________
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
[Attachment #5 (text/html)]
<div dir="ltr"><div dir="ltr"><div>Hi Chris,</div><div><br></div><div>I'm excited \
that you're working on OffscreenCanvas because I think it would be a valuable \
feature, and I'm confident we can come up with a strategy to limit its privacy \
& security risk as we see fit.</div><div><br></div><div>However, many of your \
patches seem to ignore the fact most of WebCore objects aren't thread safe. For \
example, CSS parser and the entire CSS object model aren't designed to used in \
non-main thread. Regardless of how ready Linux ports might be, we can't land or \
enable this feature in WebKit until all thread safety issues are sorted \
out.</div><div><br></div><div>Unfortunately, I can't make a time commitment to \
review & find every thread safety issue in your patches. Please work with \
relevant experts and go over your code changes.</div><div><br></div><div>For example, \
it's never safe to an object that's RefCounted in multiple threads because \
RefCounted isn't thread safe. One would have to use ThreadSafeRefCounted. \
It's never safe to use AtomString from one another in another because AtomString \
has a pool of strings per thread. For that matter, it's never safe to use a \
single String object from two or more threads because String itself is RefCounted and \
isn't thread safe. It's not not okay to do readonly access to basic container \
types like Vector, HashMap, etc... because they don't guarantee atomic update of \
internal data structures and doing so can result in UAF.</div><div><br></div><div>I \
think the hardest part of this project is validating that enabling this feature \
wouldn't introduce dozens of new thread safety issues and thereby security \
vulnerabilities.</div><div><br></div><div>- R. Niwa</div><div><br></div><div \
dir="ltr">On Thu, Oct 10, 2019 at 4:23 AM Chris Lord <<a \
href="mailto:clord@igalia.com">clord@igalia.com</a>> wrote:<br></div><div \
class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><br>
I've spent the last month or so 'finishing' the implementation of<br>
OffscreenCanvas[1], based on Žan Doberšek's work from a year ago[2].<br>
OffscreenCanvas is an API for being able to use canvas drawing without a<br>
visible canvas, and from within Workers. It's supported by Blink and has<br>
partial support in Gecko.<br>
<br>
It's at the point now where I'd consider it a finished draft - it is<br>
almost fully implemented and passes the majority of relevant tests in a<br>
debug build without crashing, but has some areas that need completion on<br>
other platforms (async drawing on non-Linux) and some missing parts (Web<br>
Inspector, ImageBitmapRenderingContext). It almost certainly needs<br>
reworking in places.<br>
<br>
My work is on GitHub[3] - I'd like to solicit reviews and comment. Some<br>
of the bugs hanging off [2] have patches that need review and I think<br>
are near ready to being landable as the foundation of this work. It is<br>
broadly split up like so:<br>
<br>
- Refactor to move functionality from HTMLCanvasElement to CanvasBase<br>
- Refactor to not unnecessarily require HTMLCanvasElement in places<br>
- Implement OffscreenCanvas functionality<br>
- Make font loading/styling usable from a Worker and without a Document<br>
- Implement AnimationFrameProvider on DedicatedWorkerGlobalScope<br>
- Implement asynchronous drawing updates on placeholder canvases<br>
<br>
I expect the font-related stuff to be the most contentious, and my<br>
AnimationFrameProvider implementation may be too trivial (but might be<br>
ok for a first go?)<br>
<br>
All feedback appreciated. Best regards,<br>
<br>
Chris<br>
<br>
[1]<br>
<a href="https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface" \
rel="noreferrer" target="_blank">https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface</a><br>
[2] <a href="https://bugs.webkit.org/show_bug.cgi?id=183720" rel="noreferrer" \
target="_blank">https://bugs.webkit.org/show_bug.cgi?id=183720</a><br> [3] <a \
href="https://github.com/Cwiiis/webkit/tree/offscreen-canvas" rel="noreferrer" \
target="_blank">https://github.com/Cwiiis/webkit/tree/offscreen-canvas</a><br> \
_______________________________________________<br> webkit-dev mailing list<br>
<a href="mailto:webkit-dev@lists.webkit.org" \
target="_blank">webkit-dev@lists.webkit.org</a><br> <a \
href="https://lists.webkit.org/mailman/listinfo/webkit-dev" rel="noreferrer" \
target="_blank">https://lists.webkit.org/mailman/listinfo/webkit-dev</a><br> \
</blockquote></div></div></div>
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic