New issue
Advanced search Search tips

Issue 803753 link

Starred by 2 users
Status: Fixed
Owner:
xidac...@chromium.org
Closed: Feb 2018
Cc:
ikilpatrick@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Singly registered DocumentPaintDefinition may be used before second registration.

Project Member Reported by flackr@chromium.org, Jan 19 2018 
From my reading of the code, from the moment we register the first instance of a DocumentPaintDefinition[1] until the moment we register a second[2] we may use the first paint definition even though it may become invalid when the second is registered.

https://chromium-review.googlesource.com/c/chromium/src/+/869891/6/third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp#68

I think this is incorrect behavior as we shouldn't start using the paint definition until we have seen that both definitions match.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/csspaint/PaintWorkletGlobalScope.cpp?type=cs&q=document_definition_map.Set%5C(.*document_definition&sq=package:chromium&l=293
[2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/csspaint/PaintWorkletGlobalScope.cpp?type=cs&q=document_definition_map.Set%5C(.*kInvalid&sq=package:chromium&l=277
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 8 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d0fabf875756123849ddbeaeb236c310e730009

commit 3d0fabf875756123849ddbeaeb236c310e730009
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Feb 08 16:15:45 2018

[PaintWorklet] Make sure there are two document definition before used

At this moment, it is possible that we start using a singly registered
document paint definition, before the second one gets registered. This
should not happen, because we want to make sure that there are two
global scope where each one holds a document paint definition that matches
each other.

This CL fixes the problem by checking that there are two registered
document paint definition when we try to use them.

Bug:  803753 
Change-Id: Id2801095404d5f51e46ebcdce9e4bc2b38ab0ef8
Reviewed-on: https://chromium-review.googlesource.com/905890
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535397}
[modify] https://crrev.com/3d0fabf875756123849ddbeaeb236c310e730009/third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.cpp
[modify] https://crrev.com/3d0fabf875756123849ddbeaeb236c310e730009/third_party/WebKit/Source/modules/csspaint/CSSPaintImageGeneratorImpl.h
[modify] https://crrev.com/3d0fabf875756123849ddbeaeb236c310e730009/third_party/WebKit/Source/modules/csspaint/PaintWorkletTest.cpp

Comment 2 by xidac...@chromium.org, Feb 8 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment