DevTools: implement search for CPUProfiler FlameChart view

original search implementation had no idea about flame chart and did search only for data grid nodes.

Also there was a single search box for both types, cpu and heap.

Two separate search view were introduced in the patch.
One for cpu view, the other for heap view.

New code for search in flame chart was written.

Original code for search in cpu data grid was moved to CPUProfileDataGrid.

HeapSnapshot search implementation was slightly tweaked.

BUG=425928
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185011

[loislo, 2014-11-05 18:13:13]

loislo@chromium.org changed reviewers:
+ alph@chromium.org, yurys@chromium.org

[loislo, 2014-11-05 18:13:31]

ptal

[yurys, 2014-11-06 11:39:59]

lgtm provided the comments are addressed

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/components/FlameChart.js (right):

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/components/FlameChart.js:445: var timelineData =
this._timelineData();
Should we check for null data like in some other places?

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/components/FlameChart.js:453:
this._vScrollElement.scrollTop = y;
Did you consider scrolling it so that selected bar would be close to the center
rather than top of the viewport? Would that make sense?

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/components/FlameChart.js:458: var delta =
this._timeWindowLeft - entryEndTime + minEntryTimeWindow;
How about  delta = this._timeWindowLeft - entryStartTime so that leftmost part
of the selected record is always visible. Otherwise if I'm reading the code
correctly you may show right part of the record and cut the label.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/components/FlameChart.js:461: var delta =
entryStartTime - this._timeWindowRight + minEntryTimeWindow;
Maybe delta = entryStartTime - this._timeWindowLeft?

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/profiler/CPUProfileDataGrid.js (right):

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:459: var
millisecondsUnits = (query.length > 2 && query.lastIndexOf("ms") ===
(query.length - 2));
query.endsWith("ms")

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:460: var secondsUnits =
(!millisecondsUnits && query.lastIndexOf("s") === (query.length - 1));
!millisecondsUnits && query.endsWith("s");

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:530: {
} should go on the previous line

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:541: if
(matchesQuery(current)) {
skip {} around 1-line statement

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/profiler/CPUProfileFlameChart.js (right):

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileFlameChart.js:444: var entriesCount
= this._dataProvider._entryNodes.length;
Why do we access private field on the data provider here? Can we expose length
though its public api instead? Also I'm wondering why jscompiler did catch that.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/profiler/HeapSnapshotView.js (right):

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/HeapSnapshotView.js:45: 
revert this

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/HeapSnapshotView.js:594: // Do not remove. It
needs for heap snapshot search tests.
We should turn this into an event then.

[loislo, 2014-11-09 15:32:58]

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/components/FlameChart.js (right):

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/components/FlameChart.js:445: var timelineData =
this._timelineData();
On 2014/11/06 11:39:58, yurys wrote:
> Should we check for null data like in some other places?

Done.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/components/FlameChart.js:458: var delta =
this._timeWindowLeft - entryEndTime + minEntryTimeWindow;
On 2014/11/06 11:39:58, yurys wrote:
> How about  delta = this._timeWindowLeft - entryStartTime so that leftmost part
> of the selected record is always visible. Otherwise if I'm reading the code
> correctly you may show right part of the record and cut the label.

The idea is to do the minimal scrolling but show all the element if it is
possible. If it is not then we will show the rightmost part of the element here.

As far as I can remember if we moves the visible window to the right side of the
chart the label on the element will stick to the leftmost visible point of the
element.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/components/FlameChart.js:461: var delta =
entryStartTime - this._timeWindowRight + minEntryTimeWindow;
On 2014/11/06 11:39:58, yurys wrote:
> Maybe delta = entryStartTime - this._timeWindowLeft?

ditto

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/profiler/CPUProfileDataGrid.js (right):

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:459: var
millisecondsUnits = (query.length > 2 && query.lastIndexOf("ms") ===
(query.length - 2));
On 2014/11/06 11:39:58, yurys wrote:
> query.endsWith("ms")

Done.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:460: var secondsUnits =
(!millisecondsUnits && query.lastIndexOf("s") === (query.length - 1));
On 2014/11/06 11:39:58, yurys wrote:
> !millisecondsUnits && query.endsWith("s");

Done.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:530: {
On 2014/11/06 11:39:58, yurys wrote:
> } should go on the previous line

Done.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:530: {
On 2014/11/06 11:39:58, yurys wrote:
> } should go on the previous line

Done.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileDataGrid.js:541: if
(matchesQuery(current)) {
On 2014/11/06 11:39:58, yurys wrote:
> skip {} around 1-line statement

Done.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/profiler/CPUProfileFlameChart.js (right):

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/CPUProfileFlameChart.js:444: var entriesCount
= this._dataProvider._entryNodes.length;
On 2014/11/06 11:39:58, yurys wrote:
> Why do we access private field on the data provider here? Can we expose length
> though its public api instead? Also I'm wondering why jscompiler did catch
that.

_dataProvider is the instance of WebInspector.CPUFlameChartDataProvider class
and this class is declared in this file.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
File Source/devtools/front_end/profiler/HeapSnapshotView.js (right):

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/HeapSnapshotView.js:45: 
On 2014/11/06 11:39:59, yurys wrote:
> revert this

Done.

https://codereview.chromium.org/696703003/diff/60001/Source/devtools/front_en...
Source/devtools/front_end/profiler/HeapSnapshotView.js:594: // Do not remove. It
needs for heap snapshot search tests.
On 2014/11/06 11:39:59, yurys wrote:
> We should turn this into an event then.

I can fire the event from _dataGrid.highlightObjectByHeapSnapshotId
but I think this place is too generic.
I can fire the event right from this function but it looks as over design.

[loislo, 2014-11-09 15:35:18]

The CQ bit was checked by loislo@chromium.org

[commit-bot: I haz the power, 2014-11-09 15:35:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/696703003/80001

[commit-bot: I haz the power, 2014-11-09 16:37:22]

Committed patchset #5 (id:80001) as 185011