| Summary: | Web Inspector: Auto expand retaining path until there are more than one retainer. | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alexei Filippov <alph> | ||||||||||||||
| Component: | Web Inspector (Deprecated) | Assignee: | Alexei Filippov <alph> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | apavlov, dglazkov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||||||||||
| Priority: | P2 | ||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||
| Hardware: | All | ||||||||||||||||
| OS: | All | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Alexei Filippov
2013-03-18 10:55:11 PDT
Created attachment 193606 [details]
Patch
Comment on attachment 193606 [details]
Patch
Please provide a test.
Created attachment 194050 [details]
Patch
(In reply to comment #2) > (From update of attachment 193606 [details]) > Please provide a test. Added a test. (In reply to comment #0) > When retainers view gets opened automatically expand retaining path until reaching an object that has more than one retainer. What about expanding shortest path to the Window? Comment on attachment 194050 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194050&action=review > Source/WebCore/inspector/front-end/HeapSnapshotDataGrids.js:508 > + if (this.children.length === 1) { Can it happen that we didn't receive response from the worker and don't know number of children here? Created attachment 194206 [details]
Patch
Comment on attachment 194050 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194050&action=review >> Source/WebCore/inspector/front-end/HeapSnapshotDataGrids.js:508 >> + if (this.children.length === 1) { > > Can it happen that we didn't receive response from the worker and don't know number of children here? AFAICT populateComplete event arrives on response from the worker. If for some reason there's no response it just stops expanding the chain. (In reply to comment #5) > (In reply to comment #0) > > When retainers view gets opened automatically expand retaining path until reaching an object that has more than one retainer. > > What about expanding shortest path to the Window? Good idea. However it sounds like a task for a separate CL. Comment on attachment 194206 [details] Patch Attachment 194206 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17112602 New failing tests: inspector/profiler/heap-snapshot-summary-retainers.html platform/chromium/fast/forms/calendar-picker/calendar-picker-appearance-month-popup.html Created attachment 194212 [details]
Archive of layout-test-results from gce-cr-linux-01
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-3.3.8-gcg-201212281604-x86_64-with-GCEL-10.04-gcel_10.04
Created attachment 194265 [details]
Patch
Comment on attachment 194265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194265&action=review > LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:106 > + setTimeout(step5.bind(null, retainersRoot), 500); This is bound to flake. Please replace the timeout with some deterministic condition. > LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:183 > + setTimeout(step5.bind(null, retainersRoot), 500); Ditto. Comment on attachment 194265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194265&action=review >> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:106 >> + setTimeout(step5.bind(null, retainersRoot), 500); > > This is bound to flake. Please replace the timeout with some deterministic condition. ok >> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:183 >> + setTimeout(step5.bind(null, retainersRoot), 500); > > Ditto. This one is not a flake. It is a deterministic poll loop. It will wait until there's at least 20 items expanded. See lines 189-190. Comment on attachment 194265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194265&action=review >>> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:183 >>> + setTimeout(step5.bind(null, retainersRoot), 500); >> >> Ditto. > > This one is not a flake. It is a deterministic poll loop. It will wait until there's at least 20 items expanded. See lines 189-190. You should be able to listen to some event instead of polling with 500ms interval. Created attachment 195323 [details]
Patch
Comment on attachment 194265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194265&action=review >>>> LayoutTests/inspector/profiler/heap-snapshot-summary-retainers.html:183 >>>> + setTimeout(step5.bind(null, retainersRoot), 500); >>> >>> Ditto. >> >> This one is not a flake. It is a deterministic poll loop. It will wait until there's at least 20 items expanded. See lines 189-190. > > You should be able to listen to some event instead of polling with 500ms interval. done Comment on attachment 195323 [details] Patch Clearing flags on attachment: 195323 Committed r147090: <http://trac.webkit.org/changeset/147090> All reviewed patches have been landed. Closing bug. |