[webkit-reviews] review canceled: [Bug 40063] Web Inspector: make SourceFrame BreakpointManager's listener : [Attachment 57791] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has canceled Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 40063: Web Inspector: make SourceFrame BreakpointManager's listener
https://bugs.webkit.org/show_bug.cgi?id=40063

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
WebCore/inspector/front-end/ScriptsPanel.js:279
 +		if (script._scriptView) {
Nit: encapsulate this into script.disposeView();

WebCore/inspector/front-end/ScriptsPanel.js:448
 +			object._scriptView.willRemoveScript();
Why is this needed?

WebCore/inspector/front-end/SourceFrame.js:198
 +	    if (!(breakpoint.sourceID in this._scriptIds)) {
So you ended up with making SourceFrame both sourceID and sourceURL aware
without external binding. It might be simple, not sure it is good design-wise
though.

WebCore/inspector/front-end/SourceView.js:129
 +		this.sourceFrame.addBreakpointsForURL(resource.sourceURL);
I thought the point was to make SourceFrame breakpoint manager's listener so
that breakpoints are not populated from outside.

WebCore/inspector/front-end/SourceView.js:240
 +		this.localSourceFrame = new
WebInspector.SourceFrame(this.localContentElement, null, null, null, null);
just pass single parameter.