[Webkit-unassigned] [Bug 43258] Implement chromium WebDeviceOrientationClient wrapper and have WebViewImpl get it from WebViewClient.

[bugzilla-daemon at webkit.org]

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


Jeremy Orlow <jorlow at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #63312|review?                     |review-
               Flag|                            |




--- Comment #7 from Jeremy Orlow <jorlow at chromium.org>  2010-08-03 03:23:55 PST ---
(From update of attachment 63312)
WebKit/chromium/src/WebDeviceOrientationClientDummy.cpp:37
 +      DeviceOrientationClientDummy() : orientation(WebCore::DeviceOrientation::create()) { }
usually we split onto 2 lines right before the :

WebKit/chromium/public/WebDeviceOrientation.h:32
 +  #include <wtf/PassRefPtr.h>
You can just do "namespace WTF { template <typename T> class PassRefPtr; }"

WebKit/chromium/public/WebDeviceOrientation.h:43
 +          initialize(canProvideAlpha, alpha, canProvideBeta, beta, canProvideGamma, gamma);
Darin, in the past you've been pretty adamant that constructors be inline.  Do you think that doing it this way is better than the alternative?  I'm not so sure.

WebKit/chromium/public/WebDeviceOrientation.h:48
 +      operator PassRefPtr<WebCore::DeviceOrientation>() const;
add the other 2 operators we have in just about every other class like this

WebKit/chromium/public/WebDeviceOrientationClient.h:50
 +      WebDeviceOrientationClient() { }
This works??  I'm pretty sure this will need to be public and virtual.

WebKit/chromium/src/WebViewImpl.cpp:267
 +      , m_webDeviceOrientationClient(WebRuntimeFeatures::isDeviceOrientationEnabled() ? (client ? client->createWebDeviceOrientationClient() : WebDeviceOrientationClientDummy::create()) : 0)
What is the dummy for?  I don't think client can ever be non-null.  And, if it can be, then we should just assume the device orientation client is null too then.

I don't see any point to the dummy.  I'd rather not add logic to WebKit to handle the client and DOclient being null unless there's already precedent.  But even if so, I think that's better than having some dummy client.

Another option, if all else fails, you could just allocate the mock.


hmmm...lets just talk in person

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