Bug 185087

Summary: Refactor filter list checking code
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: CompositingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: REOPENED ---    
Severity: Normal CC: ews, fred.wang, mmaxfield, simon.fraser, tonikitoo, webkit-bug-importer, ysuzuki, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Follow-up fix of possible build failure
none
Another follow-up fix of potential build failure
none
Patch none

Description Simon Fraser (smfr) 2018-04-27 13:33:49 PDT
Refactor filter list checking code
Comment 1 Simon Fraser (smfr) 2018-04-27 13:35:24 PDT
Created attachment 339018 [details]
Patch
Comment 2 Simon Fraser (smfr) 2018-04-27 15:02:17 PDT
https://trac.webkit.org/changeset/231112/webkit
Comment 3 Radar WebKit Bug Importer 2018-04-27 15:03:36 PDT
<rdar://problem/39805938>
Comment 4 Frédéric Wang (:fredw) 2018-09-07 07:50:41 PDT
Comment on attachment 339018 [details]
Patch

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

> Source/WebCore/animation/KeyframeEffectReadOnly.h:153
> +    bool checkForMatchingFilterFunctionLists(CSSPropertyID, const std::function<const FilterOperations& (const RenderStyle&)>&) const;

@smfr: I think we should really forward-declare FilterOperations here.
Comment 5 Simon Fraser (smfr) 2018-09-07 11:23:37 PDT
Sure.
Comment 6 Frédéric Wang (:fredw) 2018-09-07 12:05:52 PDT
Created attachment 349173 [details]
Follow-up fix of possible build failure

(In reply to Simon Fraser (smfr) from comment #5)
> Sure.

For the record, I'm found this after getting a build failure with a patch for bug 188043 ; probably because of the unified build thing.
Comment 7 Frédéric Wang (:fredw) 2018-09-09 23:23:20 PDT
smfr: review ping?
Comment 8 Frédéric Wang (:fredw) 2018-09-10 05:01:06 PDT
Committed r235847: <https://trac.webkit.org/changeset/235847>
Comment 9 Frédéric Wang (:fredw) 2018-09-11 03:00:38 PDT
(In reply to Simon Fraser (smfr) from comment #2)
> https://trac.webkit.org/changeset/231112/webkit

Mmh, that commit adds one more change with respect to the patch in Source/WebCore/page/animation/KeyframeAnimation.h that requires similar fixup.
Comment 10 Frédéric Wang (:fredw) 2018-09-11 05:42:51 PDT
Created attachment 349392 [details]
Another follow-up fix of potential build failure
Comment 11 Antonio Gomes 2018-09-11 05:46:22 PDT
Comment on attachment 349392 [details]
Another follow-up fix of potential build failure

rs=me
Comment 12 Frédéric Wang (:fredw) 2018-09-11 05:48:43 PDT
Committed r235894: <https://trac.webkit.org/changeset/235894>
Comment 13 Frédéric Wang (:fredw) 2018-09-12 09:03:01 PDT
Reopening to attach new patch.
Comment 14 Frédéric Wang (:fredw) 2018-09-12 09:03:04 PDT
Created attachment 349552 [details]
Patch
Comment 15 Myles C. Maxfield 2018-09-12 09:57:12 PDT
Comment on attachment 349552 [details]
Patch

We often inline functions for performance, and fonts are used pretty often. Is this a performance regression?
Comment 16 Myles C. Maxfield 2018-09-12 09:58:22 PDT
r=me if this isn’t a performance regression
Comment 17 Frédéric Wang (:fredw) 2018-09-12 11:30:27 PDT
Comment on attachment 349552 [details]
Patch

Mmh, wrong bug sorry...