Bug 185087 - Refactor filter list checking code
Summary: Refactor filter list checking code
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Compositing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-04-27 13:33 PDT by Simon Fraser (smfr)
Modified: 2019-06-16 21:07 PDT (History)
8 users (show)

See Also:


Attachments
Patch (6.17 KB, patch)
2018-04-27 13:35 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Follow-up fix of possible build failure (1.07 KB, patch)
2018-09-07 12:05 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Another follow-up fix of potential build failure (1006 bytes, patch)
2018-09-11 05:42 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (3.43 KB, patch)
2018-09-12 09:03 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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...