Network traffic annotation added to popular_sites_impl.

components/ntp_tiles/popular_sites_impl.cc

Index: components/ntp_tiles/popular_sites_impl.cc
diff --git a/components/ntp_tiles/popular_sites_impl.cc b/components/ntp_tiles/popular_sites_impl.cc
index 566202b07d88c8cc228312ce4c98dbf0a5776b03..688f6349a957902e1a2b37ac7c1e42c75e95071e 100644
--- a/components/ntp_tiles/popular_sites_impl.cc
+++ b/components/ntp_tiles/popular_sites_impl.cc
@@ -31,6 +31,7 @@
 #include "components/variations/variations_associated_data.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 
 #if defined(OS_ANDROID) || defined(OS_IOS)
 #include "base/json/json_reader.h"
@@ -306,7 +307,29 @@ void PopularSitesImpl::RegisterProfilePrefs(
 }
 
 void PopularSitesImpl::FetchPopularSites() {
-  fetcher_ = URLFetcher::Create(pending_url_, URLFetcher::GET, this);
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("...", R"(

[Marc Treib, 2017/02/22 09:49:37]

popular_sites or popular_sites_fetch?

[sfiera, 2017/02/22 10:06:12]

Should we prefix "popular sites" with "new tab page" where it appears here and
below? If these identifiers are going to be user-visible, then we should make
sure not to imply that this is the bandwidth that popular sites are using.

[Ramin Halavati, 2017/02/22 10:35:08]

The first identifier is mostly a unique (meaningful) id, and the sender would be
used in reports and needs to be human readable and 

+        semantics {
+          sender: "..."

[Marc Treib, 2017/02/22 09:49:37]

popular sites

[Ramin Halavati, 2017/02/22 10:35:08]

Done.

+          description: "..."

[Marc Treib, 2017/02/22 09:49:37]

Google Chrome may display a list of regionally-popular web sites on the New Tab
Page.

[sfiera, 2017/02/22 10:06:12]

Do we not disable it for enterprise users, as we do articles for you?

[Marc Treib, 2017/02/22 12:16:37]

No, I don't think so.
Shouldn't be too hard to set up, but AFAIK nobody has asked for it.

+          trigger: "..."

[Marc Treib, 2017/02/22 09:49:37]

Triggered automatically once per day, unless no popular web sites are required
(because the New Tab Page is filled with suggestions based on the user's
browsing history).

[Ramin Halavati, 2017/02/22 10:35:08]

Done.

+          data: "..."

[Marc Treib, 2017/02/22 09:49:37]

A two-letter country code based on the user's location.

[Ramin Halavati, 2017/02/22 10:35:08]

Done.

+          destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER

[Marc Treib, 2017/02/22 09:49:37]

GOOGLE_OWNED_SERVICE

[Ramin Halavati, 2017/02/22 10:35:08]

Done.

+        }
+        policy {
+          cookies_allowed: false/true

[Marc Treib, 2017/02/22 09:49:36]

false

[Ramin Halavati, 2017/02/22 10:35:09]

Done.

+          cookies_store: "..."

[Marc Treib, 2017/02/22 09:49:37]

n/a

[Ramin Halavati, 2017/02/22 10:35:09]

Done.

+          setting: "..."

[Marc Treib, 2017/02/22 09:49:36]

NA

[Ramin Halavati, 2017/02/22 10:35:09]

Done.

+          policy {
+            [POLICY_NAME] {
+              policy_options {mode: MANDATORY/RECOMMENDED/UNSET}
+              value: ...
+            }
+          }
+          policy_exception_justification: "..."

[Marc Treib, 2017/02/22 09:49:37]

Not sure - since we don't send any PII, I think a policy is generally not
required?

[Ramin Halavati, 2017/02/22 10:35:08]

But as you are sending user's country, it might cause a privacy concern. 
battre@:
Any comments?

+        })");
+  fetcher_ = URLFetcher::Create(pending_url_, URLFetcher::GET, this,
+                                traffic_annotation);
   data_use_measurement::DataUseUserData::AttachToFetcher(

[sfiera, 2017/02/22 10:06:12]

Will this new feature eventually supersede data_use_measurement?

[Ramin Halavati, 2017/02/22 10:35:09]

I don't think so.

battre@:
Any comments?

[Marc Treib, 2017/02/22 12:16:37]

I wondered the same thing: We now have two things that should be attached to
every URLFetcher. But since they have completely different purposes, I'm not
sure if it makes sense to merge them.

       fetcher_.get(), data_use_measurement::DataUseUserData::NTP_TILES);
   fetcher_->SetRequestContext(download_context_);