Network traffic annotation added to sync.

components/sync/driver/sync_stopped_reporter.cc

Index: components/sync/driver/sync_stopped_reporter.cc
diff --git a/components/sync/driver/sync_stopped_reporter.cc b/components/sync/driver/sync_stopped_reporter.cc
index 7daf57c25eba6a07d5b6d0d60b34291a11cafef0..e28a0eb2fb4df79cd00c26b9892a2bd03c44bc78 100644
--- a/components/sync/driver/sync_stopped_reporter.cc
+++ b/components/sync/driver/sync_stopped_reporter.cc
@@ -13,6 +13,7 @@
 #include "components/sync/protocol/sync.pb.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 
 namespace {
 
@@ -61,8 +62,29 @@ void SyncStoppedReporter::ReportSyncStopped(const std::string& access_token,
   std::string msg;
   event_request.SerializeToString(&msg);
 
-  fetcher_ =
-      net::URLFetcher::Create(sync_event_url_, net::URLFetcher::POST, this);
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("...", R"(

[Nicolas Zea, 2017/02/23 22:15:54]

It would be nice if we didn't have to repeat this literal across all these
files. Perhaps hard code it somewhere?

[Ramin Halavati, 2017/02/24 07:53:03]

Please elaborate me if I didn't understand your comment correctly.

The main annotation text will be dropped off during compile time and the
hardcoded |unique_id| will be kept and a pointer to that will be passed to other
functions.

As we want the annotation text not to be part of binary and run-time, we need it
to be in the same compilation unit as where it is used, to extract it using
clang tools.

So if there are two usage sites that need to have similar annotation, might we
have a function that creates URLFetcher for both of them and assigns the
annotation to it there?

[Nicolas Zea, 2017/02/24 20:37:38]

I was just thinking have the R"..." string be saved into a header (maybe within
net/traffic_annotation?) as a constant, and then use that constant here and in
the other sync callsites (or any other places with the same semantics/policy.
I'm mainly thinking about readability/maintainability of the code.

That said, I'm not sure how you're planning to use these annotations. If we had
say a GOOGLE_OWNED constant (or maybe even macro?) used at the sync callsites,
would that defeat what you're trying to accomplish?

[Ramin Halavati, 2017/02/27 09:23:16]

Current plan is extraction of R"..." strings using clang tools in separate
reporting and auditing tools and use them there. Clang tool considers the R"..."
boundary and does not apply macros or constants to it.
I think serialized protobuf is quite readable in code and the reports will be
created after parsing the protobufs. A presubmit tool will be added shortly to
check it the format is correct to help maintainability.

battre@:
Any comments?

+        semantics {
+          sender: "..."

[Nicolas Zea, 2017/03/03 22:39:55]

sender: "chrome sync"
description: "A network request to inform Chrome Sync that sync has been
disabled for this device."
trigger: "User disabled sync."
data: "the sync device id and store birthday"
cookies: false
cookies_store: false
settings: "NA"
policy: UNSET
policy exception: "This network request is only performed after the user has
opted in to sync and then disables sync"

[Ramin Halavati, 2017/03/06 06:55:45]

Thanks, could you elaborate on "Store Birthday"

Can't we use 'SyncDisabled' as a policy here as well? By preventing enabling
Sync, disabling it will also be prevented.

[Nicolas Zea, 2017/03/06 17:49:20]

Store birthday is the identifier for the lifetime of the account data that gets
reset every time the user clears their sync data via the google dashboard.
Perhaps we can just refer to this as "sync device identifier and metadata"?
That's true. I wasn't sure if the policy was meant to be tied to the specific
request or not. I'm fine with repeating SyncDsabled here.

[Ramin Halavati, 2017/03/07 08:53:47]

Done.

+          description: "..."
+          trigger: "..."
+          data: "..."
+          destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER
+        }
+        policy {
+          cookies_allowed: false/true
+          cookies_store: "..."
+          setting: "..."
+          policy {
+            [POLICY_NAME] {
+              policy_options {mode: MANDATORY/RECOMMENDED/UNSET}
+              value: ...
+            }
+          }
+          policy_exception_justification: "..."
+        })");
+  fetcher_ = net::URLFetcher::Create(sync_event_url_, net::URLFetcher::POST,
+                                     this, traffic_annotation);
   fetcher_->AddExtraRequestHeader(base::StringPrintf(
       "%s: Bearer %s", net::HttpRequestHeaders::kAuthorization,
       access_token.c_str()));