Bug 113499 – WebCore refactoring of device motion/orientation, to use the platform-based approach

• Component: New Bugs
• Status: UNCONFIRMED

WebCore refactoring of device motion/orientation and fixing of webkit ports

Created attachment 195536 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[WebKit Review Bot]

Attachment 195536 [details] did not pass style-queue:

Source/WebCore/page/DeviceController.cpp:54:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/page/DeviceController.cpp:64:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/page/DeviceController.cpp:74:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:40:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:41:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:42:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:47:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:48:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:49:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:50:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:51:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp:39:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp:42:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp:34:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp:37:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:43:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:44:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceMotionController.cpp:46:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:59:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceMotionController.cpp:67:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:71:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:37:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:38:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:39:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:40:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:43:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:45:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:47:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:37:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:38:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:39:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:42:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:45:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:39:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:40:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/DeviceOrientationController.cpp:47:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:48:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceOrientationController.cpp:50:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:59:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:65:  One space before end of line comments  [whitespace/comments] [5]
Source/WebCore/dom/DeviceOrientationController.cpp:65:  ShouldFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platform/chromium/public/Platform.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/dom/DeviceMotionController.cpp', u'Source/WebCore/dom/DeviceMotionController.h', u'Source/WebCore/dom/DeviceOrientationController.cpp', u'Source/WebCore/dom/DeviceOrientationController.h', u'Source/WebCore/history/PageCache.cpp', u'Source/WebCore/inspector/InspectorPageAgent.cpp', u'Source/WebCore/page/DOMWindow.cpp', u'Source/WebCore/page/DOMWindow.h', u'Source/WebCore/page/DeviceController.cpp', u'Source/WebCore/page/DeviceController.h', u'Source/WebCore/platform/PlatformDeviceMotion.h', u'Source/WebCore/platform/PlatformDeviceOrientation.h', u'Source/WebCore/platform/chromium/DeviceMotionChromium.cpp', u'Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp', u'Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp', u'Source/WebCore/platform/qt/DeviceMotionProviderQt.h', u'Source/WebCore/platform/qt/DeviceMotionQt.cpp', u'Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp', u'Source/WebCore/platform/qt/DeviceOrientationProviderQt.h', u'Source/WebCore/platform/qt/DeviceOrientationQt.cpp', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/WebDeviceOrientationController.cpp', u'Source/WebKit/chromium/src/WebViewImpl.cpp']" exit_code: 1
 have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:74:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:79:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 59 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 195536 [details]
Patch

Attachment 195536 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17322223

[Early Warning System Bot]

Comment on attachment 195536 [details]
Patch

Attachment 195536 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17257513

[Early Warning System Bot]

Comment on attachment 195536 [details]
Patch

Attachment 195536 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17257514

[WebKit Review Bot]

Comment on attachment 195536 [details]
Patch

Attachment 195536 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17320314

[Build Bot]

Comment on attachment 195536 [details]
Patch

Attachment 195536 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17358006

[kov's GTK+ EWS bot]

Comment on attachment 195536 [details]
Patch

Attachment 195536 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17257524

[WebKit Review Bot]

Comment on attachment 195536 [details]
Patch

Attachment 195536 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17322226

New failing tests:
accessibility/aria-checkbox-checked.html
accessibility/aria-labelledby-on-input.html
animations/3d/matrix-transform-type-animation.html
http/tests/appcache/cyrillic-uri.html
animations/animation-add-events-in-handler.html
http/tests/appcache/deferred-events-delete-while-raising.html
canvas/philip/tests/2d.clearRect.globalalpha.html
accessibility/aria-describedby-on-input.html
http/tests/appcache/crash-when-navigating-away-then-back.html
animations/animation-direction-alternate-reverse.html
accessibility/aria-labelledby-overrides-label.html
http/tests/appcache/credential-url.html
http/tests/appcache/access-via-redirect.php
http/tests/appcache/different-scheme.html
accessibility/aria-hidden.html
http/tests/appcache/destroyed-frame.html
http/tests/appcache/deferred-events-delete-while-raising-timer.html
animations/animation-controller-drt-api.html
accessibility/aria-hidden-hides-all-elements.html
animations/3d/transform-perspective.html
accessibility/aria-hidden-updates-alldescendants.html
accessibility/accessibility-node-reparent.html
canvas/philip/tests/2d.canvas.reference.html
animations/3d/state-at-end-event-transform.html
canvas/philip/tests/2d.clearRect.basic.html
accessibility/aria-fallback-roles.html
animations/animation-direction-reverse-fill-mode-hardware.html
accessibility/adjacent-continuations-cause-assertion-failure.html
http/tests/appcache/detached-iframe.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.html

[WebKit Review Bot]

Created attachment 195541 [details]
Archive of layout-test-results from gce-cr-linux-06 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

Created attachment 195542 [details]
Patch

[WebKit Review Bot]

Attachment 195542 [details] did not pass style-queue:

Source/WebCore/page/DeviceController.cpp:54:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/page/DeviceController.cpp:64:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/page/DeviceController.cpp:74:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:40:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:41:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:42:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:47:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:48:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:49:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:50:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:51:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp:39:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp:42:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp:34:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp:37:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:43:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:44:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceMotionController.cpp:46:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:59:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceMotionController.cpp:67:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:71:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:37:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:38:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:39:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:40:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:43:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:45:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:47:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:37:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:38:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:39:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:42:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:45:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:39:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:40:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/DeviceOrientationController.cpp:47:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:48:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceOrientationController.cpp:50:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:59:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:65:  One space before end of line comments  [whitespace/comments] [5]
Source/WebCore/dom/DeviceOrientationController.cpp:65:  ShouldFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platform/chromium/public/Platform.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/dom/DeviceMotionController.cpp', u'Source/WebCore/dom/DeviceMotionController.h', u'Source/WebCore/dom/DeviceOrientationController.cpp', u'Source/WebCore/dom/DeviceOrientationController.h', u'Source/WebCore/history/PageCache.cpp', u'Source/WebCore/inspector/InspectorPageAgent.cpp', u'Source/WebCore/page/DOMWindow.cpp', u'Source/WebCore/page/DOMWindow.h', u'Source/WebCore/page/DeviceController.cpp', u'Source/WebCore/page/DeviceController.h', u'Source/WebCore/platform/PlatformDeviceMotion.h', u'Source/WebCore/platform/PlatformDeviceOrientation.h', u'Source/WebCore/platform/chromium/DeviceMotionChromium.cpp', u'Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp', u'Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp', u'Source/WebCore/platform/qt/DeviceMotionProviderQt.h', u'Source/WebCore/platform/qt/DeviceMotionQt.cpp', u'Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp', u'Source/WebCore/platform/qt/DeviceOrientationProviderQt.h', u'Source/WebCore/platform/qt/DeviceOrientationQt.cpp', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/WebDeviceOrientationController.cpp', u'Source/WebKit/chromium/src/WebViewImpl.cpp']" exit_code: 1
 have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:74:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:79:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 59 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 195542 [details]
Patch

Attachment 195542 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17320323

[kov's GTK+ EWS bot]

Comment on attachment 195542 [details]
Patch

Attachment 195542 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17358014

[Early Warning System Bot]

Comment on attachment 195542 [details]
Patch

Attachment 195542 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17353086

[Early Warning System Bot]

Comment on attachment 195542 [details]
Patch

Attachment 195542 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17320324

[WebKit Review Bot]

Comment on attachment 195542 [details]
Patch

Attachment 195542 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17351141

Created attachment 195548 [details]
Patch

[WebKit Review Bot]

Attachment 195548 [details] did not pass style-queue:

Source/WebCore/page/DeviceController.cpp:54:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/page/DeviceController.cpp:64:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/page/DeviceController.cpp:74:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:40:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:41:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:42:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:47:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:48:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:49:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:50:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp:51:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp:39:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp:42:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp:34:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp:37:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:43:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:44:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceMotionController.cpp:46:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:59:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceMotionController.cpp:67:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceMotionController.cpp:71:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:37:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:38:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:39:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:40:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:43:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:45:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceMotionQt.cpp:47:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:37:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:38:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:39:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:42:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:45:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/qt/DeviceOrientationQt.cpp:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:35:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:36:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:39:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/platform/chromium/DeviceMotionChromium.cpp:40:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/DeviceOrientationController.cpp:47:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:48:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/dom/DeviceOrientationController.cpp:50:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:59:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:65:  One space before end of line comments  [whitespace/comments] [5]
Source/WebCore/dom/DeviceOrientationController.cpp:65:  ShouldFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platform/chromium/public/Platform.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.gypi', u'Source/WebCore/dom/DeviceMotionController.cpp', u'Source/WebCore/dom/DeviceMotionController.h', u'Source/WebCore/dom/DeviceOrientationController.cpp', u'Source/WebCore/dom/DeviceOrientationController.h', u'Source/WebCore/history/PageCache.cpp', u'Source/WebCore/inspector/InspectorPageAgent.cpp', u'Source/WebCore/page/DOMWindow.cpp', u'Source/WebCore/page/DOMWindow.h', u'Source/WebCore/page/DeviceController.cpp', u'Source/WebCore/page/DeviceController.h', u'Source/WebCore/platform/PlatformDeviceMotion.h', u'Source/WebCore/platform/PlatformDeviceOrientation.h', u'Source/WebCore/platform/chromium/DeviceMotionChromium.cpp', u'Source/WebCore/platform/chromium/DeviceOrientationChromium.cpp', u'Source/WebCore/platform/qt/DeviceMotionClientQt.cpp', u'Source/WebCore/platform/qt/DeviceMotionProviderQt.cpp', u'Source/WebCore/platform/qt/DeviceMotionProviderQt.h', u'Source/WebCore/platform/qt/DeviceMotionQt.cpp', u'Source/WebCore/platform/qt/DeviceOrientationClientQt.cpp', u'Source/WebCore/platform/qt/DeviceOrientationProviderQt.cpp', u'Source/WebCore/platform/qt/DeviceOrientationProviderQt.h', u'Source/WebCore/platform/qt/DeviceOrientationQt.cpp', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/WebDeviceOrientationController.cpp', u'Source/WebKit/chromium/src/WebViewImpl.cpp']" exit_code: 1
 have a space between // and comment  [whitespace/comments] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:74:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WebCore/dom/DeviceOrientationController.cpp:79:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 59 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[kov's GTK+ EWS bot]

Comment on attachment 195548 [details]
Patch

Attachment 195548 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17320327

[EFL EWS Bot]

Comment on attachment 195548 [details]
Patch

Attachment 195548 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17358022

[Early Warning System Bot]

Comment on attachment 195548 [details]
Patch

Attachment 195548 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17358025

[WebKit Review Bot]

Comment on attachment 195548 [details]
Patch

Attachment 195548 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17298440

[Early Warning System Bot]

Comment on attachment 195548 [details]
Patch

Attachment 195548 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17329306

Created attachment 195553 [details]
Patch

[Early Warning System Bot]

Comment on attachment 195553 [details]
Patch

Attachment 195553 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17340183

[kov's GTK+ EWS bot]

Comment on attachment 195553 [details]
Patch

Attachment 195553 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17322244

Created attachment 195558 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 195558 [details]
Patch

Attachment 195558 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17353112

[EFL EWS Bot]

Comment on attachment 195558 [details]
Patch

Attachment 195558 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17292412

[Early Warning System Bot]

Comment on attachment 195558 [details]
Patch

Attachment 195558 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17351154

[Early Warning System Bot]

Comment on attachment 195558 [details]
Patch

Attachment 195558 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17320344

Created attachment 195564 [details]
Patch

[EFL EWS Bot]

Comment on attachment 195564 [details]
Patch

Attachment 195564 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17320353

[Early Warning System Bot]

Comment on attachment 195564 [details]
Patch

Attachment 195564 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17311610

[Build Bot]

Comment on attachment 195564 [details]
Patch

Attachment 195564 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17311605

[Early Warning System Bot]

Comment on attachment 195564 [details]
Patch

Attachment 195564 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17329334

[WebKit Review Bot]

Comment on attachment 195564 [details]
Patch

Attachment 195564 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17330332

[Build Bot]

Comment on attachment 195564 [details]
Patch

Attachment 195564 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17353135

[Alexey Proskuryakov]

Comment on attachment 195564 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195564&action=review

What is the goal of this refactoring?

I realize that this is not marked for review yet, but too much in this patch looks questionable.

> Source/WebCore/dom/DeviceMotionController.cpp:43
> +DeviceMotionController* DeviceMotionController::getInstance()

This is wrong style - getXXX functions in WebKit return their results by reference as out arguments.

The common name for this is shared(), and the common way to implement is through DEFINE_STATIC_LOCAL macro.

> Source/WebCore/dom/DeviceMotionController.cpp:51
> +void DeviceMotionController::didChangeDeviceMotion(PassRefPtr<DeviceMotionData> deviceMotionData)

There is no ownership passing here, so it's incorrect to use PassRefPtr. A plain pointer should be used.

The purpose of PassRefPtr is to avoid reference count thrashing. It also gives a little protection against leaks when returning results from functions. It's not the goal to use it everywhere.

> Source/WebCore/dom/DeviceMotionController.cpp:60
> +    // TODO(timvolodine): check how to implement this properly

Are you planning to get rid of this before sending the patch for review? This is not a proper style for FIXME comments in WebKit.

> Source/WebCore/dom/DeviceOrientationController.cpp:54
> +void DeviceOrientationController::didChangeDeviceOrientation(PassRefPtr<DeviceOrientationData> orientation)

Ditto about PassRefPtr.

> Source/WebCore/dom/DeviceOrientationController.cpp:57
> +    LOG_ERROR(WTF::String::number(orientation.get()->alpha()).ascii().data());
> +    LOG_ERROR(WTF::String::number(orientation.get()->canProvideGamma()).ascii().data());

Please don't prefix WTF::.

> Source/WebCore/dom/DeviceOrientationController.cpp:59
> +    // orientation = InspectorInstrumentation::overrideDeviceOrientation(m_page, orientation);

Please remove commented out code before sending a patch for review.

> Source/WebCore/dom/DeviceOrientationController.cpp:66
> +    // TODO(timvolodine): figure out how to implement this
> +    // maybe : (m_lastOrientation.get() != 0);

...

> Source/WebCore/dom/DeviceOrientationController.cpp:70
>  PassRefPtr<Event> DeviceOrientationController::getLastEvent()

Existing code, but this is very wrong, as explained in above comments.

> Source/WebCore/page/DOMWindow.h:411
> +        Page* page();

Did you check why this function was private? I suspect that another idiom could be preferred for getting the page.

> Source/WebCore/page/DeviceController.h:41
> +    explicit DeviceController();

"Explicit" is meaningless with this constructor, now that it has no arguments.

[WebKit Review Bot]

Comment on attachment 195564 [details]
Patch

Attachment 195564 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17235780

New failing tests:
accessibility/aria-checkbox-checked.html
accessibility/aria-labelledby-on-input.html
animations/3d/matrix-transform-type-animation.html
http/tests/appcache/cyrillic-uri.html
animations/animation-add-events-in-handler.html
http/tests/appcache/deferred-events-delete-while-raising.html
canvas/philip/tests/2d.clearRect.globalalpha.html
accessibility/aria-describedby-on-input.html
http/tests/appcache/crash-when-navigating-away-then-back.html
animations/animation-direction-alternate-reverse.html
accessibility/aria-labelledby-overrides-label.html
http/tests/appcache/credential-url.html
http/tests/appcache/access-via-redirect.php
http/tests/appcache/different-scheme.html
accessibility/aria-hidden.html
http/tests/appcache/destroyed-frame.html
http/tests/appcache/deferred-events-delete-while-raising-timer.html
animations/animation-controller-drt-api.html
accessibility/aria-hidden-hides-all-elements.html
animations/3d/transform-perspective.html
accessibility/aria-hidden-updates-alldescendants.html
accessibility/accessibility-node-reparent.html
canvas/philip/tests/2d.canvas.reference.html
animations/3d/state-at-end-event-transform.html
canvas/philip/tests/2d.clearRect.basic.html
accessibility/aria-fallback-roles.html
animations/animation-direction-reverse-fill-mode-hardware.html
accessibility/adjacent-continuations-cause-assertion-failure.html
http/tests/appcache/detached-iframe.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.html

[WebKit Review Bot]

Created attachment 195609 [details]
Archive of layout-test-results from gce-cr-linux-06 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

[Build Bot]

Comment on attachment 195564 [details]
Patch

Attachment 195564 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17345710

Created attachment 196175 [details]
Patch

Comment on attachment 195564 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195564&action=review

Thanks Alexey, I addressed most of your comments, so now the code should look better.

>> Source/WebCore/dom/DeviceMotionController.cpp:43
>> +DeviceMotionController* DeviceMotionController::getInstance()
> 
> This is wrong style - getXXX functions in WebKit return their results by reference as out arguments.
> 
> The common name for this is shared(), and the common way to implement is through DEFINE_STATIC_LOCAL macro.

done

>> Source/WebCore/dom/DeviceMotionController.cpp:51
>> +void DeviceMotionController::didChangeDeviceMotion(PassRefPtr<DeviceMotionData> deviceMotionData)
> 
> There is no ownership passing here, so it's incorrect to use PassRefPtr. A plain pointer should be used.
> 
> The purpose of PassRefPtr is to avoid reference count thrashing. It also gives a little protection against leaks when returning results from functions. It's not the goal to use it everywhere.

done

>> Source/WebCore/dom/DeviceMotionController.cpp:60
>> +    // TODO(timvolodine): check how to implement this properly
> 
> Are you planning to get rid of this before sending the patch for review? This is not a proper style for FIXME comments in WebKit.

done

>> Source/WebCore/dom/DeviceOrientationController.cpp:54
>> +void DeviceOrientationController::didChangeDeviceOrientation(PassRefPtr<DeviceOrientationData> orientation)
> 
> Ditto about PassRefPtr.

done

>> Source/WebCore/dom/DeviceOrientationController.cpp:57
>> +    LOG_ERROR(WTF::String::number(orientation.get()->canProvideGamma()).ascii().data());
> 
> Please don't prefix WTF::.

done

>> Source/WebCore/dom/DeviceOrientationController.cpp:59
>> +    // orientation = InspectorInstrumentation::overrideDeviceOrientation(m_page, orientation);
> 
> Please remove commented out code before sending a patch for review.

done

>> Source/WebCore/dom/DeviceOrientationController.cpp:66
>> +    // maybe : (m_lastOrientation.get() != 0);
> 
> ...

done

>> Source/WebCore/dom/DeviceOrientationController.cpp:70
>>  PassRefPtr<Event> DeviceOrientationController::getLastEvent()
> 
> Existing code, but this is very wrong, as explained in above comments.

don't really see why this is wrong now

>> Source/WebCore/page/DOMWindow.h:411
>> +        Page* page();
> 
> Did you check why this function was private? I suspect that another idiom could be preferred for getting the page.

hmm, at this point not sure how to get to the page from window in any other way.

>> Source/WebCore/page/DeviceController.h:41
>> +    explicit DeviceController();
> 
> "Explicit" is meaningless with this constructor, now that it has no arguments.

done

The latest patch contains the WebCore side of the new device motion/orientation implementation. We decided not to include changes to ports for now, as this would make the patch much larger and difficult to review.

[Early Warning System Bot]

Comment on attachment 196175 [details]
Patch

Attachment 196175 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17352332

[Early Warning System Bot]

Comment on attachment 196175 [details]
Patch

Attachment 196175 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17404029

[EFL EWS Bot]

Comment on attachment 196175 [details]
Patch

Attachment 196175 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17367295

[Build Bot]

Comment on attachment 196175 [details]
Patch

Attachment 196175 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17320682

[WebKit Review Bot]

Comment on attachment 196175 [details]
Patch

Attachment 196175 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17313523

New failing tests:
inspector/device-orientation-success.html
fast/dom/DeviceOrientation/no-synchronous-events.html
fast/dom/DeviceOrientation/basic-operation.html
fast/dom/DeviceOrientation/multiple-frames.html
fast/dom/DeviceOrientation/null-values.html
fast/dom/DeviceOrientation/updates.html
fast/dom/DeviceOrientation/add-listener-from-callback.html

[WebKit Review Bot]

Created attachment 196199 [details]
Archive of layout-test-results from gce-cr-linux-06 for chromium-linux-x86_64

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: chromium-linux-x86_64  Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04

[Build Bot]

Comment on attachment 196175 [details]
Patch

Attachment 196175 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17358457

[Build Bot]

Comment on attachment 196175 [details]
Patch

Attachment 196175 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17385124