Refactoring DownloadManager::DownloadURL to add proper annotation.

content/browser/download/download_request_core.cc

 // Copyright (c) 2012 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/download/download_request_core.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 22 matching lines @@
 #include "content/public/browser/web_contents.h"
 #include "device/power_save_blocker/power_save_blocker.h"
 #include "net/base/elements_upload_data_stream.h"
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/url_request_context.h"
 
 namespace content {
 
 namespace {
 
 // This is a UserData::Data that will be attached to a URLRequest as a
 // side-channel for passing download parameters.
 class DownloadRequestData : public base::SupportsUserData::Data {
  public:
@@ skipping 56 matching lines @@
 // static
 std::unique_ptr<net::URLRequest> DownloadRequestCore::CreateRequestOnIOThread(
     uint32_t download_id,
     DownloadUrlParameters* params) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(download_id == DownloadItem::kInvalidId ||
          !params->content_initiated())
       << "Content initiated downloads shouldn't specify a download ID";
   DCHECK(params->offset() >= 0);
 
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("...", R"(

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

download_manager_url_request

+        semantics {
+          sender: "..."

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

download_manager

+          description: "..."

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

A download request that can come from a variety of sources (see trigger) and
will download a file and save it to disk.

+          trigger: "..."

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

This needs to be filled in based on who is building the DownloadUrlParameters. 
I'd suggest (from the call sites I can find).  Let me know if you think there
are better naming suggestions!

downloads_api.cc: "extension download API"
webstore_installer.cc: "extension install"
render_view_context_menu.cc: "save link as"
background_fetch_job_controller.cc: "background fetch API"
drag_download_file.cc: "drag file to save"
render_frame_message_filter.cc: "user navigation triggered download"
indexed_db_internals_ui.cc: "saving internal indexed db data"
web_contents_impl.cc: "user saved page"
download_worker.cc: "parallel download request"
download_manager_impl.cc: "download resumption"

+          data: "..."

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

Could you clarify, do we want to describe the data we're sending or receiving?

If it's sending, this probably also has a lot to do with the request.  Many of
these we aren't sure because they're triggered by an extension or a site.

+          destination: WEBSITE/GOOGLE_OWNED_SERVICE/OTHER/LOCAL

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

WEBSITE.  Although in the future we'll need GOOGLE_OWNED_SERVICE too.  We can
defer that for now and make sure it gets added once DownloadService gets built
out.  This is the field that we probably need to persist beyond restarts.

+        }
+        policy {
+          cookies_allowed: false/true

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

true.  It depends on the caller and which UrlRequestContext gets built.  We
might need to pass this in as well if we want specifics.  Let me know if we need
to be more detailed here.  Will look into the call sites listed above for
"trigger" for this as well.

+          cookies_store: "..."

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

Same as above.  Probably depends on the call site.

+          setting: "..."

[David Trainor- moved to gerrit, 2017/05/23 01:07:40]

Also depends on the call site.  Sadly it looks like a bunch of the above might
:(.  It might be best to require the DownloadManager::DownloadUrl (and download
internal call sites) to build a few pieces of this.  Or maybe a helper function
where they pass in all the optional parameters and we build the annotation for
them.

+          chrome_policy {
+            [POLICY_NAME] {
+              policy_options {mode: MANDATORY/RECOMMENDED/UNSET}
+              [POLICY_NAME]: ... //(value to disable it)
+            }
+          }
+          policy_exception_justification: "..."
+        })");
   // ResourceDispatcherHost{Base} is-not-a URLRequest::Delegate, and
   // DownloadUrlParameters can-not include resource_dispatcher_host_impl.h, so
   // we must down cast. RDHI is the only subclass of RDH as of 2012 May 4.
   std::unique_ptr<net::URLRequest> request(
       params->url_request_context_getter()
           ->GetURLRequestContext()
-          ->CreateRequest(params->url(), net::DEFAULT_PRIORITY, nullptr));
+          ->CreateRequest(params->url(), net::DEFAULT_PRIORITY, nullptr,
+                          traffic_annotation));
   request->set_method(params->method());
 
   if (!params->post_body().empty()) {
     const std::string& body = params->post_body();
     std::unique_ptr<net::UploadElementReader> reader(
         net::UploadOwnedBytesElementReader::CreateWithString(body));
     request->set_upload(
         net::ElementsUploadDataStream::CreateWithReader(std::move(reader), 0));
   }
 
@@ skipping 554 matching lines @@
   // old servers that didn't implement "If-Match" and must be ignored when
   // "If-Match" presents.
   if (has_last_modified) {
     request->SetExtraRequestHeaderByName(
         net::HttpRequestHeaders::kIfUnmodifiedSince, params->last_modified(),
         true);
   }
 }
 
 }  // namespace content