RESOLVED FIXED 82386
[Win] Some Blob tests crash in CFNetwork in advanceCurrentStream(FormStreamFields*)
https://bugs.webkit.org/show_bug.cgi?id=82386
Summary [Win] Some Blob tests crash in CFNetwork in advanceCurrentStream(FormStreamFi...
Alexey Proskuryakov
Reported 2012-03-27 15:03:10 PDT
The code in FormDataStreamCFNet.cpp relies on CFNetwork body parts implementation, which doesn't know about blobs. We need the same lower level approach we took in FormDataStreamMac.mm originally. <rdar://problem/11121501>
Attachments
proposed fix (38.31 KB, patch)
2012-03-27 15:08 PDT, Alexey Proskuryakov
buildbot: commit-queue-
build fix attempt (40.24 KB, patch)
2012-03-28 11:31 PDT, Alexey Proskuryakov
buildbot: commit-queue-
more build fix attempt (40.20 KB, patch)
2012-03-28 12:21 PDT, Alexey Proskuryakov
buildbot: commit-queue-
more build fixing (41.03 KB, patch)
2012-03-28 13:44 PDT, Alexey Proskuryakov
buildbot: commit-queue-
moar! (41.69 KB, patch)
2012-03-28 15:08 PDT, Alexey Proskuryakov
buildbot: commit-queue-
moarrrrr! (42.37 KB, patch)
2012-03-28 15:42 PDT, Alexey Proskuryakov
no flags
Alexey Proskuryakov
Comment 1 2012-03-27 15:08:15 PDT
Created attachment 134147 [details] proposed fix
WebKit Review Bot
Comment 2 2012-03-27 15:22:48 PDT
Attachment 134147 [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/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:60: Extra space after ( in function call [whitespace/parens] [4] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:72: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:72: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:104: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:106: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:159: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:189: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:287: One line control clauses should not use braces. [whitespace/braces] [4] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:337: One line control clauses should not use braces. [whitespace/braces] [4] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:400: One line control clauses should not use braces. [whitespace/braces] [4] Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 11 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 3 2012-03-27 15:34:41 PDT
Comment on attachment 134147 [details] proposed fix Attachment 134147 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12148668
Alexey Proskuryakov
Comment 4 2012-03-28 11:31:29 PDT
Created attachment 134351 [details] build fix attempt
WebKit Review Bot
Comment 5 2012-03-28 11:35:48 PDT
Attachment 134351 [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/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "event" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:85: The parameter name "alloc" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:127: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:129: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:182: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:212: Use 0 instead of NULL. [readability/null] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:310: One line control clauses should not use braces. [whitespace/braces] [4] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:422: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 11 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 6 2012-03-28 12:13:40 PDT
Comment on attachment 134351 [details] build fix attempt Attachment 134351 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12156252
Alexey Proskuryakov
Comment 7 2012-03-28 12:21:23 PDT
Created attachment 134366 [details] more build fix attempt
WebKit Review Bot
Comment 8 2012-03-28 12:23:55 PDT
Attachment 134366 [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/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "event" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:85: The parameter name "alloc" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 5 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 9 2012-03-28 12:47:58 PDT
Comment on attachment 134366 [details] more build fix attempt Attachment 134366 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12159246
Alexey Proskuryakov
Comment 10 2012-03-28 13:44:18 PDT
Created attachment 134386 [details] more build fixing
WebKit Review Bot
Comment 11 2012-03-28 13:49:18 PDT
Attachment 134386 [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/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "event" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:85: The parameter name "alloc" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 5 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 12 2012-03-28 13:57:09 PDT
Comment on attachment 134386 [details] more build fixing Attachment 134386 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12152403
Alexey Proskuryakov
Comment 13 2012-03-28 15:08:42 PDT
WebKit Review Bot
Comment 14 2012-03-28 15:13:38 PDT
Attachment 134414 [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/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "event" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:85: The parameter name "alloc" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 5 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 15 2012-03-28 15:35:26 PDT
Alexey Proskuryakov
Comment 16 2012-03-28 15:42:26 PDT
Created attachment 134428 [details] moarrrrr! Wow, so we've been building with broken fileSystemRepresentation all the time.
WebKit Review Bot
Comment 17 2012-03-28 15:45:32 PDT
Attachment 134428 [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/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:84: The parameter name "event" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:85: The parameter name "alloc" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "stream" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp:95: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 5 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brady Eidson
Comment 18 2012-03-28 17:03:03 PDT
Comment on attachment 134428 [details] moarrrrr! Plz to be fixing the style bot gripes
Alexey Proskuryakov
Comment 19 2012-03-28 17:07:35 PDT
Comment on attachment 134428 [details] moarrrrr! Actually, I think that we're keeping all argument names in CF-style code generally.
WebKit Review Bot
Comment 20 2012-03-28 17:59:00 PDT
Comment on attachment 134428 [details] moarrrrr! Clearing flags on attachment: 134428 Committed r112482: <http://trac.webkit.org/changeset/112482>
WebKit Review Bot
Comment 21 2012-03-28 17:59:06 PDT
All reviewed patches have been landed. Closing bug.
Jessie Berlin
Comment 22 2012-03-28 21:09:53 PDT
Attempted Windows build fix in http://trac.webkit.org/changeset/112498. Alexey, you should probably look it over to make sure it is right. I tried to go by the error codes in http://msdn.microsoft.com/en-us/library/5814770t.aspx.
Alexey Proskuryakov
Comment 23 2012-03-28 21:46:38 PDT
What were the build failures? As you can see, EWS built this patch successfully. It is difficult to tell whether using a different error code will affect anything. CFStreamError structure contains a domain and an error code. Domain for OSStatus codes like fnfErr is kCFStreamErrorDomainMacOSStatus (numerically, it's 2), but the existing code doesn't set the domain either. This CFStreamError code is not used immediately, but it's stored inside CFReamStream for later use. I think that it would be better to find a fix that doesn't involve using an unexpected error code. Is there any header in your setup that defines fnfErr? Why does it not have the CoreServices framework?
Jessie Berlin
Comment 24 2012-03-29 08:32:39 PDT
(In reply to comment #23) > What were the build failures? As you can see, EWS built this patch successfully. 6>..\platform\network\cf\FormDataStreamCFNet.cpp(48) : fatal error C1083: Cannot open include file: 'CoreServices/CoreServices.h': No such file or directory When I resolved that by #defining it out on Windows, I ran into fnfErr not found. > > It is difficult to tell whether using a different error code will affect anything. CFStreamError structure contains a domain and an error code. Domain for OSStatus codes like fnfErr is kCFStreamErrorDomainMacOSStatus (numerically, it's 2), but the existing code doesn't set the domain either. This CFStreamError code is not used immediately, but it's stored inside CFReamStream for later use. > > I think that it would be better to find a fix that doesn't involve using an unexpected error code. Is there any header in your setup that defines fnfErr? Why does it not have the CoreServices framework? There was no header on Windows that defines fnfErr. Also, you will notice that all uses of #include <CoreServices/CoreServices.h> are not defined on Windows. I don't see that header on Windows, and I am confused as to how this passed EWS.
Note You need to log in before you can comment on or make changes to this bug.