Network traffic annotation added to image_data_fetcher.

chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc

Index: chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc
diff --git a/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc b/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc
index 01ebd602a2c4d7a2368a41267951a3fe147abce1..4152d0a07bd13068b3b2cd154dee3b301e36ed48 100644
--- a/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc
+++ b/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc
@@ -26,6 +26,7 @@
 #include "components/ntp_tiles/icon_cacher_impl.h"
 #include "components/ntp_tiles/metrics.h"
 #include "components/ntp_tiles/most_visited_sites.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 
 using suggestions::SuggestionsServiceFactory;
 
@@ -113,6 +114,27 @@ ChromeMostVisitedSitesFactory::NewForProfile(Profile* profile) {
     return nullptr;
   }
 
+  net::NetworkTrafficAnnotationTag traffic_annotation =

[Marc Treib, 2017/05/09 09:43:46]

icon_cacher_traffic_annotation

[Ramin Halavati, 2017/05/09 11:08:23]

Done.

+      net::DefineNetworkTrafficAnnotation("...", R"(
+        semantics {
+          sender: "..."

[sfiera, 2017/05/09 10:08:17]

Popular Sites New Tab Fetch

[Ramin Halavati, 2017/05/09 11:08:23]

Done.

+          description: "..."

[sfiera, 2017/05/09 10:08:17]

Google Chrome may display a list of regionally-popular web sites on the New Tab
Page. This service fetches icons from those sites.

[Ramin Halavati, 2017/05/09 11:08:23]

Done.

+          trigger: "..."

[sfiera, 2017/05/09 10:08:17]

Whenever a popular site would be displayed, but its icon is not yet cached in
the browser.

[Ramin Halavati, 2017/05/09 11:08:23]

Done.

+          data: "..."

[sfiera, 2017/05/09 10:08:17]

None

[Ramin Halavati, 2017/05/09 11:08:23]

Shouldn't it be URL of the required icon?

[sfiera, 2017/05/09 11:20:20]

Well, yes, all HTTP requests include the URL of the thing they're requesting,
but is that useful to list here?

[Ramin Halavati, 2017/05/09 13:16:47]

Acknowledged.

+          destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER/LOCAL

[sfiera, 2017/05/09 10:08:17]

WEBSITE

[Ramin Halavati, 2017/05/09 11:08:23]

Done.

+        }
+        policy {
+          cookies_allowed: false/true

[sfiera, 2017/05/09 10:08:17]

false

[Ramin Halavati, 2017/05/09 11:08:23]

Done.

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

[sfiera, 2017/05/09 10:08:17]

This is in progress—an experiment exists, but nothing generally available yet.

[Ramin Halavati, 2017/05/09 11:08:23]

Done.

+          chrome_policy {
+            [POLICY_NAME] {
+              policy_options {mode: MANDATORY/RECOMMENDED/UNSET}
+              [POLICY_NAME]: ... //(value to disable it)
+            }
+          }
+          policy_exception_justification: "..."
+        })");
   return base::MakeUnique<ntp_tiles::MostVisitedSites>(
       profile->GetPrefs(), TopSitesFactory::GetForProfile(profile),
       SuggestionsServiceFactory::GetForProfile(profile),
@@ -126,6 +148,6 @@ ChromeMostVisitedSitesFactory::NewForProfile(Profile* profile) {
               profile, ServiceAccessType::IMPLICIT_ACCESS),
           base::MakeUnique<image_fetcher::ImageFetcherImpl>(
               base::MakeUnique<suggestions::ImageDecoderImpl>(),
-              profile->GetRequestContext())),
+              profile->GetRequestContext(), traffic_annotation)),

[sfiera, 2017/05/09 10:08:17]

Isn't it necessary to update the iOS factory too?

It feels wrong to me for the factory to declare this and not the code that uses
the fetcher. Can this be bassed as part of StartOrQueueNetworkRequest()?

[Ramin Halavati, 2017/05/09 11:08:23]

I didn't quite get it. You mean instead of constructor of image_fetcher_imp,
annotation would be given to StartOrQueueNetworkRequest?

[sfiera, 2017/05/09 11:20:20]

Yes. I think it's better for the annotation to be declared once by
IconCacherImpl, rather than multiple times by different factories. Only
IconCacherImpl can really say how it uses the image fetcher.

       base::MakeUnique<SupervisorBridge>(profile));
 }