[DevTools] Migrate CallStackSidebarPane to UI.ListControl.

[DevTools] Migrate CallStackSidebarPane to UI.ListControl.

This pane is one of the two clients of UIList, which is going
to be replaced by UI.ListControl.

This patch also moves call stacks pane to shadow dom.

BUG=chromium:630554
Review-Url: https://codereview.chromium.org/2609653002
Cr-Commit-Position: refs/heads/master@{#444971}
Committed: https://chromium.googlesource.com/chromium/src/+/bdeae63b378752373d7ea31f1feadc621f941e17

[dgozman, 2016-12-30 00:51:05]

The CQ bit was checked by dgozman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-30 00:51:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dgozman, 2016-12-30 00:51:45]

dgozman@chromium.org changed reviewers:
+ caseq@chromium.org

[dgozman, 2016-12-30 00:51:45]

Take a look please.

[caseq, 2016-12-30 01:54:43]

lgtm

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js
(right):

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:81:
if (!showBlackboxed) {
showBlackboxed = showBlackboxed || details.callFrames.every(frame =>
Bindings.blackboxManager.isBlackboxedRawLocation(frame.location())

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:96:
for (var i = 0; i < details.callFrames.length; i++) {
var items = details.callFrames.map(frame => ({debuggerCallFrame:
details.callFrames[i]}));
if (!showBlackbox)
 items = items.filter(item => { const location = this._itemLocation(item);
return !location ||  !Bindings.blackboxManager.isBlackboxedRawLocation })

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:165:
element.addEventListener('contextmenu', this._onContextMenu.bind(this, item),
true);
can we have just one event listener for the entire control?

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:173:
element.addEventListener('click', event => {
ditto.

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:183:
Bindings.debuggerWorkspaceBinding.createCallFrameLiveLocation(location,
liveLocation => {
nit: extract a named function

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:202:
return 0;
throw 'Not implemented'?

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:223:
fromElement.removeChild(fromElement.lastElementChild);
can we toggle a class instead?

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:274:
element.createTextChild(' ');
Let's not have multiple adjacent text nodes.

[commit-bot: I haz the power, 2016-12-30 02:15:49]

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

[commit-bot: I haz the power, 2016-12-30 02:15:50]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-01-18 23:14:24]

dgozman@chromium.org changed reviewers:
+ kozyatinskiy@chromium.org

[dgozman, 2017-01-18 23:14:25]

Alexey, could you please take a look?

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js
(right):

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:81:
if (!showBlackboxed) {
On 2016/12/30 01:54:43, caseq wrote:
> showBlackboxed = showBlackboxed || details.callFrames.every(frame =>
> Bindings.blackboxManager.isBlackboxedRawLocation(frame.location())

Done.

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:96:
for (var i = 0; i < details.callFrames.length; i++) {
On 2016/12/30 01:54:43, caseq wrote:
> var items = details.callFrames.map(frame => ({debuggerCallFrame:
> details.callFrames[i]}));
> if (!showBlackbox)
>  items = items.filter(item => { const location = this._itemLocation(item);
> return !location ||  !Bindings.blackboxManager.isBlackboxedRawLocation })

Done.

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:165:
element.addEventListener('contextmenu', this._onContextMenu.bind(this, item),
true);
On 2016/12/30 01:54:43, caseq wrote:
> can we have just one event listener for the entire control?

Done.

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:173:
element.addEventListener('click', event => {
On 2016/12/30 01:54:43, caseq wrote:
> ditto.

Done.

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:183:
Bindings.debuggerWorkspaceBinding.createCallFrameLiveLocation(location,
liveLocation => {
On 2016/12/30 01:54:43, caseq wrote:
> nit: extract a named function

Done.

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:202:
return 0;
On 2016/12/30 01:54:43, caseq wrote:
> throw 'Not implemented'?

Done.

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:223:
fromElement.removeChild(fromElement.lastElementChild);
On 2016/12/30 01:54:43, caseq wrote:
> can we toggle a class instead?

Done.

https://codereview.chromium.org/2609653002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:274:
element.createTextChild(' ');
On 2016/12/30 01:54:43, caseq wrote:
> Let's not have multiple adjacent text nodes.

Done.

[kozy, 2017-01-18 23:42:51]

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/bindings/BlackboxManager.js
(right):

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/BlackboxManager.js:22: /**
@type {!Array<function()>} */
Set?

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js
(right):

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:49:
Bindings.blackboxManager.addChangeListener(this._update.bind(this));
We don't really need to rebuild everything on blackbox state changed -
blackboxManager will trigger updateLocation when location blackboxed state is
changed.
Could we update blackboxed message and per element classes in updateLocation
method?

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:181:
throw 'Should not be called ever!';
Let's replace this with console.assert.

[kozy, 2017-01-18 23:51:49]

lgtm

[kozy, 2017-01-18 23:53:35]

Description was changed from

==========
[DevTools] Migrate CallStackSidebarPane to UI.ListControl.

This pane is one of the two clients of UIList, which is going
to be replaced by UI.ListControl.

This patch also moves call stacks pane to shadow dom.

BUG=none
==========

to

==========
[DevTools] Migrate CallStackSidebarPane to UI.ListControl.

This pane is one of the two clients of UIList, which is going
to be replaced by UI.ListControl.

This patch also moves call stacks pane to shadow dom.

BUG=chromium:630554
==========

[dgozman, 2017-01-20 02:04:59]

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/bindings/BlackboxManager.js
(right):

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/BlackboxManager.js:22: /**
@type {!Array<function()>} */
On 2017/01/18 23:42:51, kozyatinskiy wrote:
> Set?

Done.

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js
(right):

https://codereview.chromium.org/2609653002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:181:
throw 'Should not be called ever!';
On 2017/01/18 23:42:51, kozyatinskiy wrote:
> Let's replace this with console.assert.

Done.

[dgozman, 2017-01-20 02:18:24]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2017-01-20 02:18:25]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org,
kozyatinskiy@chromium.org
Link to the patchset: https://codereview.chromium.org/2609653002/#ps40001
(title: "review fixes")

[commit-bot: I haz the power, 2017-01-20 02:18:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 03:50:55]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484878704984520,
"parent_rev": "0b4be1946c9953f17157f063b43251e32394e193", "commit_rev":
"bdeae63b378752373d7ea31f1feadc621f941e17"}

[commit-bot: I haz the power, 2017-01-20 03:51:33]

Description was changed from

==========
[DevTools] Migrate CallStackSidebarPane to UI.ListControl.

This pane is one of the two clients of UIList, which is going
to be replaced by UI.ListControl.

This patch also moves call stacks pane to shadow dom.

BUG=chromium:630554
==========

to

==========
[DevTools] Migrate CallStackSidebarPane to UI.ListControl.

This pane is one of the two clients of UIList, which is going
to be replaced by UI.ListControl.

This patch also moves call stacks pane to shadow dom.

BUG=chromium:630554

Review-Url: https://codereview.chromium.org/2609653002
Cr-Commit-Position: refs/heads/master@{#444971}
Committed:
https://chromium.googlesource.com/chromium/src/+/bdeae63b378752373d7ea31f1fea...
==========

[commit-bot: I haz the power, 2017-01-20 03:51:34]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/bdeae63b378752373d7ea31f1fea...

[lushnikov, 2017-01-23 05:14:17]

lushnikov@chromium.org changed reviewers:
+ lushnikov@chromium.org

[lushnikov, 2017-01-23 05:14:18]

This is exciting!!!