Bug 100344 - [chromium] remove remaining usages of webkit_support from the TestRunner library
Summary: [chromium] remove remaining usages of webkit_support from the TestRunner library
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: jochen
URL:
Keywords:
Depends on: 100486
Blocks:
  Show dependency tree / graph
 
Reported: 2012-10-25 00:28 PDT by jochen
Modified: 2012-10-29 03:50 PDT (History)
3 users (show)

See Also:

Attachments
Patch (27.43 KB, patch)
2012-10-25 00:29 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch for landing (26.22 KB, patch)
2012-10-25 00:54 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch for landing (27.38 KB, patch)
2012-10-25 10:20 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch for landing (27.20 KB, patch)
2012-10-25 10:37 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch for landing, for realz (27.09 KB, patch)
2012-10-25 10:39 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch for landing, for realz (28.58 KB, patch)
2012-10-25 12:27 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch for landing, for realz (28.66 KB, patch)
2012-10-25 12:35 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch (33.60 KB, patch)
2012-10-25 15:00 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch (33.65 KB, patch)
2012-10-25 15:18 PDT, jochen 		no flags Details | Formatted Diff | Diff
Patch (33.65 KB, patch)
2012-10-29 02:55 PDT, jochen 		no flags Details | Formatted Diff | Diff
Show Obsolete (9) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description jochen 2012-10-25 00:28:48 PDT 
[chromium] remove remaining usages of webkit_support from the TestRunner library
Comment 1 jochen 2012-10-25 00:29:54 PDT 
Created attachment 170570 [details]
Patch
Comment 2 Adam Barth 2012-10-25 00:39:45 PDT 
Comment on attachment 170570 [details]
Patch

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

> Tools/DumpRenderTree/chromium/TestRunner/public/WebEventSender.h:67
> +    enum {
> +        VKEY_RETURN   = 0x0D,
> +        VKEY_PRIOR    = 0x21,
> +        VKEY_NEXT     = 0x22,
> +        VKEY_END      = 0x23,
> +        VKEY_HOME     = 0x24,
> +        VKEY_LEFT     = 0x25,
> +        VKEY_UP       = 0x26,
> +        VKEY_RIGHT    = 0x27,
> +        VKEY_DOWN     = 0x28,
> +        VKEY_SNAPSHOT = 0x2C,
> +        VKEY_INSERT   = 0x2D,
> +        VKEY_DELETE   = 0x2E,
> +        VKEY_APPS     = 0x5D,
> +        VKEY_F1       = 0x70,
> +        VKEY_LSHIFT   = 0xA0,
> +        VKEY_RSHIFT   = 0xA1,
> +        VKEY_LCONTROL = 0xA2,
> +        VKEY_RCONTROL = 0xA3,
> +        VKEY_LMENU    = 0xA4,
> +        VKEY_RMENU    = 0xA5,
> +    };

Do these need to be declared in the public header rather than in the implementation somewhere?  I guess you're going to COMPILE_ASSERT them on the Chromium side or something?
Comment 3 jochen 2012-10-25 00:43:50 PDT 
Comment on attachment 170570 [details]
Patch

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

>> Tools/DumpRenderTree/chromium/TestRunner/public/WebEventSender.h:67
>> +    };
> 
> Do these need to be declared in the public header rather than in the implementation somewhere?  I guess you're going to COMPILE_ASSERT them on the Chromium side or something?

yes, that was my intention.

OTOH these values are from the windows API which I think is unlikely to change, so we could just move it to EventSender.cpp, wdyt?
Comment 4 Adam Barth 2012-10-25 00:47:03 PDT 
> OTOH these values are from the windows API which I think is unlikely to change, so we could just move it to EventSender.cpp, wdyt?

Your plan sounds good.
Comment 5 jochen 2012-10-25 00:54:30 PDT 
Created attachment 170575 [details]
Patch for landing
Comment 6 WebKit Review Bot 2012-10-25 05:02:35 PDT 
Comment on attachment 170575 [details]
Patch for landing

Rejecting attachment 170575 [details] from commit-queue.

New failing tests:
plugins/keyboard-events.html
Full output: http://queues.webkit.org/results/14543646
Comment 7 jochen 2012-10-25 10:20:00 PDT 
Created attachment 170682 [details]
Patch for landing
Comment 8 jochen 2012-10-25 10:20:57 PDT 
Comment on attachment 170682 [details]
Patch for landing

It turns out that it's not enough to translate the windows keysyms to gdk/x11 keysyms, but they need to be translated to hardware keycodes.

Adam, still ok?
Comment 9 WebKit Review Bot 2012-10-25 10:23:32 PDT 
Attachment 170682 [details] did not pass style-queue:

Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:285:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:286:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:287:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:288:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:289:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:290:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:291:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:292:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:293:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:294:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:295:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:296:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:297:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:298:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:299:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:300:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:301:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:302:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:303:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:304:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:305:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:306:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:307:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:308:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:309:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:310:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:313:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:314:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:315:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:316:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:317:  One space before end of line comments  [whitespace/comments] [5]
ToolFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/DumpRenderTree/c..." exit_code: 1
s/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:318:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:319:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:320:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:321:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:322:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:326:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:328:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 38 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 jochen 2012-10-25 10:37:21 PDT 
Created attachment 170688 [details]
Patch for landing
Comment 11 jochen 2012-10-25 10:39:07 PDT 
Created attachment 170689 [details]
Patch for landing, for realz
Comment 12 jochen 2012-10-25 10:47:41 PDT 
Comment on attachment 170689 [details]
Patch for landing, for realz

hum, not good yet
Comment 13 jochen 2012-10-25 10:51:03 PDT 
e.g. third_party/WebKit/LayoutTests/editing/input/reveal-caret-of-multiline-input.html sends a '>'.

I guess I'll just add a table for all US-ASCII chars.
Comment 14 jochen 2012-10-25 12:27:38 PDT 
Created attachment 170710 [details]
Patch for landing, for realz
Comment 15 WebKit Review Bot 2012-10-25 12:30:20 PDT 
Attachment 170710 [details] did not pass style-queue:

Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:281:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:282:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:283:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:284:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:285:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:286:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:287:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:288:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:291:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:292:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:293:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:295:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:296:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:300:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:302:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:303:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:304:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:305:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:306:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:307:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:308:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:309:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:310:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:311:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:312:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:314:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:315:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:316:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:317:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:318:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:319:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:320:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:321:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:322:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:323:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:324:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:325:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:326:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:327:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:328:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:339:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:340:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:341:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:342:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:343:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:344:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:345:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:372:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:373:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:374:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/sFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/DumpRenderTree/c..." exit_code: 1
rc/EventSender.cpp:375:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:376:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:405:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:406:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:407:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:408:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 56 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 jochen 2012-10-25 12:35:31 PDT 
Created attachment 170712 [details]
Patch for landing, for realz
Comment 17 WebKit Review Bot 2012-10-25 12:43:57 PDT 
Attachment 170712 [details] did not pass style-queue:

Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:313:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:322:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:324:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:325:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:329:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:330:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:331:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:332:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:333:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:334:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:335:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:336:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:337:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:338:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:339:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:340:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:341:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:342:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:343:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:344:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:345:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:346:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:347:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:348:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:349:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:350:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:351:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:352:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:353:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:354:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:355:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:356:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:357:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:358:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:359:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:360:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:361:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:362:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:363:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:364:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:365:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:366:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:367:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:368:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:369:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:370:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:371:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:375:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:376:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:377:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:378:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:379:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:380:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:381:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:382:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:383:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:384:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:385:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:386:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:387:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:388:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:389:  One space before end of line comments  [whitespace/comments] [5]
Tools/DuFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/DumpRenderTree/c..." exit_code: 1
mpRenderTree/chromium/TestRunner/src/EventSender.cpp:390:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:391:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:392:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:393:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:394:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:395:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:396:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:397:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:398:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:399:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:400:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:401:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:402:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:403:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:404:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:405:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:406:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:407:  One space before end of line comments  [whitespace/comments] [5]
Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:408:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 81 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 jochen 2012-10-25 12:45:48 PDT 
the formatting the style bot wants looks just ugly :(
Comment 19 Tony Chang 2012-10-25 13:28:35 PDT 
Comment on attachment 170712 [details]
Patch for landing, for realz

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

> Tools/DumpRenderTree/chromium/TestRunner/src/EventSender.cpp:282
> +#if OS(LINUX) && USE(GTK)
> +int NativeKeyCodeForWindowsKeyCode(int keysym)
> +{
> +    // See /usr/share/X11/xkb/keycodes/*
> +    static const int asciiToKeyCode[] = {
> +        0,
> +        0,

Can we put this table in a separate .h and .cpp file?
Comment 20 jochen 2012-10-25 15:00:30 PDT 
Created attachment 170737 [details]
Patch
Comment 21 Tony Chang 2012-10-25 15:07:19 PDT 
Comment on attachment 170737 [details]
Patch

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

> Tools/DumpRenderTree/chromium/TestRunner/src/KeyCodeMapping.cpp:242
> +    return 0;

You probably need an UNUSED_PARAM(keysym) for this to compile on Mac clang.
Comment 22 jochen 2012-10-25 15:18:45 PDT 
Created attachment 170742 [details]
Patch
Comment 23 WebKit Review Bot 2012-10-25 18:51:17 PDT 
Comment on attachment 170742 [details]
Patch

Clearing flags on attachment: 170742

Committed r132552: <http://trac.webkit.org/changeset/132552>
Comment 24 WebKit Review Bot 2012-10-25 18:51:22 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 25 WebKit Review Bot 2012-10-26 02:44:48 PDT 
Re-opened since this is blocked by bug 100486
Comment 26 jochen 2012-10-29 02:55:53 PDT 
Created attachment 171186 [details]
Patch
Comment 27 WebKit Review Bot 2012-10-29 03:50:47 PDT 
Comment on attachment 171186 [details]
Patch

Clearing flags on attachment: 171186

Committed r132781: <http://trac.webkit.org/changeset/132781>
Comment 28 WebKit Review Bot 2012-10-29 03:50:52 PDT 
All reviewed patches have been landed.  Closing bug.