[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 #63884|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #21 from Jeremy Orlow <jorlow at chromium.org>  2010-08-09 07:21:35 PST ---
(From update of attachment 63884)
WebKit/chromium/public/WebDeviceOrientation.h:49
 +      static WebDeviceOrientation nullOrientation() { return WebDeviceOrientation(); }
Since you're using (), C++ guarantees us that everything is 0'ed out?

WebKit/chromium/public/WebDeviceOrientationClientMock.h:41
 +      void setController(WebDeviceOrientationController*);
Either virtual of WEBKIT_API

WebKit/chromium/public/WebDeviceOrientationClientMock.h:49
 +      void initialize();
WEBKIT_API

WebKit/chromium/public/WebDeviceOrientationClientMock.h:52
 +      WebCore::DeviceOrientationClientMock* m_clientMock;
I can't remember if this is allowed or not.  We might prefer a void* and casting...see if you can find other examples (like in PrivatePtr).

WebKit/chromium/src/WebDeviceOrientation.cpp:70
 +          return PassRefPtr<WebCore::DeviceOrientation>(0);
Can't you just say "return 0;" ?

WebKit/chromium/src/WebDeviceOrientationClientMock.cpp:53
 +      return WebDeviceOrientation(lastOrientation);
Why store it to the temp variable...doesn't seem to give you much....and why do you need to cast it explicitly either?  It should just work.

WebKit/chromium/src/WebViewImpl.cpp:290
 +      if (WebRuntimeFeatures::isDeviceOrientationEnabled())
Can't you do this either way?  Worst case, it gets set to null which is what would have happened anyway.


Minor comments so r=me, but you're not a committer so you'll need to post another rev.  Let me know if you only address stuff I mention so I can avoid yet another review of the whole thing.

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