Bug 39479 – Add DeviceOrientation and DeviceOrientationClient

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Steve Block]

Add DeviceOrientation and DeviceOrientationClient

[Steve Block]

This is the next step in implementing DeviceOrientation. See Bug 30335

[Steve Block]

Created attachment 56692 [details]
Patch

[Eric Seidel (no email)]

Attachment 56692 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/2248475

[Early Warning System Bot]

Attachment 56692 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/2288390

[WebKit Review Bot]

Attachment 56692 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/2264396

[Jeremy Orlow]

Comment on attachment 56692 [details]
Patch

You forget WebCore/CMakeLists.txt and visual studio.

WebCore/dom/DeviceOrientation.h:38
 +      // TODO: Add methods to start and stop the service.
FIXME

WebCore/dom/DeviceOrientation.cpp:45
 +      // TODO: Fire DeviceOrientationEvents on the window object of all frames
FIXME

[Steve Block]

> You forget WebCore/CMakeLists.txt and visual studio.
Looks like I did with my first patch too - will fix here.

> WebCore/dom/DeviceOrientation.h:38
>  +      // TODO: Add methods to start and stop the service.
> FIXME
Done

[WebKit Review Bot]

Attachment 56692 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2261407

[Steve Block]

Created attachment 56705 [details]
Patch

[Jeremy Orlow]

Comment on attachment 56705 [details]
Patch

WebCore/ChangeLog:14
 +          No new tests are possible at this time.
Please explain why.  And promise to land them soon.  :-)

WebCore/dom/DeviceOrientation.cpp:47
 +      UNUSED_PARAM(alpha);
You may need to include wtf/UnusedParam.h

WebKit/chromium/src/WebViewImpl.cpp:264
 +                          0));
I don't know why these are split across multiple lines like this, but it doesn't match WebKit style...so mind putting these all on one line?

WebKit/mac/WebView/WebView.mm:666
 +      _private->page = new Page(new WebChromeClient(self), new WebContextMenuClient(self), new WebEditorClient(self), new WebDragClient(self), new WebInspectorClient(self), new WebPluginHalterClient(self), geolocationControllerClient, deviceOrientationClient);
Where is deviceOrientationClient defined?

[Steve Block]

(In reply to comment #10)
> (From update of attachment 56705 [details])
> WebCore/ChangeLog:14
>  +          No new tests are possible at this time.
> Please explain why.  And promise to land them soon.  :-)
Done

> WebCore/dom/DeviceOrientation.cpp:47
>  +      UNUSED_PARAM(alpha);
> You may need to include wtf/UnusedParam.h
Done

> WebKit/chromium/src/WebViewImpl.cpp:264
>  +                          0));
> I don't know why these are split across multiple lines like this, but it doesn't match WebKit style...so mind putting these all on one line?
Done

> WebKit/mac/WebView/WebView.mm:666
>  +      _private->page = new Page(new WebChromeClient(self), new WebContextMenuClient(self), new WebEditorClient(self), new WebDragClient(self), new WebInspectorClient(self), new WebPluginHalterClient(self), geolocationControllerClient, deviceOrientationClient);
> Where is deviceOrientationClient defined?
On the line above. Since all the other params are named, I thought it best to do it like this rather than use 0. What do you think?

[Jeremy Orlow]

 
> > WebKit/mac/WebView/WebView.mm:666
> >  +      _private->page = new Page(new WebChromeClient(self), new WebContextMenuClient(self), new WebEditorClient(self), new WebDragClient(self), new WebInspectorClient(self), new WebPluginHalterClient(self), geolocationControllerClient, deviceOrientationClient);
> > Where is deviceOrientationClient defined?
> On the line above. Since all the other params are named, I thought it best to do it like this rather than use 0. What do you think?

Oh yeah...forgot objective c always names stuff like that.

[Steve Block]

Created attachment 56708 [details]
Patch

[Jeremy Orlow]

Comment on attachment 56708 [details]
Patch

r=me

[Steve Block]

Comment on attachment 56708 [details]
Patch

Clearing flags on attachment: 56708

Committed r59935: <http://trac.webkit.org/changeset/59935>

[Steve Block]

All reviewed patches have been landed.  Closing bug.