Bug 39479

Summary: Add DeviceOrientation and DeviceOrientationClient
Product: WebKit Reporter: Steve Block <steveblock>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, gns, hans, jorlow, steveblock, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 30335    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Steve Block 2010-05-21 03:24:22 PDT
Add DeviceOrientation and DeviceOrientationClient
Comment 1 Steve Block 2010-05-21 03:27:18 PDT
This is the next step in implementing DeviceOrientation. See Bug 30335
Comment 2 Steve Block 2010-05-21 03:44:29 PDT
Created attachment 56692 [details]
Patch
Comment 3 Eric Seidel (no email) 2010-05-21 03:53:43 PDT
Attachment 56692 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/2248475
Comment 4 Early Warning System Bot 2010-05-21 03:54:48 PDT
Attachment 56692 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/2288390
Comment 5 WebKit Review Bot 2010-05-21 04:03:22 PDT
Attachment 56692 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/2264396
Comment 6 Jeremy Orlow 2010-05-21 04:03:34 PDT
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
Comment 7 Steve Block 2010-05-21 04:16:55 PDT
> 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
Comment 8 WebKit Review Bot 2010-05-21 04:19:29 PDT
Attachment 56692 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2261407
Comment 9 Steve Block 2010-05-21 06:53:27 PDT
Created attachment 56705 [details]
Patch
Comment 10 Jeremy Orlow 2010-05-21 07:00:56 PDT
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?
Comment 11 Steve Block 2010-05-21 07:09:18 PDT
(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?
Comment 12 Jeremy Orlow 2010-05-21 07:12:35 PDT
 
> > 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.
Comment 13 Steve Block 2010-05-21 07:50:27 PDT
Created attachment 56708 [details]
Patch
Comment 14 Jeremy Orlow 2010-05-21 07:56:41 PDT
Comment on attachment 56708 [details]
Patch

r=me
Comment 15 Steve Block 2010-05-21 09:07:42 PDT
Comment on attachment 56708 [details]
Patch

Clearing flags on attachment: 56708

Committed r59935: <http://trac.webkit.org/changeset/59935>
Comment 16 Steve Block 2010-05-21 09:07:55 PDT
All reviewed patches have been landed.  Closing bug.