Bug 89220 - Adds DeviceMotionClientMock
Summary: Adds DeviceMotionClientMock
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 89504
Blocks: 89197
  Show dependency treegraph
 
Reported: 2012-06-15 09:07 PDT by Amy Ousterhout
Modified: 2012-06-20 03:37 PDT (History)
5 users (show)

See Also:


Attachments
Patch (10.87 KB, patch)
2012-06-15 09:13 PDT, Amy Ousterhout
no flags Details | Formatted Diff | Diff
Patch (10.98 KB, patch)
2012-06-19 11:18 PDT, Amy Ousterhout
no flags Details | Formatted Diff | Diff
Patch (11.00 KB, patch)
2012-06-20 02:48 PDT, Amy Ousterhout
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Amy Ousterhout 2012-06-15 09:07:52 PDT
Adds DeviceMotionClientMock
Comment 1 Amy Ousterhout 2012-06-15 09:13:37 PDT
Created attachment 147833 [details]
Patch
Comment 2 Hans Wennborg 2012-06-18 09:13:53 PDT
This looks good to me.

Steve, would you like to take a look?
Comment 3 Steve Block 2012-06-19 08:24:24 PDT
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
Comment 4 Amy Ousterhout 2012-06-19 11:18:36 PDT
Created attachment 148366 [details]
Patch
Comment 5 Amy Ousterhout 2012-06-19 11:19:41 PDT
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.
Comment 6 WebKit Review Bot 2012-06-19 12:28:04 PDT
Comment on attachment 148366 [details]
Patch

Clearing flags on attachment: 148366

Committed r120744: <http://trac.webkit.org/changeset/120744>
Comment 7 WebKit Review Bot 2012-06-19 12:28:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Kenneth Russell 2012-06-19 13:02:21 PDT
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.
Comment 9 Kenneth Russell 2012-06-19 13:12:12 PDT
Reverted r120744 for reason:

Broke build on Chromium Windows canary bots

Committed r120748: <http://trac.webkit.org/changeset/120748>
Comment 10 Kenneth Russell 2012-06-19 13:19:19 PDT
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.
Comment 11 Steve Block 2012-06-20 02:31:35 PDT
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?
Comment 12 Amy Ousterhout 2012-06-20 02:48:12 PDT
Created attachment 148528 [details]
Patch
Comment 13 Amy Ousterhout 2012-06-20 02:53:21 PDT
(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.
Comment 14 Steve Block 2012-06-20 03:04:16 PDT
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?
Comment 15 Amy Ousterhout 2012-06-20 03:14:39 PDT
(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.
Comment 16 Steve Block 2012-06-20 03:17:22 PDT
Comment on attachment 148528 [details]
Patch

r=me
Comment 17 WebKit Review Bot 2012-06-20 03:37:07 PDT
Comment on attachment 148528 [details]
Patch

Clearing flags on attachment: 148528

Committed r120814: <http://trac.webkit.org/changeset/120814>
Comment 18 WebKit Review Bot 2012-06-20 03:37:12 PDT
All reviewed patches have been landed.  Closing bug.