Bug 89220 – Adds DeviceMotionClientMock

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Amy Ousterhout]

Adds DeviceMotionClientMock

[Amy Ousterhout]

Created attachment 147833 [details]
Patch

[Hans Wennborg]

This looks good to me.

Steve, would you like to take a look?

[Steve Block]

Comment on attachment 147833 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147833&action=review

r=me

> Source/WebCore/platform/mock/DeviceMotionClientMock.h:52
> +    virtual void deviceMotionControllerDestroyed() OVERRIDE { }

Maybe add a comment about why we don't need to do anything here

[Amy Ousterhout]

Created attachment 148366 [details]
Patch

[Amy Ousterhout]

thanks for the review!

(In reply to comment #3)
> (From update of attachment 147833 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=147833&action=review
> 
> r=me
> 
> > Source/WebCore/platform/mock/DeviceMotionClientMock.h:52
> > +    virtual void deviceMotionControllerDestroyed() OVERRIDE { }
> 
> Maybe add a comment about why we don't need to do anything here

Done.

[WebKit Review Bot]

Comment on attachment 148366 [details]
Patch

Clearing flags on attachment: 148366

Committed r120744: <http://trac.webkit.org/changeset/120744>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Kenneth Russell]

This looks like it broke the Chromium Windows canaries. A sample build failure:

http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Win%20Builder/builds/26273

I'm going to roll it out.

[Kenneth Russell]

Reverted r120744 for reason:

Broke build on Chromium Windows canary bots

Committed r120748: <http://trac.webkit.org/changeset/120748>

[Kenneth Russell]

One more example failure, from the build.webkit.org bots: http://build.webkit.org/builders/Chromium%20Win%20Release/builds/45417 . Looks like WebCore\dom\DeviceMotion.h isn't present in the Windows build.

[Steve Block]

Comment on attachment 148366 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148366&action=review

> Source/WebCore/WebCore.gypi:626
> +            'dom/DeviceMotion.h',

Sorry, I should have spotted this - it looks like this is leftover from the old all-in-one patchset - and broke the build.

> Source/WebCore/WebCore.gypi:627
> +            'dom/DeviceMotionClient.h',

Presumably this was erroneously left out of a previous change?

[Amy Ousterhout]

Created attachment 148528 [details]
Patch

[Amy Ousterhout]

(In reply to comment #11)
> (From update of attachment 148366 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148366&action=review
> 
> > Source/WebCore/WebCore.gypi:626
> > +            'dom/DeviceMotion.h',
> 
> Sorry, I should have spotted this - it looks like this is leftover from the old all-in-one patchset - and broke the build.

Sorry about that - I fixed it in the new attachment.

> > Source/WebCore/WebCore.gypi:627
> > +            'dom/DeviceMotionClient.h',
> 
> Presumably this was erroneously left out of a previous change?

Yes, I think so, since DeviceMotionClient.h already existed.

[Steve Block]

Comment on attachment 148528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148528&action=review

> Source/WebCore/WebCore.gypi:627
> +            'dom/DeviceMotionData.h',

I guess this one was missing too?

[Amy Ousterhout]

(In reply to comment #14)
> (From update of attachment 148528 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148528&action=review
> 
> > Source/WebCore/WebCore.gypi:627
> > +            'dom/DeviceMotionData.h',
> 
> I guess this one was missing too?

Yes, it appears so.

[Steve Block]

Comment on attachment 148528 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 148528 [details]
Patch

Clearing flags on attachment: 148528

Committed r120814: <http://trac.webkit.org/changeset/120814>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.