Bug 74084 – Web Inspector: Introduce a Map class allowing to store values indexed by arbitrary objects.

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

[Vsevolod Vlasov]

Introduce a Map class allowing to store values indexed by arbitrary objects.

[Vsevolod Vlasov]

Created attachment 118381 [details]
Patch

[Pavel Feldman]

Comment on attachment 118381 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118381&action=review

> Source/WebCore/inspector/front-end/Map.js:49
> +            key[this._mapFieldName] = this._lastObjectIdentifier++;

Why would you need such complexity? Why lastObjectIdentifier can't be static and keys have the same identifiers in all the maps? Also, the pattern for this kind of code it

var objectIdentifier = key[name];
if (!objectIdentifier) {
    objectIdentifier = ++this.lastObjectIdentifier;
    key[name] = objectIdentifier
}

but as I mentioned, you should not need it.

> Source/WebCore/inspector/front-end/inspector.html:42
> +    <script type="text/javascript" src="treeoutline.js"></script>

treeoutline and utilities.s were considered reusable utilities with no WebInspector* dependency. I don't think we should change that.

> Source/WebCore/inspector/front-end/treeoutline.js:-41
> -    this._knownTreeElements = [];

Just leave this as it or define Map in the utilities.js with no WebInspector prefix.

[Timothy Hatcher]

Comment on attachment 118381 [details]
Patch

What is the motivation for this change?

[Vsevolod Vlasov]

Created attachment 118433 [details]
Patch

[Vsevolod Vlasov]

(In reply to comment #3)
> (From update of attachment 118381 [details])
> What is the motivation for this change?

The Map class is added because I am going to need it the new scripts panel navigator I am developing to map script to a tree elements that is representing it. Note, that I am going to have several TreeOutlines there (scripts, content scripts, later snippets), so using tree outline's findTreeElement is not convenient.

Similar functionality is currently used in scripts panel when we set file select options as a field on UISourceCode. This is making code hard to understand and can be abstracted out using this Map class.

Moving TreeOutline to use Map once we have it seems natural to me.

[Vsevolod Vlasov]

(In reply to comment #2)
> (From update of attachment 118381 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=118381&action=review
> 
> > Source/WebCore/inspector/front-end/Map.js:49
> > +            key[this._mapFieldName] = this._lastObjectIdentifier++;
> 
> Why would you need such complexity? Why lastObjectIdentifier can't be static and keys have the same identifiers in all the maps? Also, the pattern for this kind of code it

It was done to make it possible to remove identifiers from an object once object is not a key in any map anymore, but you are right, this seems redundant.

[Vsevolod Vlasov]

Committed r102447: <http://trac.webkit.org/changeset/102447>