DevTools: render console filter in the main console toolbar.

DevTools: render console filter in the main console toolbar.

BUG=687276
Review-Url: https://codereview.chromium.org/2666013002
Cr-Commit-Position: refs/heads/master@{#447406}
Committed: https://chromium.googlesource.com/chromium/src/+/23b671d023dd981f7d7672f8ffaaa160cffbfe12

[pfeldman, 2017-01-31 02:25:29]

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

[dgozman, 2017-01-31 04:21:30]

Let's have a bug or a screenshot.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/inspector/console-show-all-messages.html
(right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/inspector/console-show-all-messages.html:26:
InspectorTest.addResult("\"Show all messages\" checkbox should be unchecked by
default");
Show all messages -> Show target messages

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js (left):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:1156:
this._filterRegex.lastIndex = 0;
Why removed?

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:79:
toolbar.appendText('');
Why this?

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:93: //
settingsToolbar.appendToolbarItem(this._filter._showTargetMessagesCheckbox);
- Commented code?
- Is it never here? How does the test pass then?

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:437:
this._hiddenByFilterCount + ' messages are hidden by filters.'));
Use %d now?

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:1048: new
UI.ToolbarCheckbox(Common.UIString('Selected target only'), undefined,
undefined, filterChangedCallback);
Didn't we agree with Paul to use 'frame' or 'context' here instead of 'target'?

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/module.json (right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/module.json:84:
"settingName": "monitoringXHREnabled",
Didn't we agree to put more console settings to the new toolbar?

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js (right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:916: var filterOption
= options[i];
It's not a filter anymore.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:921:
this._selectElement.appendChild(option);
You can assign this._selectElement to optGroupElement before for-loop to
simplify this.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:932: isActive() {
isNonDefault

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:960:
_filterChanged(event) {
_valueChanged

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:204: width: 120px;
I think this will affect more toolbars.

[pfeldman, 2017-01-31 19:50:53]

Description was changed from

==========
DevTools: render console filter in the main console toolbar.
==========

to

==========
DevTools: render console filter in the main console toolbar.

BUG=687276
==========

[pfeldman, 2017-01-31 19:54:45]

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js (left):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:1156:
this._filterRegex.lastIndex = 0;
It is done in matchesFilterRegex call below as well.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:79:
toolbar.appendText('');
On 2017/01/31 04:21:29, dgozman_slow wrote:
> Why this?

To render split.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:93: //
settingsToolbar.appendToolbarItem(this._filter._showTargetMessagesCheckbox);
On 2017/01/31 04:21:29, dgozman_slow wrote:
> - Commented code?
> - Is it never here? How does the test pass then?

This is removing item from toolbar, so the test passes. I'll restore it.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:437:
this._hiddenByFilterCount + ' messages are hidden by filters.'));
On 2017/01/31 04:21:29, dgozman_slow wrote:
> Use %d now?

It is still conditional, right?

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:1048: new
UI.ToolbarCheckbox(Common.UIString('Selected target only'), undefined,
undefined, filterChangedCallback);
On 2017/01/31 04:21:29, dgozman_slow wrote:
> Didn't we agree with Paul to use 'frame' or 'context' here instead of
'target'?

No, I don't think 'frame' is good. You always have extensions, often have
'workers', so frame is no good.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/module.json (right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/module.json:84:
"settingName": "monitoringXHREnabled",
On 2017/01/31 04:21:29, dgozman_slow wrote:
> Didn't we agree to put more console settings to the new toolbar?

I'd rather do this incrementally.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js (right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:916: var filterOption
= options[i];
On 2017/01/31 04:21:30, dgozman_slow wrote:
> It's not a filter anymore.

Done.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:921:
this._selectElement.appendChild(option);
On 2017/01/31 04:21:30, dgozman_slow wrote:
> You can assign this._selectElement to optGroupElement before for-loop to
> simplify this.

I fixed it, but I did not reassign anything.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:932: isActive() {
On 2017/01/31 04:21:30, dgozman_slow wrote:
> isNonDefault

Done in a way.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Toolbar.js:960:
_filterChanged(event) {
On 2017/01/31 04:21:29, dgozman_slow wrote:
> _valueChanged

Done.

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/toolbar.css (right):

https://codereview.chromium.org/2666013002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/toolbar.css:204: width: 120px;
On 2017/01/31 04:21:30, dgozman_slow wrote:
> I think this will affect more toolbars.

I know, but I don't yet know where this results in a bad behavior.

[pfeldman, 2017-01-31 19:55:51]

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

[commit-bot: I haz the power, 2017-01-31 19:56:32]

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

[dgozman, 2017-01-31 21:13:52]

lgtm

[commit-bot: I haz the power, 2017-01-31 22:44:41]

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

[commit-bot: I haz the power, 2017-01-31 22:44:42]

Dry run: This issue passed the CQ dry run.

[pfeldman, 2017-02-01 01:38:46]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2017-02-01 01:39:35]

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-02-01 01:47:26]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485913126900860,
"parent_rev": "fc7354b06f0318afbec6958b5514ae131352619d", "commit_rev":
"23b671d023dd981f7d7672f8ffaaa160cffbfe12"}

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

Description was changed from

==========
DevTools: render console filter in the main console toolbar.

BUG=687276
==========

to

==========
DevTools: render console filter in the main console toolbar.

BUG=687276

Review-Url: https://codereview.chromium.org/2666013002
Cr-Commit-Position: refs/heads/master@{#447406}
Committed:
https://chromium.googlesource.com/chromium/src/+/23b671d023dd981f7d7672f8ffaa...
==========

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

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