Summary: | DocumentParser::appendBytes shouldn't have a "flush" boolean parameter | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||||||||||
Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | darin, dglazkov, eric, rakuco, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Adam Barth
2011-06-11 00:26:40 PDT
Created attachment 96847 [details]
Patch
Comment on attachment 96847 [details] Patch Attachment 96847 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8825631 New failing tests: userscripts/user-script-plugin-document.html Created attachment 96849 [details]
Archive of layout-test-results from ec2-cr-linux-03
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 96847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96847&action=review > Source/WebCore/ChangeLog:10 > + some code in DocumentWriter look less rediculous. ridiculous > Source/WebCore/dom/DecodedDataDocumentParser.cpp:55 > + String renamingData = writer->createDecoderIfNeeded()->flush(); remainingData > Source/WebCore/loader/DocumentWriter.cpp:194 > + if (m_receivedData) > + return; This shows that the name, “received data” is not good for a boolean data member. I read this as “if I received data, then return”, which is nonsense. The data member needs a name like m_hasReceivedData or maybe an even clearer name. Maybe m_hasReceivedFirstData or m_hasReceivedSomeData? > Source/WebCore/loader/DocumentWriter.cpp:204 > + if (length == -1) > + length = strlen(bytes); The result of strlen does not fit into int. If someone passes a string with more than 2^31 bytes in it, we will get the length wrong. Seems possible on a 64-bit system. > Source/WebCore/loader/DocumentWriter.h:54 > - void addData(const char* string, int length = -1, bool flush = false); > + void addData(const char* bytes, int length = -1); The name “string” makes sense given the fact that the function will call strlen if the magic number -1 is used for length. That feature should almost certainly be removed. The caller can call strlen. > That feature should almost certainly be removed. The caller can call strlen.
Yeah. That's next on my list.
Created attachment 96861 [details]
Patch for landing
Comment on attachment 96861 [details] Patch for landing Rejecting attachment 96861 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 Last 500 characters of output: d (64.2%) => Tests to be fixed (1221): 6 DumpRenderTree crashes ( 0.5%) 5 tests timed out ( 0.4%) 53 text diff mismatch ( 4.3%) 326 skipped (26.7%) => Tests that will only be fixed if they crash (WONTFIX) (8114): 1 test timed out ( 0.0%) 110 text diff mismatch ( 1.4%) 7954 skipped (98.0%) Regressions: Unexpected text diff mismatch : (1) userscripts/user-script-plugin-document.html = TEXT Full output: http://queues.webkit.org/results/8830414 Created attachment 96864 [details]
Archive of layout-test-results from ec2-cq-03
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 96866 [details]
Patch for landing
Comment on attachment 96866 [details] Patch for landing Clearing flags on attachment: 96866 Committed r88609: <http://trac.webkit.org/changeset/88609> All reviewed patches have been landed. Closing bug. |