Created attachment 68360 [details] intial proposal Webkit's QNRHandler.cpp does not set up Content-Length and QNetworkRequest::DoNotBufferUploadDataAttribute and eventually forces QNetworkReplyImpl to buffer up the data; This is not fine for uploading large files, since systems will run out of buffer memory Attached is a patch for fixing the same The patch is not formatted yet, approach needs to be decided yet https://lists.webkit.org/pipermail/webkit-qt/2010-September/000870.html
Attachment 68360 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 Total errors found: 0 in 0 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 68360 [details] intial proposal View in context: https://bugs.webkit.org/attachment.cgi?id=68360&action=review r- for compilation-breaking typo and missing ChangeLog entry. Please see http://trac.webkit.org/wiki/QtWebKitContrib for information on how to make patches for WebKit, specifically the "Submitting a patch" section. > QNetworkReplyHandler.h.new:100 > + qint64 getFormDataSize() { return m_totalSize; } This method should be const. > QNetworkReplyHandler.cpp.new:70 > + m_totalSize=computeSize(); Coding style, should be "m_totalSize = computeSize();" > QNetworkReplyHandler.cpp.new:81 > + for (int i=0; i<m_formElements.size(); i++) Coding style, should be "for (int i = 0; i < m_formElements.size(); ++i)" > QNetworkReplyHandler.cpp.new:84 > + if (element.m_type == FormDataElement::data) { Coding style, shouldn't have { > QNetworkReplyHandler.cpp.new:85 > + m_dataSize+=element.m_data.size(); Coding style, should be "m_dataSize += element.m_data.size();" > QNetworkReplyHandler.cpp.new:88 > + m_fileSize+=fi.size(); Coding style, should be "m_fileSize += fi.size();" > QNetworkReplyHandler.cpp.new:94 > + return (m_dataSize + m_fileSize); Unnecessary parentheses. > QNetworkReplyHandler.cpp.new:482 > + //We may be uploading files so prevent QNR from buffering data Coding style, please leave a space after // > QNetworkReplyHandler.cpp.new:483 > + m_request.setHeader(QNetworkRequest::ContentLengthHeader, posdDevice->getFormDataSize()); Typo, "posdDevice" should be "postDevice" > QNetworkReplyHandler.cpp.new:494 > + //We may be uploading files so prevent QNR from buffering data Coding style, please leave a space after // > QNetworkReplyHandler.cpp.new:495 > + m_request.setHeader(QNetworkRequest::ContentLengthHeader, posdDevice->getFormDataSize()); Typo, "posdDevice" should be "postDevice"
Created attachment 69442 [details] Addressed style and typo errors of previous patch thanks for the review comments. Hope this one gets in
Attachment 69442 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/platform/network/qt/QNetworkReplyHandler.cpp:81: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 69442 [details] did not build on qt: Build output: http://queues.webkit.org/results/4159037
Created attachment 69445 [details] Fix style and EWS errors Hope this passes style and EWS
Comment on attachment 69445 [details] Fix style and EWS errors View in context: https://bugs.webkit.org/attachment.cgi?id=69445&action=review Close, but not quite there yet. > WebCore/platform/network/qt/QNetworkReplyHandler.cpp:88 > + if (element.m_type == FormDataElement::data) > + m_dataSize += element.m_data.size(); > + else { > + QFileInfo fi(element.m_filename); > + m_fileSize += fi.size(); > + } If building with ENABLE(BLOB), this fails to take FormDataElement::encodedBlob into account. (see FormData.h) > WebCore/platform/network/qt/QNetworkReplyHandler.cpp:92 > +#ifdef QT_NETWORK_DEBUG > + qDebug() << __PRETTY_FUNCTION__ << "FileSize=" << m_fileSize << "DataSize=" << m_dataSize; > +#endif Please remove this. This kind of output should only be added for things that are non-trivial to do using a debugger. > WebCore/platform/network/qt/QNetworkReplyHandler.h:100 > + qint64 getFormDataSize() const { return m_totalSize; } You don't need the m_totalSize member, just "return m_dataSize + m_fileSize;"
Created attachment 69454 [details] Removed m_totalSize and Debug statement Did not address BLOB support. formdataiodevice does not support BLOB yet. Will add another bug for the same.
Comment on attachment 69454 [details] Removed m_totalSize and Debug statement Very nice, r=me
Comment on attachment 69454 [details] Removed m_totalSize and Debug statement Clearing flags on attachment: 69454 Committed r69064: <http://trac.webkit.org/changeset/69064>
All reviewed patches have been landed. Closing bug.
Revision r69064 cherry-picked into qtwebkit-2.1.x with commit 2bb9aaf <http://gitorious.org/webkit/qtwebkit/commit/2bb9aaf>