Bug 192480

Summary: CSS Painting API code cleanup
Product: WebKit Reporter: Justin Michaud <justin_michaud>
Component: Layout and RenderingAssignee: Justin Michaud <justin_michaud>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, commit-queue, darin, dino, rniwa, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 191443    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Justin Michaud 2018-12-06 16:01:42 PST 
The goal of this is to fix all of Darin's comments on 191443, plus a few other housekeeping things.
Comment 1 Justin Michaud 2018-12-06 16:31:45 PST 
Created attachment 356763 [details]
Patch
Comment 2 WebKit Commit Bot 2018-12-07 13:57:56 PST 
Comment on attachment 356763 [details]
Patch

Clearing flags on attachment: 356763

Committed r238968: <https://trac.webkit.org/changeset/238968>
Comment 3 WebKit Commit Bot 2018-12-07 13:57:58 PST 
All reviewed patches have been landed.  Closing bug.
Comment 4 Radar WebKit Bug Importer 2018-12-07 13:58:26 PST 
<rdar://problem/46564091>
Comment 5 Simon Fraser (smfr) 2018-12-07 15:58:10 PST 
Comment on attachment 356763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=356763&action=review

> Source/WebCore/rendering/style/RenderStyle.cpp:1042
> +    if (changedCustomPaintWatchedProperty(*this, *m_rareNonInheritedData, other, *other.m_rareNonInheritedData))
> +        return true;

I don't think this is the right way to do this. You shouldn't trigger repaint for custom paint from here. You should evaluate inputs to custom paints, run the callback if necessary, then issue a repaint if the output changes.
Comment 6 Justin Michaud 2018-12-07 18:28:35 PST 
https://bugs.webkit.org/show_bug.cgi?id=192322
Comment 7 Justin Michaud 2018-12-07 19:06:52 PST 
Comment on attachment 356763 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=356763&action=review

>> Source/WebCore/rendering/style/RenderStyle.cpp:1042
>> +        return true;
> 
> I don't think this is the right way to do this. You shouldn't trigger repaint for custom paint from here. You should evaluate inputs to custom paints, run the callback if necessary, then issue a repaint if the output changes.

I have a test case that seems to prove you right. If you mutate an a watched property immediately after importing a worklet, then it does not paint correctly.