Network traffic annotation added to gcm_network_channel.

components/invalidation/impl/gcm_network_channel.cc

Index: components/invalidation/impl/gcm_network_channel.cc
diff --git a/components/invalidation/impl/gcm_network_channel.cc b/components/invalidation/impl/gcm_network_channel.cc
index 5b2aafeb81a01c78ad6d420f9ff989f56673e8ed..fd9ebe1b6b22247141e0202c7e8c4e4c7a9e99b0 100644
--- a/components/invalidation/impl/gcm_network_channel.cc
+++ b/components/invalidation/impl/gcm_network_channel.cc
@@ -20,6 +20,7 @@
 #include "components/invalidation/impl/gcm_network_channel_delegate.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "net/http/http_status_code.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
 
@@ -217,8 +218,30 @@ void GCMNetworkChannel::OnGetTokenComplete(
   access_token_ = token;
 
   DVLOG(2) << "Got access token, sending message";
-  fetcher_ = net::URLFetcher::Create(BuildUrl(registration_id_),
-                                     net::URLFetcher::POST, this);
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("...", R"(
+        semantics {
+          sender: "..."

[dcheng, 2017/02/23 19:15:12]

To help me understand, is this something that will be filled in more later? What
does ... mean?

(I'm not hugely enthusiastic about included text encoded protos as literals here
either, as there's zero compile-time validation that the string blob is valid,
but perhaps there are unavoidable design constraints here...)

[Ramin Halavati, 2017/02/24 10:16:00]

Sorry that the first message was not clear enough. The |...|s are the places
that I've asked you to suggest text for them.

As we wanted to exclude this text from binary code, we decided on a method were
there is no process on annotation text during compile time and therefore it will
be optimized out of the binary. A clang tool for presubmit check will be added
in next steps to check them before submission. (I've asked you to review its
first version for local use in https://codereview.chromium.org/2448133006/ and
will write the second version for presubmit once that one is landed.)

[dcheng, 2017/02/25 00:17:43]

I see. I'm probably not a good reviewer for this CL then.

+pavely, do you mind helping with the labelling here?

[pavely, 2017/02/27 19:29:41]

sender: Invalidation service

[Ramin Halavati, 2017/02/28 11:57:03]

Done.

+          description: "..."

[pavely, 2017/02/27 19:29:41]

Chrome uses cacheinvalidation library to receive push notifications from server
about sync items (bookmarks, passwords, preferences, etc.) modified on other
clients. It uses GCMClient to receive incoming messages. This request is used
for client-to-server communications. 

[Ramin Halavati, 2017/02/28 11:57:03]

Done.

+          trigger: "..."

[pavely, 2017/02/27 19:29:41]

The first message is sent to register client with the server on chrome startup.
It is then sent periodically to confirm that client is still online.
After receiving notification about server changes client sends this request to
acknowledge that notification is processed.

[Ramin Halavati, 2017/02/28 11:57:03]

Done.

+          data: "..."

[Ramin Halavati, 2017/02/28 11:57:04]

Please specify what data is sent.

[pavely, 2017/02/28 18:38:49]

No PII data is sent in this request. 
Request contains client_token generated on the server. It includes invalidation
object ids (not specific to client) and versions when confirming invalidations.
When updating registrations request includes object ids.

[Ramin Halavati, 2017/03/01 08:35:02]

Done.

+          destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER

[pavely, 2017/02/27 19:29:41]

destination: GOOGLE_OWNED_SERVICE

[Ramin Halavati, 2017/03/01 08:35:01]

Done.

+        }
+        policy {
+          cookies_allowed: false/true

[pavely, 2017/02/27 19:29:41]

cookies_allowed: false

[Ramin Halavati, 2017/02/28 11:57:03]

It seems to me that cookies are not disabled. If they are not required for this
request, I can create a CL that disables the cookies and then base this CL on
top of that.

[pavely, 2017/02/28 18:38:49]

I believe cookies are not required. Authentication is done through HTTP header.
Feel free to disable cookies.

[Ramin Halavati, 2017/03/01 08:35:02]

https://codereview.chromium.org/2723043002 created to disable cookies.

+          cookies_store: "..."
+          setting: "..."

[pavely, 2017/02/27 19:29:41]

Invalidation service is enabled by default. I don't think there is a way to
disable it by flag/ command line option or server experiment.

[Ramin Halavati, 2017/02/28 11:57:03]

Isn't it disabled if Sync is disabled?

[pavely, 2017/02/28 18:38:49]

Apart from Sync, InvalidationService is used by couple more components (cloud
policy, ChildAccountInfoFetcher). You can see them in the list of registrars in
about:invalidations. There is no code that blocks InvalidationService from
starting if some consumer requests it, but if no consumer invokes
InvalidationService then these requests wouldn't be sent.
I know if Sync is disabled it is not triggering InvalidationService
initialization. I guess other consumers only use InvalidationService when user
is signed in, but I'm not 100% sure.

[Ramin Halavati, 2017/03/01 08:35:01]

Acknowledged.

+          policy {
+            [POLICY_NAME] {

[Ramin Halavati, 2017/02/28 11:57:03]

Isn't this related?

http://dev.chromium.org/administrators/policy-list-3#SyncDisabled

[pavely, 2017/02/28 18:38:49]

This policy disables sync, it doesn't affect other services. (see my other
comment)

[Ramin Halavati, 2017/03/01 08:35:02]

Acknowledged.

+              policy_options {mode: MANDATORY/RECOMMENDED/UNSET}
+              value: ...
+            }
+          }
+          policy_exception_justification: "..."

[Ramin Halavati, 2017/02/28 11:57:03]

Do you think it's useful to have a policy to disable this feature?

[pavely, 2017/02/28 18:38:49]

Disabling InvalidationService might break features that depend on it. I think it
makes sense to control top level features that use InvalidationService.

[Ramin Halavati, 2017/03/01 08:35:01]

Done.

+        })");
+  fetcher_ =
+      net::URLFetcher::Create(BuildUrl(registration_id_), net::URLFetcher::POST,
+                              this, traffic_annotation);
   data_use_measurement::DataUseUserData::AttachToFetcher(
       fetcher_.get(), data_use_measurement::DataUseUserData::INVALIDATION);
   fetcher_->SetRequestContext(request_context_getter_.get());