Bug 233362

Summary:

[Model] add support for seeking animations

Product:

WebKit

Reporter:

Antoine Quint <graouts>

Component:

New Bugs

Assignee:

Antoine Quint <graouts>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, dino, esprehn+autocc, ews-watchlist, kondapallykalyan, thorton, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	wenson_hsieh: review+

Description Antoine Quint 2021-11-19 05:53:27 PST

[Model] add support for seeking animations

Comment 1 Antoine Quint 2021-11-19 05:57:10 PST

Created attachment 444811 [details]
Patch

Comment 2 Antoine Quint 2021-11-19 05:57:16 PST

<rdar://problem/85428812>

Comment 3 Wenson Hsieh 2021-11-19 08:11:54 PST

Comment on attachment 444811 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:367
> +        callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler), error = WebCore::ResourceError { WebCore::ResourceError::Type::General }] () mutable {

Nit - I would just pass in `WebCore::ResourceError { WebCore::ResourceError::Type::General }` below, instead of copying it in the block.

> Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:376
> +    callOnMainRunLoop([duration, weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable {

It's not super clear from reading the code (or the ChangeLog) why this would need to be called on the next runloop. Could we explain in either the ChangeLog or a comment here?

> Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:389
> +        callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler), error = WebCore::ResourceError { WebCore::ResourceError::Type::General }] () mutable {

(Ditto re: the error)

> Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:398
> +    callOnMainRunLoop([currentTime, weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable {

(Ditto)

> Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:420
> +    callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable {

(Ditto)

Comment 4 Antoine Quint 2021-11-19 10:44:30 PST

(In reply to Wenson Hsieh from comment #3)
> Comment on attachment 444811 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=444811&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:367
> > +        callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler), error = WebCore::ResourceError { WebCore::ResourceError::Type::General }] () mutable {
> 
> Nit - I would just pass in `WebCore::ResourceError {
> WebCore::ResourceError::Type::General }` below, instead of copying it in the
> block.

Will fix in commit.

> > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:376
> > +    callOnMainRunLoop([duration, weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable {
> 
> It's not super clear from reading the code (or the ChangeLog) why this would
> need to be called on the next runloop. Could we explain in either the
> ChangeLog or a comment here?
> 
> > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:389
> > +        callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler), error = WebCore::ResourceError { WebCore::ResourceError::Type::General }] () mutable {
> 
> (Ditto re: the error)
> 
> > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:398
> > +    callOnMainRunLoop([currentTime, weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable {
> 
> (Ditto)
> 
> > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:420
> > +    callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable {
> 
> (Ditto)

I'll remove all calls to callOnMainRunLoop() in the commit.

Comment 5 Antoine Quint 2021-11-19 11:06:07 PST

Committed r286068 (244455@main): <https://commits.webkit.org/244455@main>