Bug 112584 - Web Inspector: move _timelineGrid && _timelineOverviewWindow from TimelineOverviewPane into a new class OverviewGrid
Summary: Web Inspector: move _timelineGrid && _timelineOverviewWindow from TimelineOve...
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: Ilya Tikhonovsky
URL:
Keywords:
Depends on: 112594
Blocks: 112577
  Show dependency tree / graph
 
Reported: 2013-03-18 09:26 PDT by Ilya Tikhonovsky
Modified: 2013-03-19 04:48 PDT (History)
9 users (show)

See Also:

Attachments
Patch (11.21 KB, patch)
2013-03-18 09:33 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
Patch (11.55 KB, patch)
2013-03-18 09:59 PDT, Ilya Tikhonovsky 		pfeldman: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2013-03-18 09:26:29 PDT 
EOM
Comment 1 Ilya Tikhonovsky 2013-03-18 09:33:00 PDT 
Created attachment 193581 [details]
Patch
Comment 2 Pavel Feldman 2013-03-18 09:37:49 PDT 
Comment on attachment 193581 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193581&action=review

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:81
> +    this._overviewGrid.grid.element.insertBefore(this._heapGraph.element, this._overviewGrid.grid.itemsGraphsElement);

You should expose insertBefore on overview grid for now.

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:120
> +    get grid()

Don't use getters please.

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:140
> +    updateDividers: function(calculator)

Annotate please.
Comment 3 Ilya Tikhonovsky 2013-03-18 09:59:49 PDT 
Created attachment 193590 [details]
Patch
Comment 4 Pavel Feldman 2013-03-18 10:11:04 PDT 
Comment on attachment 193590 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193590&action=review

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:-98
> -    this._overviewGrid.itemsGraphsElement.appendChild(this._categoryStrips.element);

appendAfter...

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:120
> +    itemsGraphsElement: function()

Hide this one.

> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:162
> +    addEventDividers: function(dividers)

Is this used?
Comment 5 Ilya Tikhonovsky 2013-03-18 10:19:23 PDT 
Committed r146079: <http://trac.webkit.org/changeset/146079>
Comment 6 Ilya Tikhonovsky 2013-03-18 10:21:43 PDT 
Comment on attachment 193590 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193590&action=review

>> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:-98
>> -    this._overviewGrid.itemsGraphsElement.appendChild(this._categoryStrips.element);
> 
> appendAfter...

We are not adding a child after itemsGraphsElement. We are inserting a child into it.

>> Source/WebCore/inspector/front-end/TimelineOverviewPane.js:162
>> +    addEventDividers: function(dividers)
> 
> Is this used?

The code which is using the method hasn't been changed
Comment 7 WebKit Review Bot 2013-03-18 10:52:28 PDT 
Re-opened since this is blocked by bug 112594
Comment 8 Ilya Tikhonovsky 2013-03-19 04:48:56 PDT 
Committed r146199: <http://trac.webkit.org/changeset/146199>