Now the idea is to make all signals that come from the QNetworkReply to pass through a queue and to stop that queue when loading is deferred.
This way almost all the deferred logic can be removed from QNetworkReplyHandler class and encapsulated in its own class.
Created attachment 86877 [details]
I will not set the review flag because this patch depends on the patch in bug 57049 and all bots would be red.
But please review it.
- Please explain why Lock is needed in QNetworkReplyWrapper::receiveMetaData, in a comment as well would be great.
- m_queue->setDeferSignals(m_queue->deferSignals()) could cause surprises if more logic is added to setDeferSignals. Deferring bugs easily slip through tests and some explicit flush() method call would be safer.
- Unless I'm missing something, you shouldn't need the inline keyword at the declaration of receiveReadyRead
Created attachment 89232 [details]
Attachment 89232 [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/qt/QNetworkReplyHandler.h:42: The parameter name "handler" adds no information, so it should be removed. [readability/parameter_name] 
Source/WebCore/platform/network/qt/QNetworkReplyHandler.h:62: The parameter name "queue" adds no information, so it should be removed. [readability/parameter_name] 
Total errors found: 2 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 89232 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=89232&action=review
> + void readyRead();
Created attachment 89234 [details]
Comment on attachment 89234 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=89234&action=review
r=me. This is a great step towards making this spaghetti understandable without spending 2 hours reading it. :)
> + void readyRead();
didReceiveReadyRead() or something like that would sound more slot-like.
(In reply to comment #8)
> M Source/WebCore/ChangeLog
> M Source/WebCore/platform/network/qt/QNetworkReplyHandler.cpp
> M Source/WebCore/platform/network/qt/QNetworkReplyHandler.h
> M Source/WebCore/platform/network/qt/ResourceHandleQt.cpp
> Committed r83634
It was rolled out by http://trac.webkit.org/changeset/83673,
because it made two layout tests time out:
I don't see why It should be part of 2.1.x. It's a huge refactoring with some other patches and has no purpose to be in 2.1.x.