[webkit-reviews] review denied: [Bug 69146] Web Inspector: debuggerPresentatioModel.linkifyLocation leaks updateAnchor closure instances. : [Attachment 109477] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 69146: Web Inspector: debuggerPresentatioModel.linkifyLocation leaks
updateAnchor closure instances.
https://bugs.webkit.org/show_bug.cgi?id=69146

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

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


> Source/WebCore/WebCore.gypi:6237
> +	       'inspector/front-end/Linkifier.js',

Do not forget about .vcproj!

> Source/WebCore/inspector/front-end/EventListenersSidebarPane.js:59
> +    WebInspector.EventListenersSidebarPane._linkifier = new
WebInspector.Linkifier();

you should install these on instance. Installing on class in constructor does
not make sense - it leaks linkifier anyways.

> Source/WebCore/inspector/front-end/Linkifier.js:46
> +	   var rawSourceCode =
WebInspector.debuggerPresentationModel._rawSourceCodeForScript(sourceURL);

Extracting linkifier in its own class does not remote dependency from the DPM.
Not sure what you were trying to achieve here.

> Source/WebCore/inspector/front-end/ProfileDataGridTree.js:101
> +	       var urlElement =
WebInspector.ProfilesPanel.linkifier.linkifyLocation(this.profileNode.url,
lineNumber, 0, "profile-node-file");

Why not to create linkifier on the tree instead?

> Source/WebCore/inspector/front-end/TimelinePanel.js:112
> +    WebInspector.TimelinePanel._linkifier = new WebInspector.Linkifier();

ditto