[webkit-reviews] review denied: [Bug 42265] Runtime feature switch for device orientation : [Attachment 61526] Patch

[bugzilla-daemon at webkit.org]

Jeremy Orlow <jorlow at chromium.org> has denied hans at chromium.org's request for
review:
Bug 42265: Runtime feature switch for device orientation
https://bugs.webkit.org/show_bug.cgi?id=42265

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

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
WebCore/bindings/generic/RuntimeEnabledFeatures.cpp:50
 +  bool RuntimeEnabledFeatures::isDeviceOrientationEventEnabled = true;
I think you can omit "Event" from all of these names.


WebCore/page/DOMWindow.cpp:1416
 +	else if (eventType == eventNames().deviceorientationEvent && frame() &&
frame()->page() && RuntimeEnabledFeatures::deviceOrientationEventEnabled())
I'd slightly prefer putting this logic into the deviceOrientation() method and
having that return 0 if it's turned off at runtime.  (And thus check for it
being null here.)

At very least, you should add a double check in that method to make sure it
only gets instantiated when it's enabled.


Please add a test in chromium to verify this stuff can't be seen when the flag
is off.