[Webkit-unassigned] [Bug 96894] Add DeviceController base-class to remove duplication of DeviceXXXControler

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 25 22:55:07 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=96894





--- Comment #14 from Kihong Kwon <kihong.kwon at samsung.com>  2012-09-25 22:55:31 PST ---
(In reply to comment #11)
> (From update of attachment 165032 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=165032&action=review
> 
> I'm sorry but I don't understand the underlying event lifecycle and why we need m_event.

Basic idea of m_event is from lastOrientation of legacy DeviceOrientaionController.
If there is an event already fired when addDeviceEventListener is called from DOMWindow::addEventListener, we can fire an event for the listener using preserved event.(This can eliminate response time delaying)
IMHO, this is useful functionality because we can fire an event immediately without any delay, otherwise we don't guarantee when device event is fired.

> Could you explain the whole design briefly or drop me a link if you have?
> Reviewing this patch without understanding the interaction between WebKit layer is like blind man touching an elephant.

Here is the DeviceProximityController implementation using DeviceController.
https://bugs.webkit.org/attachment.cgi?id=165729&action=review

> 
> > Source/WebCore/page/DeviceController.cpp:93
> > +        listenerVector[i]->dispatchEvent(m_event);
> 
> You shouldn't dispatch same event object for different DOMWidows object
> since its JavaScript object can be re-used and it can result a possible information leak across different domain.
> We should create a fresh event for each window instead.

Actually, all attributes of device events are readonly, therefore if we use same event object, there is no way to overwrite or modify an event data.
In addition, we already use an event like this in the Device[Orientation/Motion]Controller.

> 
> Basically, passing Event objects from WebKit layer isn't good idea. Event is WebCore concept and should be live inside WebCore.
> For UI related event, WebKit has a low-level, "PlatformEvent" class for WebKit layer abstraction. We might need similar idea here.

You are right. As you can see in the Bug 97630, we don't make an event in the WebKit layer. We generate an event in the WebCore.

Thank you for your reviewing.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list