[webkit-reviews] review granted: [Bug 136370] Remove NetworkResourceLoaderClient and subclasses. : [Attachment 237350] fix cmake build

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 136370: Remove NetworkResourceLoaderClient and subclasses.
https://bugs.webkit.org/show_bug.cgi?id=136370

Attachment 237350: fix cmake build
https://bugs.webkit.org/attachment.cgi?id=237350&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=237350&action=review


> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:388
> +#endif // __MAC_OS_X_VERSION_MIN_REQUIRED >= 1090

I don’t think this comment is helpful. It’s gotten out of sync with the #if,
which is one of the main reasons I don’t like comments like this, and it’s only
a few lines away from the #if.

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.h:174
> +    struct SynchronousLoadData {
> +	   SynchronousLoadData(WebCore::ResourceRequest& request,
PassRefPtr<Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::Del
ayedReply> reply)
> +	       : m_originalRequest(request)
> +	       , m_delayedReply(reply)
> +	   {
> +	       ASSERT(m_delayedReply);
> +	   }
> +	   WebCore::ResourceRequest m_originalRequest;
> +	   WebCore::ResourceRequest m_currentRequest;
> +	  
RefPtr<Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::Delayed
Reply> m_delayedReply;
> +	   WebCore::ResourceResponse m_response;
> +	   WebCore::ResourceError m_error;
> +    };

I don’t think we need to put the definition of this struct in the header. We
can compile references to this struct and unique_ptr to this struct without
having the entire class in the header. And it would be nice to not have to
include ResourceError.h and ResourceResponse.h here. Can we forward-declare the
struct here and put the definition in the .cpp file? I believe this can be done
even for a nested struct.