Bug 69117 - [chromium] Add WebKit API for sending input events to the compositor thread
Summary: [chromium] Add WebKit API for sending input events to the compositor thread
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-29 18:33 PDT by James Robinson
Modified: 2011-09-30 00:04 PDT (History)
7 users (show)

See Also:


Attachments
Patch (34.68 KB, patch)
2011-09-29 18:41 PDT, James Robinson
no flags Details | Formatted Diff | Diff
with a #define to make a 2-sided patch easier (34.71 KB, patch)
2011-09-29 18:43 PDT, James Robinson
no flags Details | Formatted Diff | Diff
put HAVE_WEBCOMPOSITOR ifdef somewhere useful (34.79 KB, patch)
2011-09-29 19:05 PDT, James Robinson
no flags Details | Formatted Diff | Diff
Patch (34.79 KB, patch)
2011-09-29 19:25 PDT, James Robinson
fishd: review+
fishd: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2011-09-29 18:33:21 PDT
[chromium] Add WebKit API for sending input events to the compositor thread
Comment 1 James Robinson 2011-09-29 18:41:06 PDT
Created attachment 109234 [details]
Patch
Comment 2 James Robinson 2011-09-29 18:42:20 PDT
With this patch the unit tests all pass and as many layout tests pass as did before with use_threaded_compositing=1.  The actual browser hookup of the thread isn't done and will require a chromium change.
Comment 3 James Robinson 2011-09-29 18:43:27 PDT
Created attachment 109235 [details]
with a #define to make a 2-sided patch easier
Comment 4 WebKit Review Bot 2011-09-29 18:43:33 PDT
Attachment 109234 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/tests/CCThreadTest.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/chromium/src/WebCompositorImpl.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 James Robinson 2011-09-29 18:44:48 PDT
(In reply to comment #4)
> Attachment 109234 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
> 
> Source/WebKit/chromium/tests/CCThreadTest.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
> Source/WebKit/chromium/src/WebCompositorImpl.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
> Total errors found: 2 in 16 files
> 

I can't tell what sort order this script is expected - I ran the lines through sort(1) and they look fine to me.
Comment 6 WebKit Review Bot 2011-09-29 18:46:48 PDT
Attachment 109235 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/tests/CCThreadTest.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/chromium/src/WebCompositorImpl.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 James Robinson 2011-09-29 19:05:22 PDT
Created attachment 109238 [details]
put HAVE_WEBCOMPOSITOR ifdef somewhere useful
Comment 8 James Robinson 2011-09-29 19:05:36 PDT
Chromium side is here: http://codereview.chromium.org/8081014/
Comment 9 WebKit Review Bot 2011-09-29 19:07:36 PDT
Attachment 109238 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/tests/CCThreadTest.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/chromium/src/WebCompositorImpl.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Shawn Singh 2011-09-29 19:09:28 PDT
I think you can at the very least separate the "offending" includes by an extra newline.   I think the style check only checks sorting for "clusters of includes", where each cluster is separated by a newline.

but, i could be wrong...
Comment 11 James Robinson 2011-09-29 19:25:34 PDT
Created attachment 109240 [details]
Patch
Comment 12 WebKit Review Bot 2011-09-29 19:27:58 PDT
Attachment 109240 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/tests/CCThreadTest.cpp:32:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/chromium/src/WebCompositorImpl.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Darin Fisher (:fishd, Google) 2011-09-29 22:56:29 PDT
Comment on attachment 109240 [details]
Patch

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

> Source/WebKit/chromium/public/WebCompositor.h:53
> +#endif // WebCompositor_h

ditto

> Source/WebKit/chromium/public/WebCompositorClient.h:43
> +#endif // WebCompositorClient_h

style-nit: I mistyped this... the style guide suggests putting a comment on the close
of the namespace, not on the #endif.  i just mixed that up when i was typing this file
earlier.  the other headers in WebKit/chromium/public/ do it that way.

>> Source/WebKit/chromium/tests/CCThreadTest.cpp:32
>> +#include "CCThreadImpl.h"
> 
> Alphabetical sorting problem.  [build/include_order] [4]

Perhaps this is saying that "CC" sorts before "cc"??
Comment 14 James Robinson 2011-09-30 00:04:28 PDT
Committed r96392: <http://trac.webkit.org/changeset/96392>