[RequestAnimationFrame] Remove vendor prefix
Created attachment 168312 [details] Patch
Comment on attachment 168312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168312&action=review R=me > Source/WebCore/page/DOMWindow.idl:218 > // WebKit animation extensions, being standardized in the WebPerf WG probably worth updating this comment
Comment on attachment 168312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168312&action=review > Source/WebCore/page/DOMWindow.idl:-221 > - long webkitRequestAnimationFrame(in [Callback] RequestAnimationFrameCallback callback); > - void webkitCancelAnimationFrame(in long id); > - void webkitCancelRequestAnimationFrame(in long id); // This is a deprecated alias for webkitCancelAnimationFrame(). Remove this when removing vendor prefix. Do we want to expose both sets of APIs for a while?
Comment on attachment 168312 [details] Patch This patch seems a bit aggressive for this API. Have we measured the usage of the prefixed versions? Will this patch break sites?
IMHO, we should expose both versions for a while and use V8MeasureAs to measure how quickly web sites are transitioning from the prefixed to the unprefixed versions.
See http://lists.webkit.org/pipermail/webkit-dev/2012-September/022239.html for instructions about how to use V8MeasureAs and FeatureObserver.
There were a couple of reasons for making the transition. 1. Everyone else is already (or in the process of) shipping unprefixed. 2. We've changed our semantics to match the spec. So the old webkit* functions aren't compatible. 3. The major toolkits I checked already use the unprefixed version.
Would you be willing to email webkit-dev with this information? Different folks in the community have different risk tolerances for these sorts of changes, and it's better to talk things through so that folks don't feel railroaded in one direction or another. Here's a wiki page we wrote up based on previous discussions of similar changes: https://trac.webkit.org/wiki/DeprecatingFeatures
Created attachment 168445 [details] Patch
Comment on attachment 168445 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168445&action=review > Source/WebCore/page/DOMWindow.h:270 > + void webkitCancelAnimationFrame(int id) { cancelAnimationFrame(id); } > + void webkitCancelRequestAnimationFrame(int id) { cancelAnimationFrame(id); } You can just use ImplementedAs in the IDL rather than having these stub functions here. > Source/WebCore/page/DOMWindow.idl:216 > + // W3C Web Performance WG Request Animation Frame standard. I would skip this comment. It's not adding any information. > Source/WebCore/page/DOMWindow.idl:221 > + long requestAnimationFrame(in [Callback] RequestAnimationFrameCallback callback); > + void cancelAnimationFrame(in long id); > long webkitRequestAnimationFrame(in [Callback] RequestAnimationFrameCallback callback); > void webkitCancelAnimationFrame(in long id); > void webkitCancelRequestAnimationFrame(in long id); // This is a deprecated alias for webkitCancelAnimationFrame(). Remove this when removing vendor prefix. Please add V8MeasureAs attributes so that we can measure how often the prefixed versions are used. That way we'll know have more information about when we can remove them.
Created attachment 168457 [details] Patch
Comment on attachment 168457 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168457&action=review > Source/WebCore/page/DOMWindow.idl:220 > + long requestAnimationFrame(in [Callback] RequestAnimationFrameCallback callback); > + void cancelAnimationFrame(in long id); > + [ImplementedAs=requestAnimationFrame, V8MeasureAs=PrefixedRequestAnimationFrame] long webkitRequestAnimationFrame(in [Callback] RequestAnimationFrameCallback callback); > + [ImplementedAs=cancelAnimationFrame] void webkitCancelAnimationFrame(in long id); > + [ImplementedAs=cancelAnimationFrame] void webkitCancelRequestAnimationFrame(in long id); // This is a deprecated alias for webkitCancelAnimationFrame(). Remove this when removing vendor prefix. Should we measure the unprefixed one as well, so we can compare, like we do for IndexedDB?
Created attachment 168460 [details] Patch for landing
Comment on attachment 168460 [details] Patch for landing Clearing flags on attachment: 168460 Committed r131214: <http://trac.webkit.org/changeset/131214>
All reviewed patches have been landed. Closing bug.
Breaks gtk? http://queues.webkit.org/results/14267238
Sounds like the CPP bindings need to understand ImplementedAs. DerivedSources/webkit/WebKitDOMDOMWindow.cpp: In function 'void webkit_dom_dom_window_webkit_cancel_animation_frame(WebKitDOMDOMWindow*, glong)': DerivedSources/webkit/WebKitDOMDOMWindow.cpp:1116:11: error: 'class WebCore::DOMWindow' has no member named 'webkitCancelAnimationFrame' DerivedSources/webkit/WebKitDOMDOMWindow.cpp: In function 'void webkit_dom_dom_window_webkit_cancel_request_animation_frame(WebKitDOMDOMWindow*, glong)': DerivedSources/webkit/WebKitDOMDOMWindow.cpp:1125:11: error: 'class WebCore::DOMWindow' has no member named 'webkitCancelRequestAnimationFrame'
(In reply to comment #17) > Sounds like the CPP bindings need to understand ImplementedAs. Is someone going to do that or should I go back to the old way of forwarding?
(In reply to comment #18) > (In reply to comment #17) > > Sounds like the CPP bindings need to understand ImplementedAs. > > Is someone going to do that or should I go back to the old way of forwarding? I have a patch almost ready. Filed bug 99214.
The changelog and commit message don't say if the prefixed version was retained. Consensus on webkit-dev was to keep both prefixed and unprefixed.
This patch should have been r- because of the lack of details in the changlog.
> This patch should have been r- because of the lack of details in the changlog. Thanks for the reminder. I was somewhat distracted today with other disasters. The code in the patch should be correct, however.