Bug 219231

Summary: Web Inspector: implement Multimap.prototype.take()
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Revised fix none

Description BJ Burg 2020-11-20 17:34:13 PST
Splitting off from some other work.
Comment 1 BJ Burg 2020-11-20 17:36:33 PST
Created attachment 414746 [details]
Patch
Comment 2 Devin Rousso 2020-11-20 17:44:55 PST
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;
```
Comment 3 Joseph Pecoraro 2020-11-20 17:50:08 PST
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?
Comment 4 BJ Burg 2020-11-21 09:59:39 PST
Created attachment 414763 [details]
Revised fix
Comment 5 EWS 2020-11-21 11:09:14 PST
Committed r270149: <https://trac.webkit.org/changeset/270149>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414763 [details].
Comment 6 Radar WebKit Bug Importer 2020-11-21 11:10:21 PST
<rdar://problem/71656735>