[webkit-reviews] review denied: [Bug 42865] Implement AccelerometerEvent : [Attachment 63296] Updated patch after the Page constructor changes

[bugzilla-daemon at webkit.org]

Steve Block <steveblock at google.com> has denied Dean Jackson <dino at apple.com>'s
request for review:
Bug 42865: Implement AccelerometerEvent
https://bugs.webkit.org/show_bug.cgi?id=42865

Attachment 63296: Updated patch after the Page constructor changes
https://bugs.webkit.org/attachment.cgi?id=63296&action=review

------- Additional Comments from Steve Block <steveblock at google.com>
LayoutTests/fast/dom/Window/window-properties-device-orientation.html:2
 +    These properties are currently guarded by ENABLE_DEVICE_ORIENTATION.</p>
Looks like this last sentence isn't in the expected result.

LayoutTests/platform/gtk/Skipped:1096
 +  fast/dom/Accelerometer/window-property.html
alpha ordering

WebCore/WebCore.gypi:525
 +		'bindings/js/JSAccelerometerEventCustom.cpp',
alpha ordering

WebCore/WebCore.pro:286
 +	bindings/js/JSAccelerometerEventCustom.cpp \
alpha ordering

WebCore/WebCore.xcodeproj/project.pbxproj: 
 +				59A86007119DAFA100DEF1EF /*
JSDeviceOrientationEvent.h */,
Is this intentional?

WebKitTools/Scripts/build-webkit:102
 +  
This isn't needed. It was added in
http://trac.webkit.org/changeset/64270/trunk/WebKitTools/Scripts/build-webkit

WebCore/dom/AccelerometerController.cpp:29
 +  #if ENABLE(DEVICE_ORIENTATION)
I'm not sure there's a formal policy for using guards, but my approach has been
to use as few as possible to make sure the feature isn't exposed when it's not
required. I find this keeps things as simple as possible. In particular, there
should be no need to guard the contents of these cpp files.

LayoutTests/ChangeLog:27
 +	    * fast/dom/Accelerometer/window-property.html: Added.
Could you file a bug to track the fact that this new test won't pass with V8.
CC me and I'll provide the V8-specific result.