[DevTools] Show blocked requests in network log.

[DevTools] Show blocked requests in network log.

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

[dgozman, 2015-08-26 21:40:02]

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

[dgozman, 2015-08-26 21:40:02]

Please take a look.

[pfeldman, 2015-08-26 23:28:26]

lgtm

[dgozman, 2015-08-26 23:30:08]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2015-08-26 23:30:35]

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

[commit-bot: I haz the power, 2015-08-27 01:48:10]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201275

[brucedawson, 2015-08-28 17:19:55]

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

[brucedawson, 2015-08-28 17:19:56]

https://codereview.chromium.org/1309033003/diff/20001/Source/core/inspector/I...
File Source/core/inspector/InspectorResourceAgent.cpp (right):

https://codereview.chromium.org/1309033003/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorResourceAgent.cpp:380: bool blocked = true;
This 'blocked' variable shadows the 'blocked' variable used to hold individual
blocked URLs. This isn't a bug but it is a bit confusing. Consider renaming one
of the variables?

[dgozman, 2015-08-28 17:48:34]

https://codereview.chromium.org/1309033003/diff/20001/Source/core/inspector/I...
File Source/core/inspector/InspectorResourceAgent.cpp (right):

https://codereview.chromium.org/1309033003/diff/20001/Source/core/inspector/I...
Source/core/inspector/InspectorResourceAgent.cpp:380: bool blocked = true;
On 2015/08/28 17:19:56, brucedawson wrote:
> This 'blocked' variable shadows the 'blocked' variable used to hold individual
> blocked URLs. This isn't a bug but it is a bit confusing. Consider renaming
one
> of the variables?

Thank you for spotting this! I will follow up with a patch. I'm also curious how
do you find this? Using a tool?

[brucedawson, 2015-08-28 17:50:54]

Just a routine reading of all of the CLs committed over the last 24 hours...

Actually, I have an FYI bot that runs VC++'s /analyze (static code analysis) on
Chrome. I generate a report every morning that looks at new warnings, which
includes variable shadowing.

I haven't advertised it more or recommended that others do this because of the
high false positive rate. However, I do plan to investigate enabling
gcc/clang/VC++ variable shadowing warnings in normal builds at some point.

[dgozman, 2015-08-28 18:21:08]

On 2015/08/28 17:50:54, brucedawson wrote:
> Just a routine reading of all of the CLs committed over the last 24 hours...
> 
> Actually, I have an FYI bot that runs VC++'s /analyze (static code analysis)
on
> Chrome. I generate a report every morning that looks at new warnings, which
> includes variable shadowing.
> 
> I haven't advertised it more or recommended that others do this because of the
> high false positive rate. However, I do plan to investigate enabling
> gcc/clang/VC++ variable shadowing warnings in normal builds at some point.

Sounds great, that would definitely help!