Commit 87a2d9f8 authored by Adam Raine's avatar Adam Raine Committed by Commit Bot

Fall back to main thread for non-compositable paint worklet

The worklet thread currently does not support all data types for the
native and custom properties in CSSPaintValue::GetImage.  To resolve
this, we will fall back to using the main thread for any CSSPaintValue
that contains an unsupported property type.  This fall back feature
makes use of the extra set of global scopes created on the main thread
when CSSPaint is running off main thread.

A unit test was also added for this functionality.

Bug: 948761
Change-Id: I65dea64b1d6c4f6e8a5ea3319953bcf8ebe68fcd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1667751
Commit-Queue: Adam Raine <asraine@google.com>
Reviewed-by: default avatarAnders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Reviewed-by: default avatarXida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672554}
parent 3471e6f7
......@@ -20,7 +20,8 @@ namespace blink {
CSSPaintValue::CSSPaintValue(CSSCustomIdentValue* name)
: CSSImageGeneratorValue(kPaintClass),
name_(name),
paint_image_generator_observer_(MakeGarbageCollected<Observer>(this)) {}
paint_image_generator_observer_(MakeGarbageCollected<Observer>(this)),
paint_off_thread_(true) {}
CSSPaintValue::CSSPaintValue(
CSSCustomIdentValue* name,
......@@ -68,25 +69,31 @@ scoped_refptr<Image> CSSPaintValue::GetImage(
// For Off-Thread PaintWorklet, we just collect the necessary inputs together
// and defer the actual JavaScript call until much later (during cc Raster).
if (RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled()) {
// TODO(crbug.com/946515): Break dependency on LayoutObject.
const LayoutObject& layout_object =
static_cast<const LayoutObject&>(client);
Vector<CSSPropertyID> native_properties =
generator_->NativeInvalidationProperties();
Vector<AtomicString> custom_properties =
generator_->CustomInvalidationProperties();
float zoom = layout_object.StyleRef().EffectiveZoom();
PaintWorkletStylePropertyMap::CrossThreadData style_data =
PaintWorkletStylePropertyMap::BuildCrossThreadData(
document, style, layout_object.GetNode(), native_properties,
custom_properties);
Vector<std::unique_ptr<CrossThreadStyleValue>> cross_thread_input_arguments;
BuildInputArgumentValues(cross_thread_input_arguments);
scoped_refptr<PaintWorkletInput> input =
base::MakeRefCounted<PaintWorkletInput>(
GetName(), target_size, zoom, generator_->WorkletId(),
std::move(style_data), std::move(cross_thread_input_arguments));
return PaintWorkletDeferredImage::Create(std::move(input), target_size);
if (paint_off_thread_) {
// TODO(crbug.com/946515): Break dependency on LayoutObject.
const LayoutObject& layout_object =
static_cast<const LayoutObject&>(client);
Vector<CSSPropertyID> native_properties =
generator_->NativeInvalidationProperties();
Vector<AtomicString> custom_properties =
generator_->CustomInvalidationProperties();
float zoom = layout_object.StyleRef().EffectiveZoom();
auto style_data = PaintWorkletStylePropertyMap::BuildCrossThreadData(
document, style, layout_object.GetNode(), native_properties,
custom_properties);
paint_off_thread_ = style_data.has_value();
if (paint_off_thread_) {
Vector<std::unique_ptr<CrossThreadStyleValue>>
cross_thread_input_arguments;
BuildInputArgumentValues(cross_thread_input_arguments);
scoped_refptr<PaintWorkletInput> input =
base::MakeRefCounted<PaintWorkletInput>(
GetName(), target_size, zoom, generator_->WorkletId(),
std::move(style_data.value()),
std::move(cross_thread_input_arguments));
return PaintWorkletDeferredImage::Create(std::move(input), target_size);
}
}
}
return generator_->Paint(client, target_size, parsed_input_arguments_);
......
......@@ -93,6 +93,7 @@ class CORE_EXPORT CSSPaintValue : public CSSImageGeneratorValue {
Member<Observer> paint_image_generator_observer_;
Member<CSSStyleValueVector> parsed_input_arguments_;
Vector<scoped_refptr<CSSVariableData>> argument_variable_data_;
bool paint_off_thread_;
};
template <>
......
......@@ -54,7 +54,7 @@ class PaintWorkletStylePropertyMapIterationSource final
const HeapVector<PaintWorkletStylePropertyMap::StylePropertyMapEntry> values_;
};
void BuildNativeValues(const ComputedStyle& style,
bool BuildNativeValues(const ComputedStyle& style,
Node* styled_node,
const Vector<CSSPropertyID>& native_properties,
PaintWorkletStylePropertyMap::CrossThreadData& data) {
......@@ -70,14 +70,17 @@ void BuildNativeValues(const ComputedStyle& style,
.CrossThreadStyleValueFromComputedStyle(
style, /* layout_object */ nullptr, styled_node,
/* allow_visited_style */ false);
if (value->GetType() == CrossThreadStyleValue::StyleValueType::kUnknownType)
return false;
String key = CSSProperty::Get(property_id).GetPropertyNameString();
if (!key.IsSafeToSendToAnotherThread())
key = key.IsolatedCopy();
data.Set(key, std::move(value));
}
return true;
}
void BuildCustomValues(const Document& document,
bool BuildCustomValues(const Document& document,
const ComputedStyle& style,
Node* styled_node,
const Vector<AtomicString>& custom_properties,
......@@ -89,18 +92,21 @@ void BuildCustomValues(const Document& document,
ref.GetProperty().CrossThreadStyleValueFromComputedStyle(
style, /* layout_object */ nullptr, styled_node,
/* allow_visited_style */ false);
if (value->GetType() == CrossThreadStyleValue::StyleValueType::kUnknownType)
return false;
// Ensure that the String can be safely passed cross threads.
String key = property_name.GetString();
if (!key.IsSafeToSendToAnotherThread())
key = key.IsolatedCopy();
data.Set(key, std::move(value));
}
return true;
}
} // namespace
// static
PaintWorkletStylePropertyMap::CrossThreadData
base::Optional<PaintWorkletStylePropertyMap::CrossThreadData>
PaintWorkletStylePropertyMap::BuildCrossThreadData(
const Document& document,
const ComputedStyle& style,
......@@ -111,8 +117,10 @@ PaintWorkletStylePropertyMap::BuildCrossThreadData(
PaintWorkletStylePropertyMap::CrossThreadData data;
data.ReserveCapacityForSize(native_properties.size() +
custom_properties.size());
BuildNativeValues(style, styled_node, native_properties, data);
BuildCustomValues(document, style, styled_node, custom_properties, data);
if (!BuildNativeValues(style, styled_node, native_properties, data))
return base::nullopt;
if (!BuildCustomValues(document, style, styled_node, custom_properties, data))
return base::nullopt;
return data;
}
......
......@@ -31,7 +31,7 @@ class CORE_EXPORT PaintWorkletStylePropertyMap
HashMap<String, std::unique_ptr<CrossThreadStyleValue>>;
// Build the data that will be passed to the worklet thread to construct a
// style map. Should be called on the main thread only.
static CrossThreadData BuildCrossThreadData(
static base::Optional<CrossThreadData> BuildCrossThreadData(
const Document&,
const ComputedStyle&,
Node* styled_node,
......
......@@ -97,53 +97,34 @@ class PaintWorkletStylePropertyMapTest : public PageTestBase {
exception_state.ClearException();
}
void CheckStyleMap(base::WaitableEvent* waitable_event,
scoped_refptr<PaintWorkletInput> input) {
void CheckCrossThreadData(base::WaitableEvent* waitable_event,
scoped_refptr<PaintWorkletInput> input) {
DCHECK(!IsMainThread());
PaintWorkletStylePropertyMap* map =
MakeGarbageCollected<PaintWorkletStylePropertyMap>(
input->StyleMapData());
DCHECK(map);
DummyExceptionStateForTesting exception_state;
CheckNativeProperties(map, exception_state);
CheckCustomProperties(map, exception_state);
const PaintWorkletStylePropertyMap::CrossThreadData& data =
map->StyleMapDataForTest();
EXPECT_EQ(data.size(), 8u);
EXPECT_EQ(data.at("color")->ToCSSStyleValue()->CSSText(), "rgb(0, 255, 0)");
EXPECT_EQ(data.at("color")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kUnknownType);
EXPECT_EQ(data.at("align-items")->ToCSSStyleValue()->CSSText(), "normal");
EXPECT_EQ(data.at("align-items")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kUnknownType);
EXPECT_EQ(
static_cast<CSSKeywordValue*>(data.at("display")->ToCSSStyleValue())
->value(),
"block");
EXPECT_EQ(data.at("display")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kKeywordType);
EXPECT_EQ(data.at("--foo")->ToCSSStyleValue()->CSSText(), "PaintWorklet");
EXPECT_EQ(data.size(), 4u);
EXPECT_EQ(data.at("--foo")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kUnknownType);
EXPECT_EQ(data.at("--bar")->ToCSSStyleValue()->CSSText(), "");
EXPECT_EQ(data.at("--bar")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kUnknownType);
EXPECT_EQ(data.at("--keyword")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kKeywordType);
EXPECT_EQ(
To<CSSKeywordValue>(data.at("--keyword")->ToCSSStyleValue())->value(),
"test");
EXPECT_EQ(data.at("--x")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kUnitType);
EXPECT_EQ(To<CSSUnitValue>(data.at("--x")->ToCSSStyleValue())->value(), 10);
EXPECT_EQ(To<CSSUnitValue>(data.at("--x")->ToCSSStyleValue())->unit(),
EXPECT_EQ(To<CSSUnitValue>(data.at("--foo")->ToCSSStyleValue())->value(),
134);
EXPECT_EQ(To<CSSUnitValue>(data.at("--foo")->ToCSSStyleValue())->unit(),
"px");
EXPECT_EQ(data.at("--y")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kUnknownType);
EXPECT_EQ(data.at("--y")->ToCSSStyleValue()->CSSText(), "rgb(0, 255, 0)");
EXPECT_EQ(data.at("--bar")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kUnitType);
EXPECT_EQ(To<CSSUnitValue>(data.at("--bar")->ToCSSStyleValue())->value(),
42);
EXPECT_EQ(data.at("--loo")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kKeywordType);
EXPECT_EQ(To<CSSKeywordValue>(data.at("--loo")->ToCSSStyleValue())->value(),
"test");
EXPECT_EQ(data.at("display")->ToCSSStyleValue()->GetType(),
CSSStyleValue::StyleValueType::kKeywordType);
waitable_event->Signal();
}
......@@ -151,44 +132,28 @@ class PaintWorkletStylePropertyMapTest : public PageTestBase {
std::unique_ptr<blink::Thread> thread_;
};
// This test ensures that Blink::PaintWorkletInput can be safely passed cross
// threads and no information is lost.
TEST_F(PaintWorkletStylePropertyMapTest, PassValuesCrossThread) {
Vector<CSSPropertyID> native_properties({CSSPropertyID::kColor,
CSSPropertyID::kAlignItems,
CSSPropertyID::kDisplay});
GetDocument().documentElement()->style()->setProperty(
&GetDocument(), "color", "rgb(0, 255, 0)", "", ASSERT_NO_EXCEPTION);
GetDocument().documentElement()->style()->setProperty(
&GetDocument(), "display", "block", "", ASSERT_NO_EXCEPTION);
Vector<AtomicString> custom_properties(
{"--foo", "--bar", "--keyword", "--x", "--y"});
css_test_helpers::RegisterProperty(GetDocument(), "--keyword", "test", "test",
TEST_F(PaintWorkletStylePropertyMapTest, CreateSupportedCrossThreadData) {
Vector<CSSPropertyID> native_properties({CSSPropertyID::kDisplay});
Vector<AtomicString> custom_properties({"--foo", "--bar", "--loo"});
css_test_helpers::RegisterProperty(GetDocument(), "--foo", "<length>",
"134px", false);
css_test_helpers::RegisterProperty(GetDocument(), "--bar", "<number>", "42",
false);
css_test_helpers::RegisterProperty(GetDocument(), "--x", "<length>", "42px",
css_test_helpers::RegisterProperty(GetDocument(), "--loo", "test", "test",
false);
css_test_helpers::RegisterProperty(GetDocument(), "--y", "<color>",
"rgb(0, 0, 0)", false);
GetDocument().documentElement()->style()->setProperty(
&GetDocument(), "--foo", "PaintWorklet", "", ASSERT_NO_EXCEPTION);
GetDocument().documentElement()->style()->setProperty(
&GetDocument(), "--keyword", "test", "", ASSERT_NO_EXCEPTION);
GetDocument().documentElement()->style()->setProperty(
&GetDocument(), "--x", "10px", "", ASSERT_NO_EXCEPTION);
GetDocument().documentElement()->style()->setProperty(
&GetDocument(), "--y", "rgb(0, 255, 0)", "", ASSERT_NO_EXCEPTION);
UpdateAllLifecyclePhasesForTest();
Node* node = PageNode();
Vector<std::unique_ptr<CrossThreadStyleValue>> input_arguments;
PaintWorkletStylePropertyMap::CrossThreadData data =
PaintWorkletStylePropertyMap::BuildCrossThreadData(
GetDocument(), node->ComputedStyleRef(), node, native_properties,
custom_properties);
auto data = PaintWorkletStylePropertyMap::BuildCrossThreadData(
GetDocument(), node->ComputedStyleRef(), node, native_properties,
custom_properties);
EXPECT_TRUE(data.has_value());
scoped_refptr<PaintWorkletInput> input =
base::MakeRefCounted<PaintWorkletInput>("test", FloatSize(100, 100), 1.0f,
1, std::move(data),
1, std::move(data.value()),
std::move(input_arguments));
DCHECK(input);
......@@ -197,13 +162,44 @@ TEST_F(PaintWorkletStylePropertyMapTest, PassValuesCrossThread) {
base::WaitableEvent waitable_event;
PostCrossThreadTask(
*thread_->GetTaskRunner(), FROM_HERE,
CrossThreadBindOnce(&PaintWorkletStylePropertyMapTest::CheckStyleMap,
CrossThreadUnretained(this),
CrossThreadUnretained(&waitable_event),
std::move(input)));
CrossThreadBindOnce(
&PaintWorkletStylePropertyMapTest::CheckCrossThreadData,
CrossThreadUnretained(this), CrossThreadUnretained(&waitable_event),
std::move(input)));
waitable_event.Wait();
ShutDownThread();
}
TEST_F(PaintWorkletStylePropertyMapTest, UnsupportedCrossThreadData) {
Vector<CSSPropertyID> native_properties1;
Vector<AtomicString> custom_properties1({"--foo", "--bar", "--loo"});
css_test_helpers::RegisterProperty(GetDocument(), "--foo", "<color>",
"rgb(0, 0, 0)", false);
css_test_helpers::RegisterProperty(GetDocument(), "--bar", "<number>", "42",
false);
css_test_helpers::RegisterProperty(GetDocument(), "--loo", "test", "test",
false);
UpdateAllLifecyclePhasesForTest();
Node* node = PageNode();
Vector<std::unique_ptr<CrossThreadStyleValue>> input_arguments;
auto data1 = PaintWorkletStylePropertyMap::BuildCrossThreadData(
GetDocument(), node->ComputedStyleRef(), node, native_properties1,
custom_properties1);
EXPECT_FALSE(data1.has_value());
Vector<CSSPropertyID> native_properties2(
{CSSPropertyID::kDisplay, CSSPropertyID::kColor});
Vector<AtomicString> custom_properties2;
auto data2 = PaintWorkletStylePropertyMap::BuildCrossThreadData(
GetDocument(), node->ComputedStyleRef(), node, native_properties2,
custom_properties2);
EXPECT_FALSE(data2.has_value());
}
} // namespace blink
......@@ -101,8 +101,6 @@ scoped_refptr<Image> PaintWorklet::Paint(const String& name,
const ImageResourceObserver& observer,
const FloatSize& container_size,
const CSSStyleValueVector* data) {
DCHECK(!RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled());
if (!document_definition_map_.Contains(name))
return nullptr;
......@@ -234,10 +232,13 @@ bool PaintWorklet::NeedsToCreateGlobalScope() {
WorkletGlobalScopeProxy* PaintWorklet::CreateGlobalScope() {
DCHECK(NeedsToCreateGlobalScope());
// It does not matter which thread creates its global scopes first, here we
// choose to have the worker thread global scopes created first.
// The main thread global scopes must be created first so that they are at the
// front of the vector. This is because SelectNewGlobalScope selects global
// scopes from the beginning of the vector. If this code is changed to put
// the main thread global scopes at the end, then SelectNewGlobalScope must
// also be changed.
if (!RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled() ||
GetNumberOfGlobalScopes() >= kNumGlobalScopesPerThread) {
GetNumberOfGlobalScopes() < kNumGlobalScopesPerThread) {
return MakeGarbageCollected<PaintWorkletGlobalScopeProxy>(
To<Document>(GetExecutionContext())->GetFrame(), ModuleResponsesMap(),
GetNumberOfGlobalScopes() + 1);
......
......@@ -4431,17 +4431,6 @@ crbug.com/626703 [ Win ] external/wpt/css/css-writing-modes/box-offsets-rel-pos-
crbug.com/957457 virtual/threaded/external/wpt/css/css-paint-api/background-image-tiled.https.html [ Crash ]
crbug.com/957457 virtual/threaded/external/wpt/css/css-paint-api/geometry-background-image-tiled-001.https.html [ Crash ]
crbug.com/957457 virtual/threaded/external/wpt/css/css-paint-api/invalid-image-pending-script.https.html [ Crash ]
crbug.com/957457 virtual/threaded/external/wpt/css/css-paint-api/paint2d-image.https.html [ Failure Timeout ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/registered-property-interpolation-004.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/registered-property-interpolation-010.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/registered-property-stylemap.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/registered-property-value-003.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/registered-property-value-007.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/registered-property-value-009.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/registered-property-value-018.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/style-background-image.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/style-before-pseudo.https.html [ Failure ]
crbug.com/957459 virtual/threaded/external/wpt/css/css-paint-api/style-first-letter-pseudo.https.html [ Failure ]
# Shared memory growth temporarily disabled.
crbug.com/951795 external/wpt/wasm/jsapi/memory/grow.any.html [ Pass Failure ]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment