[Webkit-unassigned] [Bug 47264] Introduce the device element as an experimental feature

[bugzilla-daemon at webkit.org]

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





--- Comment #10 from Andrei Popescu <andreip at google.com>  2011-02-01 16:08:34 PST ---
A few more comments:

>      for (int i = ((int) m_listEntries.size()) - 1; i >= 0; i--) {

Please do not use C-style casts.

> for (j = ((int) otherSelectedEntries.size()) - 1; j >= 0; j--) 

ditto

> void HTMLDeviceElement::defaultEventHandler(Event* evt)
>  {
>     if (evt->type() == eventNames().DOMActivateEvent) {
>         runDialog();

...

> void HTMLDeviceElement::runDialog()
> {

(....)

>     m_deviceList = deviceManagerProxy().createDeviceList(deviceType());

(...)

>    if (Chrome* chrome = page->chrome())
>         chrome->runDeviceDialog(m_deviceList.get());

The above design is a reason for concern: it seems you are invoking synchronously, from inside an event handler, two potentially expensive operations:

1. deviceManagerProxy().createDeviceList(deviceType());

createDeviceList() method above is synchronous and is implemented in platform code and probes for a list of devices (cameras, microphones).

2. runDeviceDialog() is also synchronous and seems to involve running a modal dialog


I am wondering why you made these choices and whether the alternative design proposed by Leandro in https://lists.webkit.org/pipermail/webkit-dev/2011-January/015822.html isn't a much better choice. 

Leandro has actually been working on this and has a patch that he could upload for review. Perhaps the best way to proceed is for him to upload it and see how it differs to the current proposal?

Thanks,
Andrei

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