DevTools: Remove popover and add properties section for paint profiler log.

Remove popover and add properties section for paint profiler log.

Review URL: https://codereview.chromium.org/393123007

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

[malch, 2014-07-18 08:17:52]

Please take a look, thank you!

[caseq, 2014-07-21 14:36:42]

Generally looks good, but please bring back abbreviated call params.
Also, I don't think it's practical to land before invalidation issues are fixed
in the platform.

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/devtools...
File Source/devtools/devtools.gypi (right):

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/devtools...
Source/devtools/devtools.gypi:287: 'front_end/paintProfilerView.css',
You can add that to layersPanel.css for the time being -- we reuse it in
Timeline panel as well.

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/paintProfilerView.css (left):

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/paintProfilerView.css:31: #include "config.h"
You may need to re-upload the patch.

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/PaintProfilerView.js (left):

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/timeline/PaintProfilerView.js:378: textContent += "("
+ this._paramsToString(logItem.params) + ")";
We still want this, so that we can have a quick overview of what the call does.

[malch, 2014-07-22 07:22:25]

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/devtools...
File Source/devtools/devtools.gypi (right):

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/devtools...
Source/devtools/devtools.gypi:287: 'front_end/paintProfilerView.css',
On 2014/07/21 14:36:41, caseq wrote:
> You can add that to layersPanel.css for the time being -- we reuse it in
> Timeline panel as well.

Done.

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/paintProfilerView.css (left):

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/paintProfilerView.css:31: #include "config.h"
On 2014/07/21 14:36:41, caseq wrote:
> You may need to re-upload the patch.

Done.

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/PaintProfilerView.js (left):

https://codereview.chromium.org/393123007/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/timeline/PaintProfilerView.js:378: textContent += "("
+ this._paramsToString(logItem.params) + ")";
On 2014/07/21 14:36:41, caseq wrote:
> We still want this, so that we can have a quick overview of what the call
does.

Done.

[caseq, 2014-07-31 13:00:11]

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:280:
WebInspector.LogTreeElement.appendLogItem(this.sidebarTree, this, this._log[i]);
Let's make it an instance method of PaintProfilerCommandLogView -- you're
passing `this' and this.sidebarTree anyway.

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:334:
TreeElement.call(this, "yo", logItem);
yo!

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:382: textContent += "("
+ this._paramsToString(logItem.params) + ")";
just inline above line + this one into the below call. It looks like you don't
need a temp var anymore.

[caseq, 2014-07-31 13:48:40]

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:483:
WebInspector.LogPropertyTreeElement.appendLogPropertyItem = function(element,
name, value)
make it _private and move just after LogPropertyTreeElement (before 'prototype =
...')

https://codereview.chromium.org/393123007/diff/130001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/130001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:278: // for (var param
in logItem.params)
please remove

https://codereview.chromium.org/393123007/diff/130001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:291: for (var i =
stepLeft; i <= stepRight; ++i) {
{} are redundant

[malch, 2014-07-31 13:55:54]

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:280:
WebInspector.LogTreeElement.appendLogItem(this.sidebarTree, this, this._log[i]);
On 2014/07/31 13:00:11, caseq wrote:
> Let's make it an instance method of PaintProfilerCommandLogView -- you're
> passing `this' and this.sidebarTree anyway.

Done.

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:334:
TreeElement.call(this, "yo", logItem);
On 2014/07/31 13:00:11, caseq wrote:
> yo!

Done.

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:382: textContent += "("
+ this._paramsToString(logItem.params) + ")";
On 2014/07/31 13:00:11, caseq wrote:
> just inline above line + this one into the below call. It looks like you don't
> need a temp var anymore.

Done.

https://codereview.chromium.org/393123007/diff/110001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:483:
WebInspector.LogPropertyTreeElement.appendLogPropertyItem = function(element,
name, value)
On 2014/07/31 13:48:39, caseq wrote:
> make it _private and move just after LogPropertyTreeElement (before 'prototype
=
> ...')

Done.

https://codereview.chromium.org/393123007/diff/130001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/130001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:278: // for (var param
in logItem.params)
On 2014/07/31 13:48:39, caseq wrote:
> please remove

Done.

https://codereview.chromium.org/393123007/diff/130001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:291: for (var i =
stepLeft; i <= stepRight; ++i) {
On 2014/07/31 13:48:39, caseq wrote:
> {} are redundant

Done.

[caseq, 2014-07-31 14:20:37]

lgtm

https://codereview.chromium.org/393123007/diff/150001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/150001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:276: var first = new
WebInspector.LogTreeElement(this, logItem);
s/first/treeElement/?

https://codereview.chromium.org/393123007/diff/150001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:353: this.hasChildren =
true;
nit: this.hasChildren = !!this.representedObject.params;

[malch, 2014-07-31 14:27:34]

https://codereview.chromium.org/393123007/diff/150001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/150001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:276: var first = new
WebInspector.LogTreeElement(this, logItem);
On 2014/07/31 14:20:37, caseq wrote:
> s/first/treeElement/?

Done.

https://codereview.chromium.org/393123007/diff/150001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:353: this.hasChildren =
true;
On 2014/07/31 14:20:37, caseq wrote:
> nit: this.hasChildren = !!this.representedObject.params;

Done.

[pfeldman, 2014-07-31 16:34:28]

lgtm

https://codereview.chromium.org/393123007/diff/170001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/170001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:470: this._update();
inline?

https://codereview.chromium.org/393123007/diff/170001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:479: return value ===
null ? "null" : typeof value;
inline?

https://codereview.chromium.org/393123007/diff/170001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:500: };
drop ;

[malch, 2014-08-13 08:46:52]

https://codereview.chromium.org/393123007/diff/170001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/PaintProfilerView.js (right):

https://codereview.chromium.org/393123007/diff/170001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:470: this._update();
On 2014/07/31 16:34:28, pfeldman wrote:
> inline?

Done.

https://codereview.chromium.org/393123007/diff/170001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:479: return value ===
null ? "null" : typeof value;
On 2014/07/31 16:34:28, pfeldman wrote:
> inline?

Done.

https://codereview.chromium.org/393123007/diff/170001/Source/devtools/front_e...
Source/devtools/front_end/timeline/PaintProfilerView.js:500: };
On 2014/07/31 16:34:28, pfeldman wrote:
> drop ;

Done.

[pfeldman, 2014-08-13 09:10:29]

lgtm

[caseq, 2014-08-18 08:59:38]

The CQ bit was checked by caseq@chromium.org

[commit-bot: I haz the power, 2014-08-18 09:00:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/malch@chromium.org/393123007/190001

[commit-bot: I haz the power, 2014-08-18 10:02:50]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/20158)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23174)

[commit-bot: I haz the power, 2014-08-18 10:27:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-18 10:27:52]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)

[malch, 2014-08-18 11:20:20]

On 2014/08/18 10:27:52, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   linux_blink_rel on tryserver.blink
>
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)

Fixed failing test.

[caseq, 2014-08-18 11:49:59]

The CQ bit was checked by caseq@chromium.org

[commit-bot: I haz the power, 2014-08-18 11:50:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/malch@chromium.org/393123007/210001

[commit-bot: I haz the power, 2014-08-18 12:55:18]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23213)

[commit-bot: I haz the power, 2014-08-18 13:33:38]

Committed patchset #12 (210001) as 180458

[eustas, 2014-08-21 12:52:04]

https://codereview.chromium.org/393123007/diff/210001/Source/devtools/front_e...
File Source/devtools/front_end/layersPanel.css (right):

https://codereview.chromium.org/393123007/diff/210001/Source/devtools/front_e...
Source/devtools/front_end/layersPanel.css:114: .paint-profiler-view canvas {
This class is never set.

Would you mind to remove this selector?