Bug 219231 – Web Inspector: implement Multimap.prototype.take()

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Blaze Burg]

Splitting off from some other work.

[Blaze Burg]

Created attachment 414746 [details]
Patch

[Devin Rousso]

Comment on attachment 414746 [details]
Patch

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

r=me, with some behavior changes

> Source/WebInspectorUI/UserInterface/Base/Multimap.js:94
> +            return new Set;

this should be `undefined` to match what `get` does when the `key` is not in `_map`

> Source/WebInspectorUI/UserInterface/Base/Multimap.js:101
> +        let taken = valueSet.take(value);
> +
> +        if (!valueSet.size)
> +            this._map.delete(key);
> +
> +        return new Set([taken]);

this should match `Set.prototype.take`
```
    let result = valueSet.take(value);
    if (!valueSet.size)
        this._map.delete(key);
    return result;
```

[Joseph Pecoraro]

Comment on attachment 414746 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Base/Utilities.js:187
> -        let exists = this.has(key);
> -        if (exists)
> +        if (this.has(key)) {
>              this.delete(key);
> -        return exists;
> +            return key;
> +        }
> +
> +        return undefined;
>      }

We should remove `Set.prototype.take`, it serves no purpose.

The way this was originally written seems to be identical to `Set.prototype.delete`.

The way this is no written is weird for falsey values (E.g. `if (set.take(0)) { ... }`).

Lets just remove it and use `Set.prototype.delete` as appropriate?

[Blaze Burg]

Created attachment 414763 [details]
Revised fix

[EWS]

Committed r270149: <https://trac.webkit.org/changeset/270149>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414763 [details].

[Radar WebKit Bug Importer]

<rdar://problem/71656735>