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

[bugzilla-daemon at webkit.org]

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


Hajime Morrita <morrita at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #165032|review?                     |review-
               Flag|                            |




--- Comment #11 from Hajime Morrita <morrita at google.com>  2012-09-24 22:48:15 PST ---
(From update of attachment 165032)
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.
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.

Thanks in advance.

> Source/WebCore/page/DeviceController.cpp:70
> +    m_event.clear();

Really?
This means m_event essentially never goes back null once it gets being set since we re-set m_event just after it is cleared.

> 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.

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.

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