[webkit-reviews] review denied: [Bug 51334] Web Inspector: Protocol cleanup task. Introduce Network, DOMStorage and Database domains. : [Attachment 77106] [patch] second iteration

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 51334: Web Inspector: Protocol cleanup task. Introduce Network, DOMStorage
and Database domains.
https://bugs.webkit.org/show_bug.cgi?id=51334

Attachment 77106: [patch] second iteration
https://bugs.webkit.org/attachment.cgi?id=77106&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=77106&action=review

I don't think that ResourceManager -> NetworkManager rename is thought through.
There are two things that do not play well with it: 1) frames aspect and 2)
direct calls from network to various panels (something resource manager was
created with). I think we should leave it as is unless there is a good
separation of network resources and resource panel resources. I am fine with
moving view factories into ResourceView.

> WebCore/inspector/front-end/NetworkManager.js:254
> +    frameDetachedFromParent: function(frameId)

"Network" manager should not be involved with frames.

> WebCore/inspector/front-end/NetworkManager.js:312
> +    _addFramesRecursively: function(framePayload)

Also, does not look good as a part of 'network'.

> WebCore/inspector/front-end/NetworkManager.js:352
> +	   var resource = this.resourceForURL(msg.url);

What does console message have to do with network?