[Webkit-unassigned] [Bug 89197] [Chromium] Implements DeviceMotion

[bugzilla-daemon at webkit.org]

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





--- Comment #16 from Amy Ousterhout <aousterh at chromium.org>  2012-06-21 07:08:37 PST ---
Thanks for the comments.

(In reply to comment #13)
> (From update of attachment 148560 [details])
> Would any of the API reviewrs like to take a look?
> 
> 
> I only have a couple of nits.
> 
> 
> View in context: https://bugs.webkit.org/attachment.cgi?id=148560&action=review
> 
> > Source/WebKit/chromium/public/WebDeviceMotionData.h:150
> > +    WebDeviceMotionData& operator=(const WebCore::DeviceMotionData*);
> 
> Hmm, I wonder if the two functions above are actually ever used or just there for "symmetry" reasons.
> I don't know if we ever create a WebDevicemotionData from a DeviceMotionData; i suspect we only do it the other way around.
> If we don't need them, maybe we should delete them because the definitions aren't too pretty.

The first function is used by WebDeviceMotionClientMock. However, the second function is unused, so I deleted it. Done.

> > LayoutTests/ChangeLog:9
> > +        Each tests corresponds to a similar existing test for DeviceOrientation, since they are part of the same spec and work very similarly.
> 
> s/Each tests/Each test/

Done.

> > LayoutTests/fast/dom/DeviceMotion/multiple-frames.html:6
> > +<script src="script-tests/multiple-frames.js"></script>
> 
> i don't know what the current WebKit policy is on this, maybe the other reviewers can comment, but if we could just put the test script in the .html file, that would be easier

I couldn't find any mention of a policy regarding this. If it conforms to WebKit policy, I agree that it would be simpler.

> > LayoutTests/fast/dom/DeviceMotion/script-tests/add-listener-from-callback.js:17
> > +if (window.layoutTestController) {
> 
> there's an effort underway to rename layoutTestController to testRunner. Currently, both names work, but any new code should ideally use testRunner.

Done.

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