Implement Vibration client for WebKit2 EFL.
Created attachment 152531 [details] Patch This is dependent on patch in bug 91081.
Created attachment 153491 [details] Patch Implement vibration provider for WebKit2 EFL and also add an API to set vibration client callbacks by application. The callbacks set by application will be called back when the controller calls vibration APIs. This way application developers can handle the device feedback implementation in their application based on the platform support.
Comment on attachment 153491 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153491&action=review > Source/WebKit2/UIProcess/API/efl/VibrationProvider.cpp:46 > + _Ewk_Vibration_Client(Ewk_Vibration_Client_Vibrate_Cb vibrate, Ewk_Vibration_Client_Vibration_Cancel_Cb cancelVibration, void *userData) Star on the wrong side. > Source/WebKit2/UIProcess/API/efl/VibrationProvider.cpp:103 > +void VibrationProvider::setVibrationClientCallbacks(Ewk_Vibration_Client_Vibrate_Cb vibrate, Ewk_Vibration_Client_Vibration_Cancel_Cb cancelVibration, void *data) star on the wrong side. > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:96 > +void ewk_context_vibration_client_callbacks_set(const Ewk_Context* ewkContext, Ewk_Vibration_Client_Vibrate_Cb vibrate, Ewk_Vibration_Client_Vibration_Cancel_Cb cancel, void *data) This is a setter, the ewkContext argument should not be const. + star on the wrong side for user data. > Source/WebKit2/UIProcess/API/efl/ewk_context.h:63 > +typedef void (*Ewk_Vibration_Client_Vibrate_Cb)(uint64_t vibration_time, void *user_data); I think we usually put the callback typdefs at the top of the file, before all the functions. > Source/WebKit2/UIProcess/API/efl/ewk_context.h:74 > + * vibration in the clint appliation when the content ask for vibration. "application", "client", "asks"
Created attachment 153500 [details] Patch Fix nits spotted by Chris. Thanks Chris!
Comment on attachment 153500 [details] Patch LGTM.
Comment on attachment 153500 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153500&action=review > Source/WebKit2/UIProcess/API/efl/VibrationProvider.h:48 > + VibrationProvider(WKVibrationRef); Missing *explicit* keyword. > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:98 > + EINA_SAFETY_ON_NULL_RETURN(ewkContext); I wonder if *cancel* and *data* parameters can be NULL. If other functions are not handle this, is it better to check if they are NULL ?
Comment on attachment 153500 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153500&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:98 >> + EINA_SAFETY_ON_NULL_RETURN(ewkContext); > > I wonder if *cancel* and *data* parameters can be NULL. If other functions are not handle this, is it better to check if they are NULL ? data is "user data" so it may be NULL for sure (it is even documented). It will work fine if the callbacks are NULL but it should be documented in the header. It is actually useful to stop listening for vibration events (you call the function again and pass NULL for the callbacks).
Comment on attachment 153500 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153500&action=review >>> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:98 >>> + EINA_SAFETY_ON_NULL_RETURN(ewkContext); >> >> I wonder if *cancel* and *data* parameters can be NULL. If other functions are not handle this, is it better to check if they are NULL ? > > data is "user data" so it may be NULL for sure (it is even documented). It will work fine if the callbacks are NULL but it should be documented in the header. It is actually useful to stop listening for vibration events (you call the function again and pass NULL for the callbacks). You are right, okay to pass NULL for the callbacks. I'm going to update the documentation in the header.
Created attachment 153558 [details] Patch Fix review comments.
Comment on attachment 153558 [details] Patch LGTM.
CC'ing Grzegorz, Could you take a look this patch ?
Created attachment 153865 [details] Patch Rebased.
Comment on attachment 153865 [details] Patch Clearing flags on attachment: 153865 Committed r123482: <http://trac.webkit.org/changeset/123482>
All reviewed patches have been landed. Closing bug.