[webkit-reviews] review granted: [Bug 42304] DeviceOrientation event listeners should never be called synchronously from addEventListener() : [Attachment 62390] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 26 08:18:13 PDT 2010


Jeremy Orlow <jorlow at chromium.org> has granted Steve Block
<steveblock at google.com>'s request for review:
Bug 42304: DeviceOrientation event listeners should never be called
synchronously from addEventListener()
https://bugs.webkit.org/show_bug.cgi?id=42304

Attachment 62390: Patch
https://bugs.webkit.org/attachment.cgi?id=62390&action=review

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
WebCore/dom/DeviceOrientationController.cpp:48
 +	m_timer.stop();
why is this necessary?

WebCore/dom/DeviceOrientationController.cpp:51
 +		m_client->lastOrientation() : DeviceOrientation::create();
I'd lean towards not wrapping these.

WebCore/dom/DeviceOrientationController.cpp:65
 +	// immediately trigger an asynchronous response.
This seems to imply that if it's not present, it would fire whenever it becomes
so...which does't seem to be true.

Also "immediately" and "asynchronous" kind of conflict.

WebCore/dom/DeviceOrientationController.h:57
 +	typedef HashSet<DOMWindow*> ListenersSet;
I'd lean towards putting newlines between the pairs of
typedef+declarations...but it's more personal preference.



r=me, but please consider making the above changes


More information about the webkit-reviews mailing list