[webkit-reviews] review granted: [Bug 193808] REGRESSION(?): Web Inspector: Can have multiple Timelines selected after edit mode : [Attachment 360312] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Matt Baker <mattbaker at apple.com>'s
request for review:
Bug 193808: REGRESSION(?): Web Inspector: Can have multiple Timelines selected
after edit mode
https://bugs.webkit.org/show_bug.cgi?id=193808

Attachment 360312: Patch

https://bugs.webkit.org/attachment.cgi?id=360312&action=review




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 360312
  --> https://bugs.webkit.org/attachment.cgi?id=360312
Patch

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

r=me, but I think we should write tests for patches that "fix a bug where...",
so I'd like to see a test for this if possible

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:350
> +		  
treeOutline._selectionController.didRemoveItems(removedIndexes);

I'd personally rather have the early-return happen inside `didRemoveItems`. 
Requiring the caller to know that you should only ever pass a non-null
`WI.IndexSet` seems unnecessary, and is likely to be forgotten in the future.

Also, should we also be early-returning if `!removedIndexes.size`?