Provide mutable members of UrlRequestContext via pure-virtual interface

net/url_request/http_user_agent_settings.h

+// 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.
+
+#ifndef NET_URL_REQUEST_HTTP_USER_AGENT_SETTINGS_H_
+#define NET_URL_REQUEST_HTTP_USER_AGENT_SETTINGS_H_
+
+#include <string>
+
+#include "net/base/net_export.h"
+
+class GURL;
+
+namespace net {
+
+// A pure-virtual interface that provides the HTTP Accept-Language,

[erikwright (departed), 2012/09/25 21:06:38]

"The interface used by HTTP jobs to retrieve HTTP Accept-Language... header
values."

My personal opinion is that "An instance ... can be used..." is probably
unnecessary. 

+// Accept-Charset and User-Agent header values.  An instance of an
+// implementation of this interface can be used by calling
+// |URLRequestContext::set_http_user_agent_settings()|.
+class NET_EXPORT HttpUserAgentSettings {
+ public:
+  // Gets the value of 'Accept-Language' header field.
+  virtual const std::string& GetAcceptLanguage() = 0;

[erikwright (departed), 2012/09/25 21:06:38]

Matt/Will, I'd like an opinion on this:

Returning a string reference seems error-prone and also restrictive. I suppose
an implementation might want to return a generated value, which would currently
be tricky.

I did a grep and found the following:

src$ git grep -E '^\s+std::string\s+[A-Z][a-zA-Z0-9_]*\(' | wc -l
818
src$ git grep -E '^\s+const\s+std::string&\s+[A-Z][a-zA-Z0-9_]*\(' | wc -l
75
src$ git grep -E '^\s+const\s+std::string&\s+[a-z][a-zA-Z0-9_]*\(' | wc -l
535
src$ git grep -E '^\s+std::string\s+[a-z][a-zA-Z0-9_]*\(' | wc -l
1786

So apart from accessors, the convention seems to be 10x in favor of value-type
and for accessors it's still 3x.

On the other hand, I guess the pre-existing API was already a const-ref so we
aren't introducing new risk. And I suppose these will be accessed relatively
frequently (though I tend to be in the camp of not pre-maturely optimizing).

[mmenke, 2012/09/26 16:28:20]

Don't think your grepping is quite correct - any method name that starts with a
lowercase letter, and then later has a capital in it is not following the Chrome
style guide.

Correcting the search and skimming over the hits, looks to me like a lot of the
functions are actually not simple accessors, so are violating the style guide,
and others are local variable declarations of the form "std::string
blah(source_string);".

That having been said, I think your concern is a valid one, and I don't think
performance is a concern here.  If we're going to do this change, think it's
best to do in this CL, and do it to all three functions.  That way, when we're
getting rid of class to the old URLRequestContext functions, we can update call
sites as needed to avoid extra copying.

+  // Gets the value of 'Accept-Charset' header field.

[erikwright (departed), 2012/09/25 21:06:38]

Blank line between declarations.

+  virtual const std::string& GetAcceptCharset() = 0;
+  // Gets the UA string to use for the given URL.  Pass an empty URL to get
+  // the default UA string.
+  virtual const std::string& GetUserAgent(const GURL& url) = 0;
+
+  HttpUserAgentSettings() {}
+  virtual ~HttpUserAgentSettings() {}
+};
+
+}  // namespace net
+
+#endif  // NET_URL_REQUEST_HTTP_USER_AGENT_SETTINGS_H_
+