Bug 195914 - [ContentChangeObserver] Add support for observing implicit transitions
Summary: [ContentChangeObserver] Add support for observing implicit transitions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-18 15:03 PDT by zalan
Modified: 2019-03-21 11:33 PDT (History)
7 users (show)

See Also:

Attachments
Patch (13.12 KB, patch)
2019-03-18 15:09 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (13.12 KB, patch)
2019-03-18 15:51 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (13.15 KB, patch)
2019-03-18 16:44 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (19.05 KB, patch)
2019-03-19 08:30 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (19.05 KB, patch)
2019-03-19 09:25 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (10.69 KB, patch)
2019-03-20 20:25 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (10.72 KB, patch)
2019-03-20 20:28 PDT, zalan 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (6) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2019-03-18 15:03:29 PDT 
ssia
Comment 1 zalan 2019-03-18 15:09:31 PDT 
Created attachment 365073 [details]
Patch
Comment 2 zalan 2019-03-18 15:51:09 PDT 
Created attachment 365080 [details]
Patch
Comment 3 zalan 2019-03-18 16:44:41 PDT 
Created attachment 365089 [details]
Patch
Comment 4 Simon Fraser (smfr) 2019-03-18 18:10:20 PDT 
Comment on attachment 365089 [details]
Patch

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

> Source/WebCore/page/ios/ContentChangeObserver.cpp:130
> +void ContentChangeObserver::didFinishAnimationOrTransition(const RenderElement& renderer)

You can be running transition on multiple properties on an element. it might transition margin-left for 10ms, and left for 200ms. What happens then? Seems like you need to track per CSS property ID.

You might also run one transition, then another animation, all with different properties.

> Source/WebCore/rendering/RenderElement.cpp:2175
> +    document().contentChangeObserver().didStartAnimationOrTransition(*this, propertyID, Seconds { timeOffset });

Is it a timeOffset or a duration?

> Source/WebCore/rendering/RenderElement.cpp:2194
> +        document().contentChangeObserver().didStartAnimationOrTransition(*this, animation->property(), Seconds { timeOffset });

Ditto. Is animation ever non-null?
Comment 5 zalan 2019-03-18 18:13:39 PDT 
(In reply to Simon Fraser (smfr) from comment #4)
> Comment on attachment 365089 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365089&action=review
> 
> > Source/WebCore/page/ios/ContentChangeObserver.cpp:130
> > +void ContentChangeObserver::didFinishAnimationOrTransition(const RenderElement& renderer)
> 
> You can be running transition on multiple properties on an element. it might
> transition margin-left for 10ms, and left for 200ms. What happens then?
> Seems like you need to track per CSS property ID.
> 
> You might also run one transition, then another animation, all with
> different properties.
We track only one property at this point.
Comment 6 zalan 2019-03-19 08:30:40 PDT 
Created attachment 365162 [details]
Patch
Comment 7 zalan 2019-03-19 09:25:03 PDT 
Created attachment 365170 [details]
Patch
Comment 8 zalan 2019-03-20 20:25:04 PDT 
Created attachment 365470 [details]
Patch
Comment 9 Radar WebKit Bug Importer 2019-03-20 20:27:03 PDT 
<rdar://problem/49091959>
Comment 10 zalan 2019-03-20 20:28:57 PDT 
Created attachment 365471 [details]
Patch
Comment 11 zalan 2019-03-21 11:33:16 PDT 
Committed r243304: <https://trac.webkit.org/changeset/243304>