Delay generation of User-Agent header to URLRequestHttpJob and accept custom User-Agent from XHR/Fe…

net/url_request/url_request_http_job.cc

Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index d2a4797bff6a8310d6aede6477d1cf2198a2a28b..8ca8392ee689560eb1d94c1deaa87442cc6ca415 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -387,6 +387,11 @@ void URLRequestHttpJob::Start() {
 
   request_info_.token_binding_referrer = request_->token_binding_referrer();
 
+  if (request_->has_default_user_agent()) {
+    request_info_.extra_headers.SetHeaderIfMissing(
+        HttpRequestHeaders::kUserAgent, request_->default_user_agent());

[mmenke, 2016/08/21 14:37:39]

I really don't think we need two different wants to set the default user agent,
and I'd rather we shrink the URLRequestJob / URLRequest dependencies, rather
than expand them.  Note, for instance, that we're not setting this one (Or the
other one!) in URLRequestFtpJob when going through an HTTP proxy, which seems
like a bug.

Is the issue that devtools also sets it, and you want a way to sneak it through
the ServiceWorker URLRequestJob subclass without actually setting the header?

[tyoshino (SeeGerritForStatus), 2016/08/23 10:01:47]

Right. I'd like to propagate DevTools' User-Agent value override to net/ stack
but outside of the headers holding members in the request holding classes, and
have the header generated in URLRequestHttpJob (or yeah, Ftp job may, too).

The override is per request info. So, URLRequestContext is not right place to
handle it. Patching URLRequestContext for each request also doesn't sound the
right thing to do.

So, I've expanded it into the URLRequest as a member variable as well as other
per-request info such as method_, referrer_, ... and let URLRequestHttpJob to
pick it.

[mmenke, 2016/08/23 14:54:33]

Could ServiceWorker just hide the header it passes to the Javascript API, and
then re-set it itself?  Fetch API doesn't allow setting of this header, so
there's no case in which anything else is allowed to set it.  I'm reluctant to
expand URLRequest's surface area here, and adding yet another complex and
unexpected behavior.

[tyoshino (SeeGerritForStatus), 2016/08/25 04:11:34]

I think embedding everything into the header holder is also source of complexity
and bugs. But I understand your sentiment. I'll check if there's any way to make
both happy.

[mmenke, 2016/08/25 04:17:01]

SGTM.  I think it's also worth noting that the behavior
URLRequest::set_default_user_agent can't really be documented, without talking
about how it's different from setting the header directly, and ServiceWorker
(Which is not a net/ layer concept)

+  }
+
   request_info_.extra_headers.SetHeaderIfMissing(
       HttpRequestHeaders::kUserAgent,
       http_user_agent_settings_ ?