[webkit-reviews] review granted: [Bug 119541] Web Inspector: Use granular DOMStorage modification events to avoid complete DataGrid update. : [Attachment 208311] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Vivek Galatage
<vivekg at webkit.org>'s request for review:
Bug 119541: Web Inspector: Use granular DOMStorage modification events to avoid
complete DataGrid update.
https://bugs.webkit.org/show_bug.cgi?id=119541

Attachment 208311: Patch
https://bugs.webkit.org/attachment.cgi?id=208311&action=review

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=208311&action=review


Much nicer! This look great. You may want to remove that one extra line before
landing.

> Source/WebInspectorUI/UserInterface/DOMStorageContentView.js:80
> +	   this._dataGrid.insertChild(childNode, this._dataGrid.children.length
- 1);
> +	   if (this._dataGrid.sortOrder)
> +	       this._sortDataGrid();

In the future this could insert at sorted index, and be a binary search +
insertion O(log n) instead of a sort O(n * log n). But this is probably good
enough for now. I don't think there are too many sites with many localStorage
values.

> Source/WebInspectorUI/UserInterface/DOMStorageContentView.js:168
> +	   this._dataGridSorted = true;

This new property seems unnecessary. Nothing checks it and it didn't exist
before. Looks like you can remove this line.