Bug 31553

Summary: Web Inspector: REGRESSION: in Profiles, focusing on function and restoring doesn't restore the original tree
Product: WebKit Reporter: Mikhail Naganov <mnaganov>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, commit-queue, joepeck, keishi, pfeldman, pmuellr, rik, timothy, tolmasky
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://v8.googlecode.com/svn/data/benchmarks/v5/run.html
Attachments:
Description Flags
Screen shots with tree breakage samples
none
Proposed fix
timothy: review-
Proposed fix, comments addressed none

Mikhail Naganov
Reported 2009-11-16 07:18:07 PST
Created attachment 43306 [details] Screen shots with tree breakage samples I tried it on a benchmark just because it generates lots of profiling data. Any other page with JavaScript can be actually used. I observe the following problems: - in Heavy view, after focusing on a non-expanded function node and then restoring back, its children disappear from view (see the first two samples on screen shots); - in Tree view, after focusing on a non-expanded function node and then restoring back, node completely disappears.
Attachments
Screen shots with tree breakage samples (149.38 KB, image/png)
2009-11-16 07:18 PST, Mikhail Naganov
no flags
Proposed fix (5.97 KB, patch)
2009-11-17 05:01 PST, Mikhail Naganov
timothy: review-
Proposed fix, comments addressed (5.96 KB, patch)
2009-11-18 01:12 PST, Mikhail Naganov
no flags
Mikhail Naganov
Comment 1 2009-11-17 05:01:53 PST
Created attachment 43354 [details] Proposed fix
Timothy Hatcher
Comment 2 2009-11-17 14:02:17 PST
Comment on attachment 43354 [details] Proposed fix > + if (this.parent && this.parent.children.indexOf(this) >= 0) { > + this._savedPosition = { > + parent: this.parent, > + index: this.parent.children.indexOf(this) You should cache the result of this.parent.children.indexOf so you don't need to do it twice. When would you not be a child of your parent? Do you need the test.
Timothy Hatcher
Comment 3 2009-11-17 14:05:05 PST
Otherwise r+.
Mikhail Naganov
Comment 4 2009-11-18 01:12:32 PST
Created attachment 43411 [details] Proposed fix, comments addressed OK, I've introduced an assertion to be sure that a node for which savePosition is called has a parent. And yes, I think I don't need to check if a node with parent really exists in its children.
WebKit Commit Bot
Comment 5 2009-11-18 14:41:45 PST
Comment on attachment 43411 [details] Proposed fix, comments addressed Clearing flags on attachment: 43411 Committed r51139: <http://trac.webkit.org/changeset/51139>
WebKit Commit Bot
Comment 6 2009-11-18 14:41:57 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.