[webkit-reviews] review granted: [Bug 102955] Web Inspector: Allow sorting in NMI snapshot grid view : [Attachment 175468] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has granted Alexei Filippov
<alph at chromium.org>'s request for review:
Bug 102955: Web Inspector: Allow sorting in NMI snapshot grid view
https://bugs.webkit.org/show_bug.cgi?id=102955

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

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=175468&action=review


> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:66
> +    this._totalNode = totalNode;

You will need to rebase.

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:73
> +	   this._storedState = {};

_storedState -> _expandedNodes ? also you can pass the map as a parameter to
_storeState/_restoreState

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:134
> +    get data()

Please use function instead of getter.

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:141
> +	   if (!this.expanded) return;

style nit: return on the next line

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:149
> +	   if (!this.dataGrid._storedState[this.uid]) return;

style nit: return on the next line

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:158
> +    get uid()

We recently decided to prefer function to getters/setters. Please use uid:
function() here.