Extensions: Add metrics for Web Request API actions.

Extensions: Add metrics for Web Request API actions.

This CL adds metrics for how the Web Request API is used. The following actions 
are logged per web request stage:
  -CANCEL : Web request is cancelled
  -REDIRECT : Web request is redirected
  -MODIFY_REQUEST_HEADERS : Request headers are modified
  -MODIFY_RESPONSE_HEADERS : Response headers are modified
  -SET_AUTH_CREDENTIALS : Auth credentials are set.

BUG=696822
Review-Url: https://codereview.chromium.org/2708923005
Cr-Commit-Position: refs/heads/master@{#453466}
Committed: https://chromium.googlesource.com/chromium/src/+/741177620dfb8253a9e35560b37e412202090d2a

[karandeepb, 2017-02-22 23:10:44]

Description was changed from

==========
Extensions: Web Request API metrics.
==========

to

==========
Extensions: Add metrics for Web Request API actions.

This CL adds metrics for how the Web Request API is used. The following actions 
are logged per web request stage:
  -CANCEL : Web request is cancelled
  -REDIRECT : Web request is redirected
  -MODIFY_REQUEST_HEADERS : Request headers are modified
  -MODIFY_REQUEST_COOKIES : Request cookies are modified
  -MODIFY_RESPONSE_HEADERS : Response headers are modified
  -MODIFY_RESPONSE_COOKIES : Response cookies are modified
  -SET_AUTH_CREDENTIALS : Auth credentials are set.

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

[karandeepb, 2017-02-23 03:25:41]

Description was changed from

==========
Extensions: Add metrics for Web Request API actions.

This CL adds metrics for how the Web Request API is used. The following actions 
are logged per web request stage:
  -CANCEL : Web request is cancelled
  -REDIRECT : Web request is redirected
  -MODIFY_REQUEST_HEADERS : Request headers are modified
  -MODIFY_REQUEST_COOKIES : Request cookies are modified
  -MODIFY_RESPONSE_HEADERS : Response headers are modified
  -MODIFY_RESPONSE_COOKIES : Response cookies are modified
  -SET_AUTH_CREDENTIALS : Auth credentials are set.

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

to

==========
Extensions: Add metrics for Web Request API actions.

This CL adds metrics for how the Web Request API is used. The following actions 
are logged per web request stage:
  -CANCEL : Web request is cancelled
  -REDIRECT : Web request is redirected
  -MODIFY_REQUEST_HEADERS : Request headers are modified
  -MODIFY_RESPONSE_HEADERS : Response headers are modified
  -SET_AUTH_CREDENTIALS : Auth credentials are set.

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

[karandeepb, 2017-02-23 03:53:06]

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

[commit-bot: I haz the power, 2017-02-23 03:53:39]

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-02-23 03:56:17]

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

[commit-bot: I haz the power, 2017-02-23 03:56:17]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[karandeepb, 2017-02-23 20:27:51]

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

[commit-bot: I haz the power, 2017-02-23 20:28:11]

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

[karandeepb, 2017-02-23 21:31:04]

karandeepb@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[karandeepb, 2017-02-23 21:31:05]

PTAL Devlin.

https://codereview.chromium.org/2708923005/diff/20001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_api.cc (right):

https://codereview.chromium.org/2708923005/diff/20001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_api.cc:1803: int
ExtensionWebRequestEventRouter::ExecuteDeltas(
This will also include modifications done via the Declarative WR API. I am
assuming that's ok, since its effect will be minimal.

[commit-bot: I haz the power, 2017-02-23 21:40:46]

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

[commit-bot: I haz the power, 2017-02-23 21:40:47]

Dry run: This issue passed the CQ dry run.

[Devlin, 2017-02-24 21:43:43]

lgtm.  Note you'll need a histogram reviewer as well.

https://codereview.chromium.org/2708923005/diff/80001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_api.cc (right):

https://codereview.chromium.org/2708923005/diff/80001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_api.cc:1837: if
(request_headers_modified)
nit: { } around multi-line if statement bodies.

https://codereview.chromium.org/2708923005/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2708923005/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:19206: +    stage of a web request.
We should mention when this is recorded, and how many times - e.g., I don't
think this is necessarily limited to only one recording per web request event,
right?

[karandeepb, 2017-02-25 03:49:21]

Patchset #6 (id:100001) has been deleted

[karandeepb, 2017-02-25 03:52:45]

Patchset #6 (id:120001) has been deleted

[karandeepb, 2017-02-25 03:55:19]

PTAL Devlin.

https://codereview.chromium.org/2708923005/diff/80001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_api.cc (right):

https://codereview.chromium.org/2708923005/diff/80001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_api.cc:1837: if
(request_headers_modified)
On 2017/02/24 21:43:43, Devlin wrote:
> nit: { } around multi-line if statement bodies.

Done.

https://codereview.chromium.org/2708923005/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2708923005/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:19206: +    stage of a web request.
On 2017/02/24 21:43:43, Devlin wrote:
> We should mention when this is recorded, and how many times - e.g., I don't
> think this is necessarily limited to only one recording per web request event,
> right?

Oh yeah, I hadn't thought about it much. Now logging once for each distinct
"requested" action.

[karandeepb, 2017-02-25 03:56:59]

karandeepb@chromium.org changed reviewers:
+ isherman@chromium.org

[karandeepb, 2017-02-25 03:57:00]

PTAL isherman@ for tools/metrics/ review.

[Devlin, 2017-02-25 18:39:34]

lgtm

https://codereview.chromium.org/2708923005/diff/140001/extensions/browser/api...
File extensions/browser/api/web_request/web_request_api.cc (right):

https://codereview.chromium.org/2708923005/diff/140001/extensions/browser/api...
extensions/browser/api/web_request/web_request_api.cc:1877: // actions are used
by extensions.
nit: Might be worth explicitly calling out that multiple actions can be taken in
a single delta execution.

[Ilya Sherman, 2017-02-27 19:29:25]

Metrics LGTM % a nit:

https://codereview.chromium.org/2708923005/diff/140001/extensions/browser/api...
File extensions/browser/api/web_request/web_request_api.cc (right):

https://codereview.chromium.org/2708923005/diff/140001/extensions/browser/api...
extensions/browser/api/web_request/web_request_api.cc:1880: RequestAction::MAX);
Please create a wrapper for emitting to this histogram, so that there's only a
single UMA_HISTOGRAM_ENUMERATION call for it.  The macro expands to a fair bit
of code, so it's helpful for that expansion to only happen once per histogram.

[karandeepb, 2017-02-28 00:52:11]

Description was changed from

==========
Extensions: Add metrics for Web Request API actions.

This CL adds metrics for how the Web Request API is used. The following actions 
are logged per web request stage:
  -CANCEL : Web request is cancelled
  -REDIRECT : Web request is redirected
  -MODIFY_REQUEST_HEADERS : Request headers are modified
  -MODIFY_RESPONSE_HEADERS : Response headers are modified
  -SET_AUTH_CREDENTIALS : Auth credentials are set.

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

to

==========
Extensions: Add metrics for Web Request API actions.

This CL adds metrics for how the Web Request API is used. The following actions 
are logged per web request stage:
  -CANCEL : Web request is cancelled
  -REDIRECT : Web request is redirected
  -MODIFY_REQUEST_HEADERS : Request headers are modified
  -MODIFY_RESPONSE_HEADERS : Response headers are modified
  -SET_AUTH_CREDENTIALS : Auth credentials are set.

BUG=696822
==========

[karandeepb, 2017-02-28 00:56:45]

Also, renamed enum field in histograms.xml to be in line with the actual enum.

https://codereview.chromium.org/2708923005/diff/140001/extensions/browser/api...
File extensions/browser/api/web_request/web_request_api.cc (right):

https://codereview.chromium.org/2708923005/diff/140001/extensions/browser/api...
extensions/browser/api/web_request/web_request_api.cc:1877: // actions are used
by extensions.
On 2017/02/25 18:39:34, Devlin wrote:
> nit: Might be worth explicitly calling out that multiple actions can be taken
in
> a single delta execution.

Done.

https://codereview.chromium.org/2708923005/diff/140001/extensions/browser/api...
extensions/browser/api/web_request/web_request_api.cc:1880: RequestAction::MAX);
On 2017/02/27 19:29:25, Ilya Sherman wrote:
> Please create a wrapper for emitting to this histogram, so that there's only a
> single UMA_HISTOGRAM_ENUMERATION call for it.  The macro expands to a fair bit
> of code, so it's helpful for that expansion to only happen once per histogram.

Done.

[karandeepb, 2017-02-28 00:57:32]

The CQ bit was checked by karandeepb@chromium.org

[karandeepb, 2017-02-28 00:57:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2708923005/#ps180001
(title: "Rename enum WebRequestAction -> RequestAction in histograms.xml to be
in line with the implementation.")

[commit-bot: I haz the power, 2017-02-28 00:58:11]

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-28 02:27:42]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1488243452417530,
"parent_rev": "beb3ea2171919006b6994d0c7a0f964577acfebb", "commit_rev":
"741177620dfb8253a9e35560b37e412202090d2a"}

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

Description was changed from

==========
Extensions: Add metrics for Web Request API actions.

This CL adds metrics for how the Web Request API is used. The following actions 
are logged per web request stage:
  -CANCEL : Web request is cancelled
  -REDIRECT : Web request is redirected
  -MODIFY_REQUEST_HEADERS : Request headers are modified
  -MODIFY_RESPONSE_HEADERS : Response headers are modified
  -SET_AUTH_CREDENTIALS : Auth credentials are set.

BUG=696822
==========

to

==========
Extensions: Add metrics for Web Request API actions.

This CL adds metrics for how the Web Request API is used. The following actions 
are logged per web request stage:
  -CANCEL : Web request is cancelled
  -REDIRECT : Web request is redirected
  -MODIFY_REQUEST_HEADERS : Request headers are modified
  -MODIFY_RESPONSE_HEADERS : Response headers are modified
  -SET_AUTH_CREDENTIALS : Auth credentials are set.

BUG=696822

Review-Url: https://codereview.chromium.org/2708923005
Cr-Commit-Position: refs/heads/master@{#453466}
Committed:
https://chromium.googlesource.com/chromium/src/+/741177620dfb8253a9e35560b37e...
==========

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

Committed patchset #8 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/741177620dfb8253a9e35560b37e...