Bug 105524

Summary:

Web Inspector: extract native heap graph representation into a separate file

Product:

WebKit

Reporter:

Yury Semikhatsky <yurys>

Component:

Web Inspector (Deprecated)

Assignee:

Yury Semikhatsky <yurys>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	apavlov: review+, apavlov: commit-queue+

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	Details Formatted Diff Diff
Patch (27.30 KB, patch) 2012-12-20 04:18 PST, Yury Semikhatsky	no flags	Details Formatted Diff Diff
Patch (27.57 KB, patch) 2012-12-20 04:26 PST, Yury Semikhatsky	no flags	Details Formatted Diff Diff
Patch (28.15 KB, patch) 2012-12-20 04:33 PST, Yury Semikhatsky	apavlov: review+ apavlov: commit-queue+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Yury Semikhatsky

Comment 1 2012-12-20 03:27:23 PST

Created attachment 180315 [details] Patch

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

Created attachment 180319 [details] Patch

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

Created attachment 180322 [details] Patch

Yury Semikhatsky

Comment 6 2012-12-20 04:33:11 PST

Created attachment 180323 [details] Patch

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

Committed r138245: <http://trac.webkit.org/changeset/138245>

Note You need to log in before you can comment on or make changes to this bug.