Bug 31553 - Web Inspector: REGRESSION: in Profiles, focusing on function and restoring doesn't restore the original tree
Summary: Web Inspector: REGRESSION: in Profiles, focusing on function and restoring do...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://v8.googlecode.com/svn/data/ben...
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-16 07:18 PST by Mikhail Naganov
Modified: 2009-11-18 14:41 PST (History)
9 users (show)

See Also:


Attachments
Screen shots with tree breakage samples (149.38 KB, image/png)
2009-11-16 07:18 PST, Mikhail Naganov
no flags Details
Proposed fix (5.97 KB, patch)
2009-11-17 05:01 PST, Mikhail Naganov
timothy: review-
Details | Formatted Diff | Diff
Proposed fix, comments addressed (5.96 KB, patch)
2009-11-18 01:12 PST, Mikhail Naganov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Naganov 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.
Comment 1 Mikhail Naganov 2009-11-17 05:01:53 PST
Created attachment 43354 [details]
Proposed fix
Comment 2 Timothy Hatcher 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.
Comment 3 Timothy Hatcher 2009-11-17 14:05:05 PST
Otherwise r+.
Comment 4 Mikhail Naganov 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.
Comment 5 WebKit Commit Bot 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>
Comment 6 WebKit Commit Bot 2009-11-18 14:41:57 PST
All reviewed patches have been landed.  Closing bug.