[Cronet] Set up HttpServerPropertiesManager

net/url_request/url_request_context_builder.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.
 
 // This class is useful for building a simple URLRequestContext. Most creators
 // of new URLRequestContexts should use this helper class to construct it. Call
 // any configuration params, and when done, invoke Build() to construct the
 // URLRequestContext. This URLRequestContext will own all its own storage.
 //
 // URLRequestContextBuilder and its associated params classes are initially
 // populated with "sane" default values. Read through the comments to figure out
 // what these are.
 
 #ifndef NET_URL_REQUEST_URL_REQUEST_CONTEXT_BUILDER_H_
 #define NET_URL_REQUEST_URL_REQUEST_CONTEXT_BUILDER_H_
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
+#include "base/memory/weak_ptr.h"
 #include "build/build_config.h"
 #include "net/base/net_export.h"
 #include "net/base/network_delegate.h"
 #include "net/dns/host_resolver.h"
 #include "net/proxy/proxy_config_service.h"
 #include "net/proxy/proxy_service.h"
 #include "net/quic/quic_protocol.h"
 #include "net/socket/next_proto.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 }
 
 namespace net {
 
 class ChannelIDService;
 class CookieStore;
 class FtpTransactionFactory;
 class HostMappingRules;
 class HttpAuthHandlerFactory;
+class HttpServerProperties;
 class ProxyConfigService;
 class URLRequestContext;
 class URLRequestInterceptor;
 
 class NET_EXPORT URLRequestContextBuilder {
  public:
   struct NET_EXPORT HttpCacheParams {
     enum Type {
       IN_MEMORY,
       DISK,
@@ skipping 145 matching lines @@
   void SetFileTaskRunner(
       const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
 
   // Note that if SDCH is enabled without a policy object observing
   // the SDCH manager and handling at least Get-Dictionary events, the
   // result will be "Content-Encoding: sdch" advertisements, but no
   // dictionaries fetches and no specific dictionaries advertised.
   // SdchOwner in net/sdch/sdch_owner.h is a simple policy object.
   void set_sdch_enabled(bool enable) { sdch_enabled_ = enable; }
 
+  void set_http_server_properties(
+      const base::WeakPtr<HttpServerProperties>& http_server_properties) {

[pauljensen, 2015/06/15 15:57:42]

I'm torn as to whether this should be a scoped_ptr or a WeakPtr. 
URLRequestContext takes a WeakPtr but URLRequestContextStorage takes a
scoped_ptr.  Using a WeakPtr promotes sharing the HttpServerProperties (which is
good for performance) but it forces callers to take ownership (which is added
complexity that a lot of URLRequestContextBuilder clients probably don't want to
entertain).  I'm tempted to go with a scoped_ptr and have
URLRequestContextBuilder take ownership, as it does for set_host_resolver()
which is another preferably share resource.  I guess this depends on how
PrefService is supposed to be used (i.e. one versus man instantiations).

On a side note, this class desperately needs a useful comment, especially
because we encourage its use.  It's frankly terrifying that so many of its
setters take pointers...but take ownership.  Then Build() transfers ownership to
the URLRequestContext, so calling Build() twice will have different results
(i.e. the first generated context get's the owned item, but the second gets
none/default).

[xunjieli, 2015/06/15 18:41:36]

I also prefer that storage takes ownership of the HttpServerPropertiesManager,
but
HttpServerPropertiesManager has this requirement that it needs to be explicitly
shut down on the pref thread
(See HttpServerPropertiesManager::ShutdownOnPrefThread, which the interface
class does not have). And In Cronet, there isn't a pref thread, we just use the
network thread as the pref thread for scheduling updates. This forces me to use
WeakPtr instead to manage the initialization and shutdown of the
HttpServerPropertiesManager instance explicitly. Not sure if there's a work
around.
Agreed.

[xunjieli, 2015/06/15 18:49:22]

I guess if we make storage to take ownership of the HttpServerPropertiesManager.
During shutdown, we can get HttpServerProperties from the URLRequestContext and
do a cast to see if it is an instance of HttpServerPropertiesManager? If yes, we
will invoke ShutDownOnPrefThread?

+    http_server_properties_ = http_server_properties;
+  }
+
   URLRequestContext* Build();
 
  private:
   struct NET_EXPORT SchemeFactory {
     SchemeFactory(const std::string& scheme, HttpAuthHandlerFactory* factory);
     ~SchemeFactory();
 
     std::string scheme;
     HttpAuthHandlerFactory* factory;
   };
@@ skipping 21 matching lines @@
   scoped_ptr<NetLog> net_log_;
   scoped_ptr<HostResolver> host_resolver_;
   scoped_ptr<ChannelIDService> channel_id_service_;
   scoped_ptr<ProxyConfigService> proxy_config_service_;
   scoped_ptr<ProxyService> proxy_service_;
   scoped_ptr<NetworkDelegate> network_delegate_;
   scoped_refptr<CookieStore> cookie_store_;
   scoped_ptr<FtpTransactionFactory> ftp_transaction_factory_;
   std::vector<SchemeFactory> extra_http_auth_handlers_;
   ScopedVector<URLRequestInterceptor> url_request_interceptors_;
+  base::WeakPtr<HttpServerProperties> http_server_properties_;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequestContextBuilder);
 };
 
 }  // namespace net
 
 #endif  // NET_URL_REQUEST_URL_REQUEST_CONTEXT_BUILDER_H_