WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
62499
DocumentParser::appendBytes shouldn't have a "flush" boolean parameter
https://bugs.webkit.org/show_bug.cgi?id=62499
Summary
DocumentParser::appendBytes shouldn't have a "flush" boolean parameter
Adam Barth
Reported
2011-06-11 00:26:40 PDT
DocumentParser::appendBytes shouldn't have a "flush" boolean parameter
Attachments
Patch
(10.69 KB, patch)
2011-06-11 00:30 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cr-linux-03
(1.58 MB, application/zip)
2011-06-11 00:50 PDT
,
WebKit Review Bot
no flags
Details
Patch for landing
(14.22 KB, patch)
2011-06-11 14:57 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cq-03
(1.51 MB, application/zip)
2011-06-11 15:42 PDT
,
WebKit Review Bot
no flags
Details
Patch for landing
(14.26 KB, patch)
2011-06-11 17:12 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Adam Barth
Comment 1
2011-06-11 00:30:34 PDT
Created
attachment 96847
[details]
Patch
WebKit Review Bot
Comment 2
2011-06-11 00:50:10 PDT
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
WebKit Review Bot
Comment 3
2011-06-11 00:50:15 PDT
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
Darin Adler
Comment 4
2011-06-11 09:03:02 PDT
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.
Adam Barth
Comment 5
2011-06-11 11:56:20 PDT
> That feature should almost certainly be removed. The caller can call strlen.
Yeah. That's next on my list.
Adam Barth
Comment 6
2011-06-11 14:57:06 PDT
Created
attachment 96861
[details]
Patch for landing
WebKit Review Bot
Comment 7
2011-06-11 15:42:44 PDT
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
WebKit Review Bot
Comment 8
2011-06-11 15:42:49 PDT
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
Adam Barth
Comment 9
2011-06-11 17:12:31 PDT
Created
attachment 96866
[details]
Patch for landing
WebKit Review Bot
Comment 10
2011-06-11 17:51:24 PDT
Comment on
attachment 96866
[details]
Patch for landing Clearing flags on attachment: 96866 Committed
r88609
: <
http://trac.webkit.org/changeset/88609
>
WebKit Review Bot
Comment 11
2011-06-11 17:51:34 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.
Top of Page
Format For Printing
XML
Clone This Bug