[DevTools] UI for blocked URLs.

[DevTools] UI for blocked URLs.

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

[dgozman, 2015-08-28 00:28:10]

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

[dgozman, 2015-08-28 00:28:11]

Take a look please.

Screenshot: http://pasteboard.co/cHJ4vaW.png

[pfeldman, 2015-08-28 17:15:49]

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/network/BlockedURLsPane.js (right):

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:13:
this.registerRequiredCSS("network/blockedURLsPane.css");
Since you have a css, could you move it into shadow?

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:21:
this._toolbar.appendToolbarItem(new WebInspector.ToolbarSpace());
Toolbar is a continuous set of buttons. Lets move the toolbar to the left and
then simply add a div with the info.

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:68: event.consume();
Why consuming WebInspectorEvent?

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:216: for (var request of
this._blockedRequests) {
Looks like you could have a URL -> count map instead.

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:242: if
(!this._updateTimeout)
Should you use throttler here instead?

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/network/NetworkPanel.js (right):

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/NetworkPanel.js:86: if
(Runtime.experiments.isEnabled("blockedURLs")) {
Looks good enough to leave the experiment to me.

[dgozman, 2015-08-31 21:17:39]

PTAL

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/network/BlockedURLsPane.js (right):

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:13:
this.registerRequiredCSS("network/blockedURLsPane.css");
On 2015/08/28 17:15:49, pfeldman wrote:
> Since you have a css, could you move it into shadow?

Definitely.

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:21:
this._toolbar.appendToolbarItem(new WebInspector.ToolbarSpace());
On 2015/08/28 17:15:49, pfeldman wrote:
> Toolbar is a continuous set of buttons. Lets move the toolbar to the left and
> then simply add a div with the info.

Changed to custom element.

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:68: event.consume();
On 2015/08/28 17:15:49, pfeldman wrote:
> Why consuming WebInspectorEvent?

Copy-pasta :)

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:216: for (var request of
this._blockedRequests) {
On 2015/08/28 17:15:49, pfeldman wrote:
> Looks like you could have a URL -> count map instead.

Done.

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/BlockedURLsPane.js:242: if
(!this._updateTimeout)
On 2015/08/28 17:15:49, pfeldman wrote:
> Should you use throttler here instead?

Done.

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/network/NetworkPanel.js (right):

https://codereview.chromium.org/1311693004/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/network/NetworkPanel.js:86: if
(Runtime.experiments.isEnabled("blockedURLs")) {
On 2015/08/28 17:15:49, pfeldman wrote:
> Looks good enough to leave the experiment to me.

Done.

[pfeldman, 2015-09-01 01:05:36]

lgtm

[dgozman, 2015-09-01 01:07:11]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2015-09-01 01:07:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311693004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311693004/40001

[commit-bot: I haz the power, 2015-09-01 01:08:33]

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

[commit-bot: I haz the power, 2015-09-01 01:08:33]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[dgozman, 2015-09-01 20:58:41]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2015-09-01 20:58:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1311693004/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2015-09-01 20:58:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311693004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311693004/60001

[commit-bot: I haz the power, 2015-09-01 22:08:43]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201576