Properly test !CSSPaintAPIArguments in CSSPaintValueTest
Previously all the tests assumed that CSSPaintAPIArguments was on, which
was covering up a bug where OffMainThreadCSSPaint would create a
deferred image even though the generator wasn't ready (because
ParseInputArguments would pass due to !CSSPaintAPIArguments). This CL
fixes the tests to actually test all combinations, and fixes the bug by
extracting the generator-ready check outside of ParseInputArguments.
Bug: None
Change-Id: I6789cf24b7efa6035a88990082a857f24ccafbb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1689953
Reviewed-by: Xida Chen <xidachen@chromium.org>
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675214}
diff --git a/third_party/blink/renderer/core/css/css_paint_value.cc b/third_party/blink/renderer/core/css/css_paint_value.cc
index 7b57a8e..5aeda58 100644
--- a/third_party/blink/renderer/core/css/css_paint_value.cc
+++ b/third_party/blink/renderer/core/css/css_paint_value.cc
@@ -63,6 +63,12 @@
GetName(), document, paint_image_generator_observer_);
}
+ // If the generator isn't ready yet, we have nothing to paint. Our
+ // |paint_image_generator_observer_| will cause us to be called again once the
+ // generator is ready.
+ if (!generator_->IsImageGeneratorReady())
+ return nullptr;
+
if (!ParseInputArguments(document))
return nullptr;
@@ -119,9 +125,7 @@
!RuntimeEnabledFeatures::CSSPaintAPIArgumentsEnabled())
return true;
- if (!generator_->IsImageGeneratorReady())
- return false;
-
+ DCHECK(generator_->IsImageGeneratorReady());
const Vector<CSSSyntaxDescriptor>& input_argument_types =
generator_->InputArgumentTypes();
if (argument_variable_data_.size() != input_argument_types.size()) {
diff --git a/third_party/blink/renderer/core/css/css_paint_value_test.cc b/third_party/blink/renderer/core/css/css_paint_value_test.cc
index 30919feb7..d412d5f 100644
--- a/third_party/blink/renderer/core/css/css_paint_value_test.cc
+++ b/third_party/blink/renderer/core/css/css_paint_value_test.cc
@@ -29,16 +29,29 @@
namespace blink {
namespace {
-class CSSPaintValueTest : public RenderingTest,
- public ::testing::WithParamInterface<bool> {
- public:
- CSSPaintValueTest() : scoped_off_main_thread_css_paint_(GetParam()) {}
-
- private:
- ScopedOffMainThreadCSSPaintForTest scoped_off_main_thread_css_paint_;
+enum {
+ kCSSPaintAPIArguments = 1 << 0,
+ kOffMainThreadCSSPaint = 1 << 1,
};
-INSTANTIATE_TEST_SUITE_P(, CSSPaintValueTest, Values(false, true));
+class CSSPaintValueTest : public RenderingTest,
+ public ::testing::WithParamInterface<unsigned>,
+ private ScopedCSSPaintAPIArgumentsForTest,
+ private ScopedOffMainThreadCSSPaintForTest {
+ public:
+ CSSPaintValueTest()
+ : ScopedCSSPaintAPIArgumentsForTest(GetParam() & kCSSPaintAPIArguments),
+ ScopedOffMainThreadCSSPaintForTest(GetParam() &
+ kOffMainThreadCSSPaint) {}
+};
+
+INSTANTIATE_TEST_SUITE_P(,
+ CSSPaintValueTest,
+ Values(0,
+ kCSSPaintAPIArguments,
+ kOffMainThreadCSSPaint,
+ kCSSPaintAPIArguments |
+ kOffMainThreadCSSPaint));
class MockCSSPaintImageGenerator : public CSSPaintImageGenerator {
public:
@@ -93,14 +106,6 @@
ProvideOverrideGenerator);
const FloatSize target_size(100, 100);
- if (RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled()) {
- // In off-thread CSS Paint, the actual paint call is deferred.
- EXPECT_CALL(*mock_generator, Paint(_, _, _)).Times(0);
- } else {
- ON_CALL(*mock_generator, Paint(_, _, _))
- .WillByDefault(
- Return(PaintGeneratedImage::Create(nullptr, target_size)));
- }
SetBodyInnerHTML(R"HTML(
<div id="target"></div>
@@ -111,12 +116,22 @@
auto* ident = MakeGarbageCollected<CSSCustomIdentValue>("testpainter");
CSSPaintValue* paint_value = MakeGarbageCollected<CSSPaintValue>(ident);
- // Initially the generator is not ready, so GetImage should fail.
+ // Initially the generator is not ready, so GetImage should fail (and no paint
+ // should happen).
+ EXPECT_CALL(*mock_generator, Paint(_, _, _)).Times(0);
EXPECT_FALSE(
paint_value->GetImage(*target, GetDocument(), style, target_size));
// Now mark the generator as ready - GetImage should then succeed.
- EXPECT_CALL(*mock_generator, IsImageGeneratorReady()).WillOnce(Return(true));
+ ON_CALL(*mock_generator, IsImageGeneratorReady()).WillByDefault(Return(true));
+ // In off-thread CSS Paint, the actual paint call is deferred and so will
+ // never happen.
+ if (!RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled()) {
+ EXPECT_CALL(*mock_generator, Paint(_, _, _))
+ .WillRepeatedly(
+ Return(PaintGeneratedImage::Create(nullptr, target_size)));
+ }
+
EXPECT_TRUE(
paint_value->GetImage(*target, GetDocument(), style, target_size));
}
