RESOLVED FIXED 105524
Web Inspector: extract native heap graph representation into a separate file
https://bugs.webkit.org/show_bug.cgi?id=105524
Summary Web Inspector: extract native heap graph representation into a separate file
Yury Semikhatsky
Reported 2012-12-20 03:25:25 PST
NativeHeapSnapshotView.js should contain only code related to the native memory profiler UI.
Attachments
Patch (26.99 KB, patch)
2012-12-20 03:27 PST, Yury Semikhatsky
no flags
Patch (27.30 KB, patch)
2012-12-20 04:18 PST, Yury Semikhatsky
no flags
Patch (27.57 KB, patch)
2012-12-20 04:26 PST, Yury Semikhatsky
no flags
Patch (28.15 KB, patch)
2012-12-20 04:33 PST, Yury Semikhatsky
apavlov: review+
apavlov: commit-queue+
Yury Semikhatsky
Comment 1 2012-12-20 03:27:23 PST
Alexander Pavlov (apavlov)
Comment 2 2012-12-20 04:04:41 PST
Comment on attachment 180315 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180315&action=review > Source/WebCore/inspector/front-end/NativeHeapGraph.js:110 > +WebInspector.NativeHeapGraphEdge = function(graph, position) AFAIK our current approach is that we should have types nested into the top-level type (WI.NativeHeapGraph.Edge). Should ask pfeldman to be absolutely sure if you want to leave it this way. > Source/WebCore/inspector/front-end/NativeHeapGraph.js:153 > +WebInspector.NativeHeapGraphNode = function(graph, position) Ditto
Yury Semikhatsky
Comment 3 2012-12-20 04:18:15 PST
Yury Semikhatsky
Comment 4 2012-12-20 04:19:20 PST
(In reply to comment #2) > (From update of attachment 180315 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=180315&action=review > > > Source/WebCore/inspector/front-end/NativeHeapGraph.js:110 > > +WebInspector.NativeHeapGraphEdge = function(graph, position) > > AFAIK our current approach is that we should have types nested into the top-level type (WI.NativeHeapGraph.Edge). Should ask pfeldman to be absolutely sure if you want to leave it this way. > Done. What is the advantage of that naming scheme over the old one? > > Source/WebCore/inspector/front-end/NativeHeapGraph.js:153 > > +WebInspector.NativeHeapGraphNode = function(graph, position) > > Ditto Done.
Yury Semikhatsky
Comment 5 2012-12-20 04:26:46 PST
Yury Semikhatsky
Comment 6 2012-12-20 04:33:11 PST
Alexander Pavlov (apavlov)
Comment 7 2012-12-20 04:35:35 PST
(In reply to comment #4) > (In reply to comment #2) > > (From update of attachment 180315 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=180315&action=review > > > > > Source/WebCore/inspector/front-end/NativeHeapGraph.js:110 > > > +WebInspector.NativeHeapGraphEdge = function(graph, position) > > > > AFAIK our current approach is that we should have types nested into the top-level type (WI.NativeHeapGraph.Edge). Should ask pfeldman to be absolutely sure if you want to leave it this way. > > > Done. What is the advantage of that naming scheme over the old one? It's a surefire way to avoid typename clashes across the frontend codebase.
Yury Semikhatsky
Comment 8 2012-12-20 04:44:10 PST
(In reply to comment #7) > (In reply to comment #4) > > (In reply to comment #2) > > > (From update of attachment 180315 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=180315&action=review > > > > > > > Source/WebCore/inspector/front-end/NativeHeapGraph.js:110 > > > > +WebInspector.NativeHeapGraphEdge = function(graph, position) > > > > > > AFAIK our current approach is that we should have types nested into the top-level type (WI.NativeHeapGraph.Edge). Should ask pfeldman to be absolutely sure if you want to leave it this way. > > > > > Done. What is the advantage of that naming scheme over the old one? > > It's a surefire way to avoid typename clashes across the frontend codebase. I understand how it works in case of WebInspector.Node But is longer WebInspector.NativeHeapGraph.Edge is more safe than WebInspector.NativeHeapGraphEdge?
Yury Semikhatsky
Comment 9 2012-12-20 05:02:55 PST
Note You need to log in before you can comment on or make changes to this bug.