[webkit-reviews] review granted: [Bug 233362] [Model] add support for seeking animations : [Attachment 444811] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Nov 19 08:11:54 PST 2021


Wenson Hsieh <wenson_hsieh at apple.com> has granted Antoine Quint
<graouts at webkit.org>'s request for review:
Bug 233362: [Model] add support for seeking animations
https://bugs.webkit.org/show_bug.cgi?id=233362

Attachment 444811: Patch

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




--- Comment #3 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 444811
  --> https://bugs.webkit.org/attachment.cgi?id=444811
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)


More information about the webkit-reviews mailing list