Skip to content

GitLab

T
tangled
Commit c1f66677 authored by Xida Chen's avatar Xida Chen Committed by Commit Bot
Browse files
Options

[PaintWorklet] Do null check for document paint definition 

In quite a few functions in the CSSPaintImageGeneratorImpl class, we
check whether the document paint definition of a particular name exists
or not, but we never do null check on that definition. This could lead
to crash if the document paint definition is a null ptr.

This CL fixes the problem by doing null check. It also changes one
layout test to execise this code path. The change to the layout test
also fixes the problem that this layout test passes in the browser
that doesn't support paint worklet.

Bug: 802970, 768683
Change-Id: Ia952ad977b63af643410b0973cc8034fac504f9f
Reviewed-on: https://chromium-review.googlesource.com/869891
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530441}
parent ec94c7e1
Hide whitespace changes
Inline Side-by-side
Showing with 48 additions and 26 deletions
third_party/WebKit/LayoutTests/external/wpt/css/css-paint-api/parse-input-arguments-018-ref.html
View file @ c1f66677
<!DOCTYPE html> <!DOCTYPE html>
<html> <html>
<body> <body>
<p>The test result should show only one black rect border. It should not paint <p>This test result should show a rect with black border, where the rect is
any content in the rect because registerPaint will be called twice and the filled with green on the lower right corner. The registerPaint('failureIndicator')
inputArguments will return two different strings, that will throw an exception will be called twice and the inputArguments will return two different strings,
and paint won't be executed.</p> which will throw an exception and the paint function with 'failureIndicator'
should never be called. In other words, there should be no red painted in the result.</p>
<canvas id ="canvas" width="100" height="100" style="border:1px solid black"></canvas> <canvas id ="canvas" width="100" height="100" style="border:1px solid black"></canvas>
<script> <script>
var canvas = document.getElementById('canvas'); var canvas = document.getElementById('canvas');
var context = canvas.getContext("2d"); var context = canvas.getContext("2d");
context.clearRect(0, 0, 100, 100); context.fillStyle = 'green'
context.fillRect(50, 50, 50, 50);
</script> </script>
</body> </body>
</html> </html>
third_party/WebKit/LayoutTests/external/wpt/css/css-paint-api/parse-input-arguments-018.https.html
View file @ c1f66677
...@@ -9,16 +9,17 @@ ...@@ -9,16 +9,17 @@
#canvas-geometry { #canvas-geometry {
border:1px solid black; border:1px solid black;
background-image: paint(geometry); background-image: paint(failureIndicator), paint(geometry);
} }
</style> </style>
<script src="/common/reftest-wait.js"></script> <script src="/common/reftest-wait.js"></script>
<script src="/common/css-paint-tests.js"></script> <script src="/common/css-paint-tests.js"></script>
<body> <body>
<p>The test result should show only one black rect border. It should not paint <p>This test result should show a rect with black border, where the rect is
any content in the rect because registerPaint will be called twice and the filled with green on the lower right corner. The registerPaint('failureIndicator')
inputArguments will return two different strings, that will throw an exception will be called twice and the inputArguments will return two different strings,
and paint won't be executed.</p> which will throw an exception and the paint function with 'failureIndicator'
should never be called. In other words, there should be no red painted in the result.</p>
<div id="canvas-geometry" class="container"></div> <div id="canvas-geometry" class="container"></div>
<script id="code" type="text/worklet"> <script id="code" type="text/worklet">
...@@ -31,7 +32,7 @@ function generateRandString(length) { ...@@ -31,7 +32,7 @@ function generateRandString(length) {
} }
try { try {
registerPaint('geometry', class { registerPaint('failureIndicator', class {
static get inputArguments() { static get inputArguments() {
// This test is testing the case where an exception should be thrown // This test is testing the case where an exception should be thrown
// when two paint definitions with different properties are registered // when two paint definitions with different properties are registered
...@@ -42,14 +43,23 @@ try { ...@@ -42,14 +43,23 @@ try {
var current_str = generateRandString(100); var current_str = generateRandString(100);
return [current_str]; return [current_str];
} }
// The paint function here should never be called because the inputArguments
// will generate two different properties, and that should throw an
// exception.
paint(ctx, geom) { paint(ctx, geom) {
ctx.strokeStyle = 'red'; ctx.fillStyle = 'red';
ctx.lineWidth = 4; ctx.fillRect(0, 0, 50, 50);
ctx.strokeRect(0, 0, geom.width, geom.height);
} }
}); });
} catch(ex) { } catch(ex) {
} }
registerPaint('geometry', class {
paint(ctx, geom) {
ctx.fillStyle = 'green';
ctx.fillRect(50, 50, 50, 50);
}
});
</script> </script>
<script> <script>
......
third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp
View file @ c1f66677
...@@ -60,41 +60,45 @@ bool CSSPaintImageGeneratorImpl::HasDocumentDefinition() const { ...@@ -60,41 +60,45 @@ bool CSSPaintImageGeneratorImpl::HasDocumentDefinition() const {
return paint_worklet_->GetDocumentDefinitionMap().Contains(name_); return paint_worklet_->GetDocumentDefinitionMap().Contains(name_);
} }
bool CSSPaintImageGeneratorImpl::GetValidDocumentDefinition(
DocumentPaintDefinition*& definition) const {
if (!HasDocumentDefinition())
return false;
definition = paint_worklet_->GetDocumentDefinitionMap().at(name_);
return definition != kInvalidDocumentDefinition;
}
const Vector<CSSPropertyID>& const Vector<CSSPropertyID>&
CSSPaintImageGeneratorImpl::NativeInvalidationProperties() const { CSSPaintImageGeneratorImpl::NativeInvalidationProperties() const {
DEFINE_STATIC_LOCAL(Vector<CSSPropertyID>, empty_vector, ()); DEFINE_STATIC_LOCAL(Vector<CSSPropertyID>, empty_vector, ());
if (!HasDocumentDefinition()) DocumentPaintDefinition* definition;
if (!GetValidDocumentDefinition(definition))
return empty_vector; return empty_vector;
DocumentPaintDefinition* definition =
paint_worklet_->GetDocumentDefinitionMap().at(name_);
return definition->NativeInvalidationProperties(); return definition->NativeInvalidationProperties();
} }
const Vector<AtomicString>& const Vector<AtomicString>&
CSSPaintImageGeneratorImpl::CustomInvalidationProperties() const { CSSPaintImageGeneratorImpl::CustomInvalidationProperties() const {
DEFINE_STATIC_LOCAL(Vector<AtomicString>, empty_vector, ()); DEFINE_STATIC_LOCAL(Vector<AtomicString>, empty_vector, ());
if (!HasDocumentDefinition()) DocumentPaintDefinition* definition;
if (!GetValidDocumentDefinition(definition))
return empty_vector; return empty_vector;
DocumentPaintDefinition* definition =
paint_worklet_->GetDocumentDefinitionMap().at(name_);
return definition->CustomInvalidationProperties(); return definition->CustomInvalidationProperties();
} }
bool CSSPaintImageGeneratorImpl::HasAlpha() const { bool CSSPaintImageGeneratorImpl::HasAlpha() const {
if (!HasDocumentDefinition()) DocumentPaintDefinition* definition;
if (!GetValidDocumentDefinition(definition))
return false; return false;
DocumentPaintDefinition* definition =
paint_worklet_->GetDocumentDefinitionMap().at(name_);
return definition->GetPaintRenderingContext2DSettings().alpha(); return definition->GetPaintRenderingContext2DSettings().alpha();
} }
const Vector<CSSSyntaxDescriptor>& const Vector<CSSSyntaxDescriptor>&
CSSPaintImageGeneratorImpl::InputArgumentTypes() const { CSSPaintImageGeneratorImpl::InputArgumentTypes() const {
DEFINE_STATIC_LOCAL(Vector<CSSSyntaxDescriptor>, empty_vector, ()); DEFINE_STATIC_LOCAL(Vector<CSSSyntaxDescriptor>, empty_vector, ());
if (!HasDocumentDefinition()) DocumentPaintDefinition* definition;
if (!GetValidDocumentDefinition(definition))
return empty_vector; return empty_vector;
DocumentPaintDefinition* definition =
paint_worklet_->GetDocumentDefinitionMap().at(name_);
return definition->InputArgumentTypes(); return definition->InputArgumentTypes();
} }
......
third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.h
View file @ c1f66677
...@@ -16,6 +16,7 @@ namespace blink { ...@@ -16,6 +16,7 @@ namespace blink {
class CSSSyntaxDescriptor; class CSSSyntaxDescriptor;
class Document; class Document;
class DocumentPaintDefinition;
class Image; class Image;
class PaintWorklet; class PaintWorklet;
...@@ -47,6 +48,11 @@ class CSSPaintImageGeneratorImpl final : public CSSPaintImageGenerator { ...@@ -47,6 +48,11 @@ class CSSPaintImageGeneratorImpl final : public CSSPaintImageGenerator {
CSSPaintImageGeneratorImpl(PaintWorklet*, const String&); CSSPaintImageGeneratorImpl(PaintWorklet*, const String&);
bool HasDocumentDefinition() const; bool HasDocumentDefinition() const;
// This function first checks whether the document definition with |name_|
// exists or not. If it does exist, the function fetches the document
// definition and checks if it is valid. The function returns true when the
// document definition exists and is valid.
bool GetValidDocumentDefinition(DocumentPaintDefinition*&) const;
Member<Observer> observer_; Member<Observer> observer_;
Member<PaintWorklet> paint_worklet_; Member<PaintWorklet> paint_worklet_;
......
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