[webkit-reviews] review denied: [Bug 61205] Web Inspector: Cache XHR content in backend, do not use initialContentSet for XHRs. : [Attachment 94770] Patch with fixes

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied  review:
Bug 61205: Web Inspector: Cache XHR content in backend, do not use
initialContentSet for XHRs.
https://bugs.webkit.org/show_bug.cgi?id=61205

Attachment 94770: Patch with fixes
https://bugs.webkit.org/attachment.cgi?id=94770&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=94770&action=review

> Source/WebCore/inspector/InspectorResourceAgent.h:61
>  class EventsCollector;

Please move EventsCollector up to keep alphabetic order.

> Source/WebCore/inspector/InspectorResourceAgent.h:124
> +    NetworkResourcesData* resourcesData() { return m_resourcesData.get(); }

resourcesData() is not used outside InspectorResourceAgent, so it should be
private or inlined.

> Source/WebCore/inspector/NetworkResourcesData.cpp:94
> +	   resourceData->content.append(content);

It may occur that ensureFreeSpace has just removed resourceData from
m_resourceDataDeque and reset its accumulated content in which case the content
here will be corrupted.

> Source/WebCore/inspector/NetworkResourcesData.h:34
> +#include <wtf/text/StringHash.h>

Remove this include?

> Source/WebCore/inspector/NetworkResourcesData.h:68
> +    ~NetworkResourcesData() { }

style: please move destructor to the next line after the constructor. 

Also, you should delete all values in the destructor. r- for this.