RESOLVED FIXED233356
[Model] add support for controlling looping animations 
https://bugs.webkit.org/show_bug.cgi?id=233356
Summary [Model] add support for controlling looping animations
Antoine Quint
Reported 2021-11-19 02:43:47 PST
[Model] add support for controlling looping animations
Attachments
Patch (22.13 KB, patch)
2021-11-19 02:45 PST, Antoine Quint 		wenson_hsieh: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Antoine Quint
Comment 1 2021-11-19 02:45:42 PST
Created attachment 444795 [details] Patch
Antoine Quint
Comment 2 2021-11-19 02:45:48 PST
<rdar://problem/85428884>
Wenson Hsieh
Comment 3 2021-11-19 08:14:20 PST
Comment on attachment 444795 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=444795&action=review > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:366 > + callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler), error = WebCore::ResourceError { WebCore::ResourceError::Type::General }] () mutable { Nit - might be simpler as `completionHandler(makeUnexpected(WebCore::ResourceError { WebCore::ResourceError::Type::General }));`, instead of copying the error via block capture. > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:397 > + callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable { Same question as the other patch — could we clarify why we need these `callOnMainRunLoop`s?
Antoine Quint
Comment 4 2021-11-19 09:59:15 PST
(In reply to Wenson Hsieh from comment #3) > Comment on attachment 444795 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444795&action=review > > > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:366 > > + callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler), error = WebCore::ResourceError { WebCore::ResourceError::Type::General }] () mutable { > > Nit - might be simpler as > `completionHandler(makeUnexpected(WebCore::ResourceError { > WebCore::ResourceError::Type::General }));`, instead of copying the error > via block capture. Absolutely, will fix when landing. > > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:397 > > + callOnMainRunLoop([weakThis = WeakPtr { *this }, completionHandler = WTFMove(completionHandler)] () mutable { > > Same question as the other patch — could we clarify why we need these > `callOnMainRunLoop`s? I think we only need those when using block-based APIs, will remove the use of `callOnMainRunLoop()` when landing.
Antoine Quint
Comment 5 2021-11-19 10:22:11 PST
Committed r286066 (244453@main): <https://commits.webkit.org/244453@main>
Note You need to log in before you can comment on or make changes to this bug.