New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 721768 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Using CSSToLengthConversionData on Workers may be dangerous

Project Member Reported by fs...@chromium.org, May 12 2017

Issue description

FilterOperationResolver uses it for filter parsing on Worker's OffscreenCanvas.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/resolver/FilterOperationResolver.cpp?type=cs&q=CreateOffscreenFilterOperations&sq=package:chromium&l=208

There may be other cases, since it's not uncommon that specs reference CSS parsing.

As @esprehn pointed out:
I think the current code is currently okay because:
- initialStyle() returns a const reference
- CSSToLengthConversionData takes a raw ptr to a ComputedStyle and doesn't store it in a RefPtr.

But from a system design perspective these classes can only be used from main, for example we might lazy allocate the nested fields inside it, or something inside CSSToLengthConversionData could call a function that refs the style.

this should be currently safe unless CSSToLengthConversionData ever decides to start changing something global. At this point, I'd say that we probably need to fork CSSToLengthConversionData (and CHECK the proper threads) and keep a thread-safe version of it. Design-wise, I don't know how to make this explicit, except for having what we currently do: have tests on separate threads and hope that when the function becomes unsafe there will be a new CHECK that will fail.

(This bug has been moved from  http://crbug.com/703908 , since that issue is closed)
 
Project Member

Comment 1 by sheriffbot@chromium.org, May 14 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Assigned (was: Untriaged)

Comment 3 by fs...@chromium.org, May 24 2018

Status: Fixed (was: Assigned)

Sign in to add a comment