Bug 29891 – Web Inspector: Introduce inspected object groups for console and watch evaluation results so that they could be released explicitly.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

Currently console and watch evaluation results are wrapped with InspectorController::wrapObject. wrapObject caches the results in the inspector controller. We clean it up upon clearConsole. Originally, wrapObject was only applied to console evaluation results, so that implementation was correct. However, now watch expressions now use same method, while their lifetime does not depend on console. As a result we leak memory on every watch expressions update.

[Pavel Feldman]

Created attachment 40318 [details]
patch

[Timothy Hatcher]

Comment on attachment 40318 [details]
patch


> +    for (Vector<String>::iterator it = groupIt->second.begin(); it != groupIt->second.end(); ++it) {
> +        m_idToWrappedObject.remove(*it);
> +    }

No need for braces here. Does find really return all objects found? I thought it just found the first and returned an iterator for that location for fast removal. I think this code will remove the found object and everything after it.

But I might be wrong. r- for now.

[Pavel Feldman]

(In reply to comment #2)
> (From update of attachment 40318 [details])
> 
> > +    for (Vector<String>::iterator it = groupIt->second.begin(); it != groupIt->second.end(); ++it) {
> > +        m_idToWrappedObject.remove(*it);
> > +    }
> 
> No need for braces here. Does find really return all objects found? I thought
> it just found the first and returned an iterator for that location for fast
> removal. I think this code will remove the found object and everything after
> it.
> 
> But I might be wrong. r- for now.

m_objectGroups contains a mapping from object group id to a Vector of values. The loop here is iterating over groupIt->second which is that vector and releases all the objects from another map (objectId -> ScriptObject one). Then it removes this single entry from m_objectGroups.

[Pavel Feldman]

Created attachment 40592 [details]
patch with review comments addressed

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/bindings/js/JSInspectorBackendCustom.cpp
	M	WebCore/bindings/v8/custom/V8InspectorBackendCustom.cpp
	M	WebCore/inspector/InspectorBackend.cpp
	M	WebCore/inspector/InspectorBackend.h
	M	WebCore/inspector/InspectorBackend.idl
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	M	WebCore/inspector/InspectorFrontend.cpp
	M	WebCore/inspector/front-end/ConsoleView.js
	M	WebCore/inspector/front-end/InjectedScript.js
	M	WebCore/inspector/front-end/ScriptsPanel.js
	M	WebCore/inspector/front-end/WatchExpressionsSidebarPane.js
Committed r49084