Bug 91532

Summary: [Chromium] Add --encode-binary command line option for DRT
Product: WebKit Reporter: Xianzhu Wang <wangxianzhu>
Component: Tools / TestsAssignee: Xianzhu Wang <wangxianzhu>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 86927    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Xianzhu Wang
Reported 2012-07-17 12:02:39 PDT
Now chromium-android requires DRT to output binary data base64-encoded because 'adb shell' can't handle binary outputs. For now the base64 encoding is hard-coded in TestEventPrinter.cpp. #if OS(ANDROID) // On Android, the layout test driver needs to read the image data through 'adb shell' which can't // handle binary data properly. Need to encode the binary data into base64. // FIXME: extract this into a function so that we can also use it to output audio data. Will do when removing test_shell mode. Fix the FIXME.
Attachments
Patch (11.06 KB, patch)
2012-07-17 14:58 PDT, Xianzhu Wang 		no flags
DetailsFormatted DiffDiff
Patch (13.38 KB, patch)
2012-07-17 15:34 PDT, Xianzhu Wang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Xianzhu Wang
Comment 1 2012-07-17 14:58:38 PDT
Created attachment 152839 [details] Patch
Adam Barth
Comment 2 2012-07-17 15:04:33 PDT
Comment on attachment 152839 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152839&action=review > Tools/DumpRenderTree/chromium/TestEventPrinter.cpp:39 > +TestEventPrinter::TestEventPrinter(bool encodeBinary) I probably would have just made this a setter on TestEventPrinter rather than a constructor argument.
Xianzhu Wang
Comment 3 2012-07-17 15:34:26 PDT
Created attachment 152852 [details] Patch
Xianzhu Wang
Comment 4 2012-07-17 15:35:10 PDT
Comment on attachment 152839 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152839&action=review >> Tools/DumpRenderTree/chromium/TestEventPrinter.cpp:39 >> +TestEventPrinter::TestEventPrinter(bool encodeBinary) > > I probably would have just made this a setter on TestEventPrinter rather than a constructor argument. Agreed. Done.
Adam Barth
Comment 5 2012-07-17 15:37:30 PDT
Comment on attachment 152852 [details] Patch Much nicer. Thank you.
WebKit Review Bot
Comment 6 2012-07-17 17:21:58 PDT
Comment on attachment 152852 [details] Patch Clearing flags on attachment: 152852 Committed r122901: <http://trac.webkit.org/changeset/122901>
WebKit Review Bot
Comment 7 2012-07-17 17:22:03 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.