[webkit-reviews] review denied: [Bug 51738] Web Inspector: refactoring: encapsulate lazy initialization of SourceFrame : [Attachment 77667] Patch.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 51738: Web Inspector: refactoring: encapsulate lazy initialization of
SourceFrame
https://bugs.webkit.org/show_bug.cgi?id=51738

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

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

Could you please split this into smaller patches? It is really hard to follow
the changes here and I feel there are bugs introduced.

> WebCore/inspector/front-end/ScriptView.js:34
> +    this.sourceFrame.addScript(script.sourceID, script.startingLine);

I did not know SourceFrame is sourceID aware. It probably should not be.

> WebCore/inspector/front-end/SourceFrame.js:869
> +WebInspector.ContentProvider = function()

SourceFrameContentProvider ?

> WebCore/inspector/front-end/SourceFrame.js:914
> +WebInspector.ResourceContentProvider = function(resource)

I thought the whole point of content provider was to isolate SourceFrame from
Script / Resource. Now I see explicit resource usage in SourceFrame.js. This
seems wrong.