Summary: | Web Inspector: [Chromium] Prepare for landing of detailed heap snapshots | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Naganov <mnaganov> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Mikhail Naganov <mnaganov> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 50510 | ||||||||||||
Attachments: |
|
Description
Mikhail Naganov
2011-01-18 05:45:32 PST
Created attachment 79271 [details]
patch
Created attachment 80037 [details]
rebaselined
Comment on attachment 80037 [details] rebaselined View in context: https://bugs.webkit.org/attachment.cgi?id=80037&action=review A bunch of nits, otherwise looks good. > Source/WebCore/bindings/js/ScriptProfiler.h:41 > + class HeapSnapshotControl { HeapSnapshotProgress: start(totalWork) worked(workDone) done() bool isCanceled() > Source/WebCore/inspector/front-end/DetailedHeapshotView.js:42 > + get statusBarItems() why do you need this? > Source/WebCore/inspector/front-end/DetailedHeapshotView.js:59 > + WebInspector.View.prototype.show.call(this, parentElement); why do you need this? > Source/WebCore/inspector/front-end/DetailedHeapshotView.js:64 > + WebInspector.View.prototype.hide.call(this); why do you need this? > Source/WebCore/inspector/front-end/DetailedHeapshotView.js:65 > + }, no trailing coma > Source/WebCore/inspector/front-end/HeapSnapshot.js:46 > + return this._getType() === this._snapshot._edgeElementType; No "get" prefixes in WebCore. Comment on attachment 80037 [details] rebaselined View in context: https://bugs.webkit.org/attachment.cgi?id=80037&action=review >> Source/WebCore/bindings/js/ScriptProfiler.h:41 >> + class HeapSnapshotControl { > > HeapSnapshotProgress: > start(totalWork) > worked(workDone) > done() > bool isCanceled() This is also a control interface -- when progress is reported it's possible to cancel job. So I made 'worked' to accept cancellation flag, and 'isCancelled' isn't needed. >> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:42 >> + get statusBarItems() > > why do you need this? Removed. ProfilesPanel required statusBarItems to be defined, I fixed this. >> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:59 >> + WebInspector.View.prototype.show.call(this, parentElement); > > why do you need this? Removed. >> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:64 >> + WebInspector.View.prototype.hide.call(this); > > why do you need this? Removed. >> Source/WebCore/inspector/front-end/DetailedHeapshotView.js:65 >> + }, > > no trailing coma Fixed. >> Source/WebCore/inspector/front-end/HeapSnapshot.js:46 >> + return this._getType() === this._snapshot._edgeElementType; > > No "get" prefixes in WebCore. Fixed all such issues. Created attachment 80050 [details]
comments addressed
Created attachment 80051 [details]
added isCanceled
Manually committed as http://trac.webkit.org/changeset/76597 2011-01-25 Mikhail Naganov <mnaganov@chromium.org> Reviewed by Pavel Feldman. Web Inspector: [Chromium] Prepare for landing of detailed heap snapshots. - Introduce Preferences.detailedHeapProfiles flag for controlling querying of detailed heap snapshots. - Add boilerplate code for the new view. - Factor out common code. https://bugs.webkit.org/show_bug.cgi?id=52624 * WebCore.gypi: * WebCore.vcproj/WebCore.vcproj: * bindings/js/ScriptProfiler.h: (WebCore::ScriptProfiler::HeapSnapshotControl::~HeapSnapshotControl): (WebCore::ScriptProfiler::takeHeapSnapshot): * bindings/v8/ScriptProfiler.cpp: (WebCore::ScriptProfiler::takeHeapSnapshot): * bindings/v8/ScriptProfiler.h: (WebCore::ScriptProfiler::HeapSnapshotControl::~HeapSnapshotControl): * inspector/Inspector.idl: * inspector/InspectorProfilerAgent.cpp: (WebCore::InspectorProfilerAgent::takeHeapSnapshot): * inspector/InspectorProfilerAgent.h: * inspector/front-end/DetailedHeapshotView.js: Added. (WebInspector.DetailedHeapshotView): (WebInspector.DetailedHeapshotView.prototype.get statusBarItems): (WebInspector.DetailedHeapshotView.prototype.get profile): (WebInspector.DetailedHeapshotView.prototype.set profile): (WebInspector.DetailedHeapshotView.prototype.show): (WebInspector.DetailedHeapshotView.prototype.hide): (WebInspector.DetailedHeapshotProfileType): (WebInspector.DetailedHeapshotProfileType.prototype.get buttonTooltip): (WebInspector.DetailedHeapshotProfileType.prototype.get buttonStyle): (WebInspector.DetailedHeapshotProfileType.prototype.buttonClicked): (WebInspector.DetailedHeapshotProfileType.prototype.get welcomeMessage): (WebInspector.DetailedHeapshotProfileType.prototype.createSidebarTreeElementForProfile): (WebInspector.DetailedHeapshotProfileType.prototype.createView): * inspector/front-end/HeapSnapshot.js: Added. (WebInspector.HeapSnapshotEdgesIterator): (WebInspector.HeapSnapshotEdgesIterator.prototype.get done): (WebInspector.HeapSnapshotEdgesIterator.prototype.get isElement): (WebInspector.HeapSnapshotEdgesIterator.prototype.get isHidden): (WebInspector.HeapSnapshotEdgesIterator.prototype.get name): (WebInspector.HeapSnapshotEdgesIterator.prototype.next): (WebInspector.HeapSnapshotEdgesIterator.prototype.get node): (WebInspector.HeapSnapshotEdgesIterator.prototype.get nodeIndex): (WebInspector.HeapSnapshotEdgesIterator.prototype._getNameOrIndex): (WebInspector.HeapSnapshotEdgesIterator.prototype._getType): (WebInspector.HeapSnapshotNodeWrapper): (WebInspector.HeapSnapshotNodeWrapper.prototype.get edges): (WebInspector.HeapSnapshotNodeWrapper.prototype.get edgesCount): (WebInspector.HeapSnapshotNodeWrapper.prototype.get instancesCount): (WebInspector.HeapSnapshotNodeWrapper.prototype.get isHidden): (WebInspector.HeapSnapshotNodeWrapper.prototype.get name): (WebInspector.HeapSnapshotNodeWrapper.prototype.get selfSize): (WebInspector.HeapSnapshotNodeWrapper.prototype._getName): (WebInspector.HeapSnapshotNodeWrapper.prototype._getEdges): (WebInspector.HeapSnapshotNodeWrapper.prototype._getType): (WebInspector.HeapSnapshot): (WebInspector.HeapSnapshot.prototype._init): (WebInspector.HeapSnapshot.prototype.get rootEdges): * inspector/front-end/HeapSnapshotView.js: (WebInspector.HeapSnapshotProfileType.prototype.buttonClicked): * inspector/front-end/ProfilesPanel.js: (WebInspector.ProfilesPanel.prototype._setRecordingProfile): (WebInspector.ProfilesPanel.prototype._reportHeapSnapshotProgress): (WebInspector.ProfilerDispatcher.prototype.setRecordingProfile): (WebInspector.ProfilerDispatcher.prototype.reportHeapSnapshotProgress): * inspector/front-end/Settings.js: * inspector/front-end/WebKit.qrc: * inspector/front-end/inspector.html: * inspector/front-end/inspector.js: (WebInspector._createPanels): * src/js/DevTools.js: (): |