Bug 43181 – Hook up LayoutTestController.setMockDeviceOrientation() on Mac.

• Component: WebKit API
• Status: RESOLVED FIXED

[Steve Block]

This new LayoutTestCotroller method is being added in Bug 39589

[Steve Block]

Created attachment 62998 [details]
Patch

[Steve Block]

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.

[Steve Block]

Created attachment 63060 [details]
Patch

[WebKit Review Bot]

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.

[Steve Block]

> Attachment 63060 [details] did not pass style-queue:
This is a false positive. Filed Bug 43252.

[Eric Seidel (no email)]

Attachment 63060 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3615342

[Steve Block]

Created attachment 63195 [details]
Patch

[WebKit Review Bot]

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.

[Steve Block]

Dean, do you know anybody who could review this? It's been in the queue for over a week now.

[Dean Jackson]

Chris, can you look at this with your new reviewing power?

[Steve Block]

Ping!

[Simon Fraser (smfr)]

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?

[Steve Block]

Created attachment 64853 [details]
Patch

[WebKit Review Bot]

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.

[Steve Block]

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.

[Adam Barth]

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.

[Steve Block]

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.

[WebKit Commit Bot]

Comment on attachment 64853 [details]
Patch

Clearing flags on attachment: 64853

Committed r66685: <http://trac.webkit.org/changeset/66685>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.