Network traffic annotation added to url_provision_fetcher.

content/browser/media/url_provision_fetcher.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/media/url_provision_fetcher.h"
 
 #include "base/memory/ptr_util.h"
 #include "content/public/browser/provision_fetcher_factory.h"
 #include "media/base/bind_to_current_loop.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/url_fetcher.h"
 
 using net::URLFetcher;
 
 namespace content {
 
 // Implementation of URLProvisionFetcher.
 
 URLProvisionFetcher::URLProvisionFetcher(
     net::URLRequestContextGetter* context_getter)
     : context_getter_(context_getter) {}
 
 URLProvisionFetcher::~URLProvisionFetcher() {}
 
 void URLProvisionFetcher::Retrieve(
     const std::string& default_url,
     const std::string& request_data,
     const media::ProvisionFetcher::ResponseCB& response_cb) {
   response_cb_ = response_cb;
 
   const std::string request_string =
       default_url + "&signedRequest=" + request_data;
   DVLOG(1) << __func__ << ": request:" << request_string;
 
   DCHECK(!request_);
-  request_ = URLFetcher::Create(GURL(request_string), URLFetcher::POST, this);
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("...", R"(

[Tima Vaisburd, 2017/03/07 18:56:43]

I'm not familiar with NetworkTrafficAnnotation. What is the meaning of "...",
are you supposed to fill this in?

[Ramin Halavati, 2017/03/08 08:52:34]

Sorry I have not been very clear in the message. I need you provide suitable
answers for all "..." positions, so that it will correctly specify why this
network connection is created and what it will do? You can refer to currently
existing examples in CS, also please tell me if I can add any clarification to
any of them.

[Tima Vaisburd, 2017/03/09 00:16:35]

Thank you!

 semantics {
  sender: "ProvisionFetcher for Android MediaDrm"
  description: "Android MediaDrm (a part of encrypted content playback) requires
provisioning of the device. This is a request to provisioning server."
  trigger: "NotProvisionedException from MediaDrm"
  data: "Opaque request generated by Android framework"
  destination: GOOGLE_OWNED_SERVICE or OTHER (?)
 }
 policy {
  cookies_allowed: false
 }

I need xhwang@ to review this.

I do not quite understand the policy section, the request happens automatically
but normally only once for the current device. A user can cause this request
again by deleting the provisioning data from the device.

[Ramin Halavati, 2017/03/09 06:28:47]

Done.

+        semantics {
+          sender: "..."
+          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: "..."
+        })");
+  request_ = URLFetcher::Create(GURL(request_string), URLFetcher::POST, this,
+                                traffic_annotation);

[Tima Vaisburd, 2017/03/07 18:56:43]

I would create another helper method like
   net::NetworkTrafficAnnotationTag CreateTrafficAnnotation(),
wdyt?

[Ramin Halavati, 2017/03/08 08:52:34]

We process this text using a clang tool that extracts it before compilation, and
we avoid any process on it during run-time, so that compiler would optimize them
out of binary. Therefore having a separate function is not preferred unless it
is really necessary or we need to create annotation in one function, then call
another one to continue with network request creation.
One alternative is using a constant outside the class (e.g. at line 17) that
defines this. How do you prefer it?

[Tima Vaisburd, 2017/03/09 00:16:35]

I think it would be better to leave it the way it is for consistency with other
entries. I did not see other entries before my fist comment, sorry.

[Ramin Halavati, 2017/03/09 06:28:46]

Done.

 
   // SetUploadData is mandatory even if we are not uploading anything.
   request_->SetUploadData("", "");
   request_->AddExtraRequestHeader("User-Agent: Widevine CDM v1.0");
   request_->AddExtraRequestHeader("Content-Type: application/json");
 
   DCHECK(context_getter_);
   request_->SetRequestContext(context_getter_);
 
   request_->Start();
@@ skipping 22 matching lines @@
 
 // Implementation of content public method CreateProvisionFetcher().
 
 std::unique_ptr<media::ProvisionFetcher> CreateProvisionFetcher(
     net::URLRequestContextGetter* context_getter) {
   DCHECK(context_getter);
   return base::MakeUnique<URLProvisionFetcher>(context_getter);
 }
 
 }  // namespace content