[Webkit-unassigned] [Bug 41616] [Chromium] DeviceOrientation plumbing

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from hans at chromium.org  2010-07-13 07:14:25 PST ---
(In reply to comment #5)
> (From update of attachment 60534 [details])
Thanks for the review.

> WebKit/chromium/src/WebViewImpl.h:44
>  +  #if ENABLE(DEVICE_ORIENTATION)
> We shoudl just turn it on now at compile time, but hide it behind a runtime flag.  Then you don't need this stuff.
Hmm, but we can't turn it on before we have landed an implementation, and we can't land this implementation without the enable guards if the switch is off, so I guess the plan would be to keep them for now and remove them eventually?

> 
> WebKit/chromium/src/WebDeviceOrientationServiceBridgeImpl.h:14
>  +   *     * Neither the name of Google Inc. nor the names of its
> Use 2 clause license
Will fix.

> 
> WebKit/chromium/src/WebDeviceOrientationServiceBridgeImpl.h:42
>  +      explicit WebDeviceOrientationServiceBridgeImpl(DeviceOrientationClientImpl* deviceOrientationClientImpl);
> only name parameters in .h files when it adds information
Will fix.

> 
> WebKit/chromium/src/WebDeviceOrientationServiceBridgeImpl.cpp:89
>  +      m_webDeviceOrientationService = NULL;
> use 0 not NULL in webkit code
Will fix.

> 
> WebKit/chromium/src/WebDeviceOrientationServiceBridgeImpl.cpp:35
>  +  #include "WebDeviceOrientationService.h"
> alpha order
Will fix.

> 
> WebKit/chromium/src/WebDeviceOrientationServiceBridgeImpl.cpp:14
>  +   *     * Neither the name of Google Inc. nor the names of its
> 2 clause
Will fix

> 
> WebKit/chromium/src/DeviceOrientationClientImpl.h:40
>  +  } // namespace WebCore
> don't need the //
Will fix. What's the convention for this? Looking at the guidelines, they seem to put a comment like this at the closing of most namespaces, but not in every case?

> 
> WebKit/chromium/src/DeviceOrientationClientImpl.cpp:51
>  +        return;
> 4 spaces
Whoops. Thanks for pointing that out.

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