Network traffic annotation added to service_worker.

Network traffic annotation added to service_worker.

Network traffic annotation is added to network request of:
content/browser/service_worker/service_worker_write_to_cache_job.cc

BUG=656607
Review-Url: https://codereview.chromium.org/2854183006
Cr-Commit-Position: refs/heads/master@{#474995}
Committed: https://chromium.googlesource.com/chromium/src/+/9ca00e8a7155d18f064358504bc460fece4232dc

[Ramin Halavati, 2017-05-03 09:23:11]

rhalavati@chromium.org changed reviewers:
+ michaeln@chromium.org

[Ramin Halavati, 2017-05-03 09:23:11]

We are annotating all network requests in Chromium with a new
NetworkTrafficAnnotation scheme. This allows enterprise users of Chrome to audit
the requests and configure Chrome in a way that meets their security policies
and expectations. Furthermore, it allows us to generate better debugging data in
chrome://net-internals and measure bandwidth consumption on a per-request-type
basis.

I've modified 1 file and added annotation template to it. Please review it and
suggest the required answers for the empty parts (marked with '...'). Please
note that the claims should be thorough and covering all cases. In case you
believe that annotation should be passed to the modified function instead of
originating from it, please tell me to change the CL accordingly.

Please take a look at the protobuf scheme in:
https://cs.chromium.org/chromium/src/tools/traffic_annotation/traffic_annotat...
for the definition of the annotations.

You can find a sample annotation in:
https://cs.chromium.org/chromium/src/components/spellcheck/browser/spelling_s...

Entriprise policy options are here:
https://cs.chromium.org/chromium/src/out/Debug/gen/components/policy/proto/ch...

And more description on enterprise policy settings is here:
http://dev.chromium.org/administrators/policy-list-3

Please tell me if you need any clarifications from my side. If you want to learn
more, see:
https://docs.google.com/document/d/1FWEFTHzdqIA1wHUo_DuSpJjppW22otCMPCOA5NumhJU.

[Ramin Halavati, 2017-05-10 07:10:01]

On 2017/05/03 09:23:11, Ramin Halavati wrote:
> We are annotating all network requests in Chromium with a new
> NetworkTrafficAnnotation scheme. This allows enterprise users of Chrome to
audit
> the requests and configure Chrome in a way that meets their security policies
> and expectations. Furthermore, it allows us to generate better debugging data
in
> chrome://net-internals and measure bandwidth consumption on a per-request-type
> basis.
> 
> I've modified 1 file and added annotation template to it. Please review it and
> suggest the required answers for the empty parts (marked with '...'). Please
> note that the claims should be thorough and covering all cases. In case you
> believe that annotation should be passed to the modified function instead of
> originating from it, please tell me to change the CL accordingly.
> 
> Please take a look at the protobuf scheme in:
>
https://cs.chromium.org/chromium/src/tools/traffic_annotation/traffic_annotat...
> for the definition of the annotations.
> 
> You can find a sample annotation in:
>
https://cs.chromium.org/chromium/src/components/spellcheck/browser/spelling_s...
> 
> Entriprise policy options are here:
>
https://cs.chromium.org/chromium/src/out/Debug/gen/components/policy/proto/ch...
> 
> And more description on enterprise policy settings is here:
> http://dev.chromium.org/administrators/policy-list-3
> 
> Please tell me if you need any clarifications from my side. If you want to
learn
> more, see:
>
https://docs.google.com/document/d/1FWEFTHzdqIA1wHUo_DuSpJjppW22otCMPCOA5NumhJU.

Hi,

A gentle reminder on this.

[michaeln, 2017-05-22 20:43:16]

These requests are similar in nature to the appcache updatejob requests. 

Is this sufficient?

https://codereview.chromium.org/2854183006/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_write_to_cache_job.cc
(right):

https://codereview.chromium.org/2854183006/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_write_to_cache_job.cc:216: })");
    net::DefineNetworkTrafficAnnotation("service_worker_write_to_cache_job", R"(
      semantics {
        sender: "ServiceWorker System"
        description:
          "When a ServiceWorker is registered, it's script and immediate "
          "imports are cached for performance and offline access. The resources
"
          "are periodically updated."
        trigger:
          "User visits a site which registers a ServiceWorker."
        data: "None"
        destination: WEBSITE
      }
      policy {
        cookies_allowed: true
        cookies_store: "user"
        setting:
          "Users can control this feature via the 'Cookies' setting under "
          "'Privacy, Content settings'. If cookies are disabled for a single "
          "site, serviceworkers are disabled for the site only. If they are
totally "
          "disabled, all serviceworker requests will be stopped."
        chrome_policy {
            DefaultCookiesSetting {
              policy_options {mode: MANDATORY}
              DefaultCookiesSetting: 2
            }
          }
      })");

[Ramin Halavati, 2017-05-23 05:02:24]

Patchset #2 (id:20001) has been deleted

[Ramin Halavati, 2017-05-23 05:03:21]

Thank you Michael, annotation updated, please review.

Can data be "Link to service worker resources"?

[michaeln, 2017-05-23 22:38:12]

On 2017/05/23 05:03:21, Ramin Halavati wrote:
> Thank you Michael, annotation updated, please review.
> 
> Can data be "Link to service worker resources"?

In general, I think it'd be good to add as little text as needed.  I don't see
the value adding the data field.

If you do describe the "data", what is that field supposed to be describing? No
data is uploaded (there is no POST body) with these requests, and the data
downloaded isn't a link, its a javascript file.

[michaeln, 2017-05-23 23:33:07]

oh, lgtm as is

[kinuko.yasuda, 2017-05-23 23:43:16]

lgtm/2, as I just saw it

[kinuko, 2017-05-24 00:03:55]

On 2017/05/23 23:43:16, kinuko.yasuda wrote:
> lgtm/2, as I just saw it

omg, from right address now...

[Ramin Halavati, 2017-05-24 07:53:15]

rhalavati@chromium.org changed reviewers:
+ msramek@chromium.org

[Ramin Halavati, 2017-05-24 07:53:16]

Thank you very much.

Martin,
What do you think about data field?

[msramek, 2017-05-26 12:02:21]

LGTM

[Ramin Halavati, 2017-05-26 12:03:45]

The CQ bit was checked by rhalavati@chromium.org

[commit-bot: I haz the power, 2017-05-26 12:04:01]

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-26 13:37:05]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495800225815680,
"parent_rev": "cfacb8361ea54ae30a645d0c7f4a278286cf8606", "commit_rev":
"9ca00e8a7155d18f064358504bc460fece4232dc"}

[commit-bot: I haz the power, 2017-05-26 13:37:15]

Description was changed from

==========
Network traffic annotation added to service_worker.

Network traffic annotation is added to network request of:
content/browser/service_worker/service_worker_write_to_cache_job.cc

BUG=656607
==========

to

==========
Network traffic annotation added to service_worker.

Network traffic annotation is added to network request of:
content/browser/service_worker/service_worker_write_to_cache_job.cc

BUG=656607

Review-Url: https://codereview.chromium.org/2854183006
Cr-Commit-Position: refs/heads/master@{#474995}
Committed:
https://chromium.googlesource.com/chromium/src/+/9ca00e8a7155d18f064358504bc4...
==========

[commit-bot: I haz the power, 2017-05-26 13:37:17]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/9ca00e8a7155d18f064358504bc4...