Summary: | Hook up LayoutTestController.setMockDeviceOrientation() on Mac. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Steve Block <steveblock> | ||||||||||
Component: | WebKit API | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cmarrin, commit-queue, dino, eric, hans, simon.fraser, steveblock, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | 39589 | ||||||||||||
Bug Blocks: | 30335, 39590 | ||||||||||||
Attachments: |
|
Description
Steve Block
2010-07-29 02:09:22 PDT
Created attachment 62998 [details]
Patch
Note that this patch will fail to apply, let alone build, until the Chromium problems are sorted out and the patch in Bug 39589 is re-landed. Created attachment 63060 [details]
Patch
Attachment 63060 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/mac/WebView/WebDeviceOrientationInternal.h:39: Extra space before ( in function call [whitespace/parens] [4]
WebKit/mac/WebView/WebDeviceOrientationControllerInternal.h:39: Extra space before ( in function call [whitespace/parens] [4]
WebKit/mac/WebView/WebViewPrivate.h:610: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 3 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
> Attachment 63060 [details] did not pass style-queue: This is a false positive. Filed Bug 43252. Attachment 63060 [details] did not build on mac: Build output: http://queues.webkit.org/results/3615342 Created attachment 63195 [details]
Patch
Attachment 63195 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/mac/WebView/WebDeviceOrientationControllerInternal.h:39: Extra space before ( in function call [whitespace/parens] [4]
WebKit/mac/WebView/WebViewPrivate.h:610: Extra space before ( in function call [whitespace/parens] [4]
WebKit/mac/WebView/WebDeviceOrientationInternal.h:39: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 3 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Dean, do you know anybody who could review this? It's been in the queue for over a week now. Chris, can you look at this with your new reviewing power? Ping! Comment on attachment 63195 [details]
Patch
I'm a little surprised that there are so many classes here: WebDeviceOrientationClient, WebDeviceOrientation, WebDeviceOrientationInternal , WebDeviceOrientationInternal, WebDeviceOrientationController, WebDeviceOrientationControllerInternal, WebDeviceOrientationProvider, WebDeviceOrientationProviderMock and WebDeviceOrientationProviderMockInternal. Are they really all needed?
Created attachment 64853 [details]
Patch
Attachment 64853 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/mac/WebView/WebViewPrivate.h:609: Extra space before ( in function call [whitespace/parens] [4]
WebKit/mac/WebView/WebDeviceOrientationProviderMockInternal.h:46: Extra space before ( in function call [whitespace/parens] [4]
WebKit/mac/WebView/WebDeviceOrientationInternal.h:39: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 3 in 19 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Thanks for the review Simon. The reason for the numerous classes is that we're implementing a WebCore client interface for which we need both a real and a mock implementation. I've uploaded a new patch which eliminates the need for WebDeviceOrientationController by doing a runtime check. I think all of the other classes are required. I'm no expert on Mac, but I've followed the pattern used by Geolocation. Here's a summary of the new classes ... - WebDeviceOrientationClient - Mac implementation of WebCore::DeviceOrientationClient. Acts a proxy to either the mock or real provider. - WebDeviceOrientationProvider - New interface used by WebDeviceOrientationClient and implemented by the real and mock providers. - WebDeviceOrientationProviderMock - Mock provider, wrapper around WebCore::DeviceOrientationClientMock - WebDeviceOrientation - Wrapper around WebCore::DeviceOrientation. Required by DRT to configure the mock, without exposing WebCore types. Where these classes are wrappers around WebCore types, I've used a FooInternal internal class to hide the WebCore types from the WebKit API. Comment on attachment 64853 [details]
Patch
This patch might be a bit outside my area (since I don't really have more than a passing understanding of ObjectiveC), but this looks reasonable to me.
Thanks for the review Adam. Unless anybody else has comments, I'll submit tomorrow, as this has now been in the review queue for some time. Comment on attachment 64853 [details] Patch Clearing flags on attachment: 64853 Committed r66685: <http://trac.webkit.org/changeset/66685> All reviewed patches have been landed. Closing bug. |