DevTools: Display product information in ConsoleContextSelector

DevTools: Display product information in ConsoleContextSelector

BUG=719058
Review-Url: https://codereview.chromium.org/2842843003
Cr-Commit-Position: refs/heads/master@{#471208}
Committed: https://chromium.googlesource.com/chromium/src/+/d3d32c02c99794b0b61f38d7b66011d5595bab2a

[einbinder, 2017-05-05 22:28:00]

Description was changed from

==========
DevTools: Soft drop down
BUG=
==========

to

==========
DevTools: Soft drop down

BUG=719058
==========

[einbinder, 2017-05-05 22:38:41]

Description was changed from

==========
DevTools: Soft drop down

BUG=719058
==========

to

==========
DevTools: Display product information in ConsoleContextSelector

BUG=719058
==========

[einbinder, 2017-05-05 22:57:58]

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

[einbinder, 2017-05-05 22:58:00]

ptal
http://i.imgur.com/HU4Ph6H.png

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

https://codereview.chromium.org/2842843003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sources/CallStackSidebarPane.js:302:
UI.context.setFlavor(SDK.DebuggerModel.CallFrame, item.debuggerCallFrame);
The ConsoleContextSelector checks the selectedCallFrame when the flavor changes,
so reordering this prevents it from being stale.

https://codereview.chromium.org/2842843003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (right):

https://codereview.chromium.org/2842843003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:315: contain: none;
This dramatically speeds up layout when changing the title on the context menu

[dgozman, 2017-05-08 23:16:51]

Not sure about selection vs hover story.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js
(right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:9:
Console.ConsoleContextSelector = class extends UI.ToolbarItem {
Should not extend from UI controls.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:15:
this._titleElement.style.width = '120px';
Remove?

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:33:
this._executionContexts = [];
You don't need this: list.itemAtIndex()

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:34:
this._list = new UI.ListControl(this, UI.ListMode.NonViewport);
Viewport it.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:41:
this._canSelect = false;
shouldHideOnMouseUp

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:92:
this._glassPane.show(this.element.ownerDocument);
nit: just cast it

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:99:
setTimeout(() => this._canSelect = true, 200);
Is it possible to put this in closure?

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:106:
setTimeout(() => this._canSelect = false, 200);
this._canSelect = false   ?

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:129:
this._executionContexts[this._executionContexts.indexOf(currentExecutionContext)
+ 1];
Let's expose selectedIndex()

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:245:
this._list.insertItemAtIndex(index, executionContext);
Let's pass optional comparator to this method.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:267:
if (this._executionContexts.indexOf(executionContext) === -1)
Let's fix this if it ever happens.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:296:
if (!executionContext || this._executionContexts.indexOf(executionContext) ===
-1)
ditto

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:350:
_onSelect(executionContext) {
remove

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:375:
_subtitleFor(executionContext) {
Should we move this into some component maybe? Let's coordinate with performance
and network on this.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:421:
return 20;
Let the list measure.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelector.css
(right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelector.css:2:
* Copyright (c) 2017 The Chromium Authors. All rights reserved.
no (c)

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:184: 
nit: remove extra blank line

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:16: contain: strict;
Does this break wrapping toolbars?

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:215:
.console-context {
This does not deserve to be in toolbar.css

[einbinder, 2017-05-09 22:00:44]

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js
(right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:9:
Console.ConsoleContextSelector = class extends UI.ToolbarItem {
On 2017/05/08 at 23:16:47, dgozman wrote:
> Should not extend from UI controls.

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:15:
this._titleElement.style.width = '120px';
On 2017/05/08 at 23:16:41, dgozman wrote:
> Remove?

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:33:
this._executionContexts = [];
On 2017/05/08 at 23:16:42, dgozman wrote:
> You don't need this: list.itemAtIndex()

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:34:
this._list = new UI.ListControl(this, UI.ListMode.NonViewport);
On 2017/05/08 at 23:16:47, dgozman wrote:
> Viewport it.

Viewporting it ended up being a lot slower.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:41:
this._canSelect = false;
On 2017/05/08 at 23:16:44, dgozman wrote:
> shouldHideOnMouseUp

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:92:
this._glassPane.show(this.element.ownerDocument);
On 2017/05/08 at 23:16:42, dgozman wrote:
> nit: just cast it

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:106:
setTimeout(() => this._canSelect = false, 200);
On 2017/05/08 at 23:16:47, dgozman wrote:
> this._canSelect = false   ?

If you click the button after the list is open, the list closes on mousedown and
needs to stay closed on click.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:129:
this._executionContexts[this._executionContexts.indexOf(currentExecutionContext)
+ 1];
On 2017/05/08 at 23:16:40, dgozman wrote:
> Let's expose selectedIndex()

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:245:
this._list.insertItemAtIndex(index, executionContext);
On 2017/05/08 at 23:16:42, dgozman wrote:
> Let's pass optional comparator to this method.

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:267:
if (this._executionContexts.indexOf(executionContext) === -1)
On 2017/05/08 at 23:16:44, dgozman wrote:
> Let's fix this if it ever happens.

This is a hard fix. There are multiple listeners to  ExecutionContextCreated,
and they might fire ExecutionContextChanged or set the flavor before we get the
Created event.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:350:
_onSelect(executionContext) {
On 2017/05/08 at 23:16:44, dgozman wrote:
> remove

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:375:
_subtitleFor(executionContext) {
On 2017/05/08 at 23:16:44, dgozman wrote:
> Should we move this into some component maybe? Let's coordinate with
performance and network on this.

Moving it anywhere else is tricky because of dependency issues. The network tab
has slightly different logic, which requires this to still pretty much have its
own code.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:421:
return 20;
On 2017/05/08 at 23:16:41, dgozman wrote:
> Let the list measure.

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelector.css
(right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelector.css:2:
* Copyright (c) 2017 The Chromium Authors. All rights reserved.
On 2017/05/08 at 23:16:47, dgozman wrote:
> no (c)

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:184: 
On 2017/05/08 at 23:16:47, dgozman wrote:
> nit: remove extra blank line

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:16: contain: strict;
On 2017/05/08 at 23:16:48, dgozman wrote:
> Does this break wrapping toolbars?

It did. done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:215:
.console-context {
On 2017/05/08 at 23:16:49, dgozman wrote:
> This does not deserve to be in toolbar.css

Where can this go? Should I inject a stylesheet into the toolbar shadow root?

[einbinder, 2017-05-09 22:00:45]

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js
(right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:15:
this._titleElement.style.width = '120px';
On 2017/05/08 at 23:16:41, dgozman wrote:
> Remove?

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:33:
this._executionContexts = [];
On 2017/05/08 at 23:16:42, dgozman wrote:
> You don't need this: list.itemAtIndex()

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:106:
setTimeout(() => this._canSelect = false, 200);
On 2017/05/08 at 23:16:47, dgozman wrote:
> this._canSelect = false   ?

If you click the button after the list is open, the list closes on mousedown and
needs to stay closed on click.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:267:
if (this._executionContexts.indexOf(executionContext) === -1)
On 2017/05/08 at 23:16:44, dgozman wrote:
> Let's fix this if it ever happens.

This is a hard fix. There are multiple listeners to  ExecutionContextCreated,
and they might fire ExecutionContextChanged or set the flavor before we get the
Created event.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelector.css
(right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelector.css:2:
* Copyright (c) 2017 The Chromium Authors. All rights reserved.
On 2017/05/08 at 23:16:47, dgozman wrote:
> no (c)

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:184: 
On 2017/05/08 at 23:16:47, dgozman wrote:
> nit: remove extra blank line

Done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:16: contain: strict;
On 2017/05/08 at 23:16:48, dgozman wrote:
> Does this break wrapping toolbars?

It did. done.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:215:
.console-context {
On 2017/05/08 at 23:16:49, dgozman wrote:
> This does not deserve to be in toolbar.css

Where can this go? Should I inject a stylesheet into the toolbar shadow root?

[dgozman, 2017-05-09 23:11:13]

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js
(right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:34:
this._list = new UI.ListControl(this, UI.ListMode.NonViewport);
On 2017/05/09 22:00:44, einbinder wrote:
> On 2017/05/08 at 23:16:47, dgozman wrote:
> > Viewport it.
> 
> Viewporting it ended up being a lot slower.

Can you elaborate on this?

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:99:
setTimeout(() => this._canSelect = true, 200);
On 2017/05/08 23:16:45, dgozman wrote:
> Is it possible to put this in closure?

What about this?

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:215:
.console-context {
On 2017/05/09 22:00:45, einbinder wrote:
> On 2017/05/08 at 23:16:49, dgozman wrote:
> > This does not deserve to be in toolbar.css
> 
> Where can this go? Should I inject a stylesheet into the toolbar shadow root?

Wrap your custom element with shadow root. See
DeviceModeToolbar._wrapToolbarItem for example.

https://codereview.chromium.org/2842843003/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js
(right):

https://codereview.chromium.org/2842843003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:91:
this._list.viewportResized();
If you don't viewport, this method is not needed.

https://codereview.chromium.org/2842843003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2842843003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:139:
includes(item) {
Let's do indexOfItem instead.

[dgozman, 2017-05-09 23:38:36]

- let's disable if there is a single entry;
- select on mouseup, not mousedown;
- cannot scroll by dragging scrollbar;
- reset selection on show;
- [26677:26677:0509/162026.562293:ERROR:CONSOLE(296)] "Uncaught Attempt to
select non-selectable item", source:
chrome-devtools://devtools/bundled/ui/ListControl.js (296)
- when paused in worker+iframe, resuming in worker makes selecting iframe
context noop;
- resizing console show popup in wrong place (when it's above the console
itself);
- text is cropped for "data:text/html,<but...";
- escape does not remove highlight;
- pagedown does not work;
- I have three "www.theverge.com" items, one of them turned out to be
PaintWorklet;
- click does not work after closing with Enter.

[einbinder, 2017-05-11 00:06:44]

On 2017/05/09 at 23:38:36, dgozman wrote:
> - let's disable if there is a single entry;
Done.
> - select on mouseup, not mousedown;
Done.
> - cannot scroll by dragging scrollbar;
Done.
> - reset selection on show;
Done.
> - [26677:26677:0509/162026.562293:ERROR:CONSOLE(296)] "Uncaught Attempt to
select non-selectable item", source:
chrome-devtools://devtools/bundled/ui/ListControl.js (296)
Fixed by not selecting unselectable items on hover.
> - when paused in worker+iframe, resuming in worker makes selecting iframe
context noop;
This happens because the ExecutionContextSelector tries to select an
unselectable context. Mediated by replacing throw with console.error in
ListControl.
> - resizing console show popup in wrong place (when it's above the console
itself);
Done.
> - text is cropped for "data:text/html,<but...";
This is the frame name, which is cropped by ParsedURL because it is a data url.
> - escape does not remove highlight;
Done.
> - pagedown does not work;
Fixed when moving back to viewport.
> - I have three "www.theverge.com" items, one of them turned out to be
PaintWorklet;
The other one is a frame with no name, but an origin of theverge. The naming
function uses origin as a last resort.
> - click does not work after closing with Enter.
Done.

[einbinder, 2017-05-11 00:07:03]

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js
(right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:34:
this._list = new UI.ListControl(this, UI.ListMode.NonViewport);
On 2017/05/09 at 23:11:12, dgozman wrote:
> On 2017/05/09 22:00:44, einbinder wrote:
> > On 2017/05/08 at 23:16:47, dgozman wrote:
> > > Viewport it.
> > 
> > Viewporting it ended up being a lot slower.
> 
> Can you elaborate on this?

Fixed.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:99:
setTimeout(() => this._canSelect = true, 200);
On 2017/05/09 at 23:11:12, dgozman wrote:
> On 2017/05/08 23:16:45, dgozman wrote:
> > Is it possible to put this in closure?
> 
> What about this?

It needs to be set to false if hidden by keyboard, which is outside the closure.

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (right):

https://codereview.chromium.org/2842843003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:215:
.console-context {
On 2017/05/09 at 23:11:12, dgozman wrote:
> On 2017/05/09 22:00:45, einbinder wrote:
> > On 2017/05/08 at 23:16:49, dgozman wrote:
> > > This does not deserve to be in toolbar.css
> > 
> > Where can this go? Should I inject a stylesheet into the toolbar shadow
root?
> 
> Wrap your custom element with shadow root. See
DeviceModeToolbar._wrapToolbarItem for example.

Done.

https://codereview.chromium.org/2842843003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2842843003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:139:
includes(item) {
On 2017/05/09 at 23:11:12, dgozman wrote:
> Let's do indexOfItem instead.

Done.

[dgozman, 2017-05-11 21:14:52]

lgtm

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js
(right):

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:319:
this._toolbarItem.element.disabled = (this._list.length() <= 1);
Let's land this in a separate patch.

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelectorButton.css
(right):

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelectorButton.css:15:
:host .title {
.title

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:294:
console.error('Attempt to select missing item');
Let's change all of them to console.errors then.

[einbinder, 2017-05-11 21:47:23]

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js
(right):

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/console/ConsoleContextSelector.js:319:
this._toolbarItem.element.disabled = (this._list.length() <= 1);
On 2017/05/11 at 21:14:52, dgozman wrote:
> Let's land this in a separate patch.

Done.

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelectorButton.css
(right):

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/console/consoleContextSelectorButton.css:15:
:host .title {
On 2017/05/11 at 21:14:52, dgozman wrote:
> .title

Done.

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/ui/ListControl.js (right):

https://codereview.chromium.org/2842843003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui/ListControl.js:294:
console.error('Attempt to select missing item');
On 2017/05/11 at 21:14:52, dgozman wrote:
> Let's change all of them to console.errors then.

I kept the throw on methods that need to return values, otherwise all the
clients would  need to cast the result.

[einbinder, 2017-05-11 22:23:40]

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

[commit-bot: I haz the power, 2017-05-11 22:24:38]

Dry run: 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-05-11 23:51:36]

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

[commit-bot: I haz the power, 2017-05-11 23:51:37]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[einbinder, 2017-05-12 01:26:57]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2017-05-12 01:26:58]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2842843003/#ps160001
(title: "fix test")

[commit-bot: I haz the power, 2017-05-12 01:27:59]

CQ is trying da patch.

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

[einbinder, 2017-05-12 01:37:30]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2017-05-12 01:37:30]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2842843003/#ps180001
(title: "f")

[commit-bot: I haz the power, 2017-05-12 01:38:29]

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-05-12 03:35:58]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1494553050035180,
"parent_rev": "ac1414588aedb11ae6a11223c7c3cca1057a4773", "commit_rev":
"d3d32c02c99794b0b61f38d7b66011d5595bab2a"}

[commit-bot: I haz the power, 2017-05-12 03:36:09]

Description was changed from

==========
DevTools: Display product information in ConsoleContextSelector

BUG=719058
==========

to

==========
DevTools: Display product information in ConsoleContextSelector

BUG=719058

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

[commit-bot: I haz the power, 2017-05-12 03:36:10]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/d3d32c02c99794b0b61f38d7b660...