[Cronet] Set up HttpServerPropertiesManager

net/url_request/url_request_context_builder.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 "net/url_request/url_request_context_builder.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/thread.h"
 #include "net/base/cache_type.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate_impl.h"
 #include "net/base/sdch_manager.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cookies/cookie_monster.h"
 #include "net/dns/host_resolver.h"
 #include "net/ftp/ftp_network_layer.h"
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_cache.h"
 #include "net/http/http_network_layer.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_server_properties_impl.h"
+#include "net/http/http_server_properties_manager.h"
 #include "net/http/transport_security_persister.h"
 #include "net/http/transport_security_state.h"
 #include "net/ssl/channel_id_service.h"
 #include "net/ssl/default_channel_id_store.h"
 #include "net/ssl/ssl_config_service_defaults.h"
 #include "net/url_request/data_protocol_handler.h"
 #include "net/url_request/static_http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_storage.h"
 #include "net/url_request/url_request_intercepting_job_factory.h"
@@ skipping 104 matching lines @@
       file_task_runner_ = file_thread_->task_runner();
     }
     return file_task_runner_;
   }
 
   void set_transport_security_persister(
       scoped_ptr<TransportSecurityPersister> transport_security_persister) {
     transport_security_persister = transport_security_persister.Pass();
   }
 
+  // Sets a HttpServerPropertiesManager. Note that the manager must be
+  // initialized prior to this call.
+  void set_http_server_properties_manager(
+      scoped_ptr<HttpServerPropertiesManager> http_server_properties_manager) {
+    http_server_properties_manager_ = http_server_properties_manager.Pass();
+    set_http_server_properties(http_server_properties_manager_->GetWeakPtr());
+  }
+
  protected:
-  ~BasicURLRequestContext() override { AssertNoURLRequests(); }
+  ~BasicURLRequestContext() override {
+    AssertNoURLRequests();
+    // Shut down the |http_server_properties_manager_| if there is one, since
+    // this is the only place in BasicURLRequestContext to do so.
+    // Note that the shutdown is performed on the network thread.
+    if (http_server_properties_manager_)
+      http_server_properties_manager_->ShutdownOnPrefThread();

[mmenke, 2015/07/07 20:28:12]

Isn't the pref thread actually the file thread?  If so, we're calling this on
the wrong thread.

Solution seems to be to have Cronet keep around a pointer to the manager, and
take care of that before shutting down the context.

Think the order would have to be:

* Post task to do that on the pref (file) thread
* Wait for that task to complete
* Post task to shut down and destroy the context
* Stop the network thread (Which implicitly waits for the previous event to
complete)
* Stop the file thread.

[xunjieli, 2015/07/08 20:07:22]

Why does the pref thread need to be the file thread? It can also be the network
thread, right? The pref thread just needs to do scheduling and listen for pref
updates. It doesn't do disk i/o. Sorry, I tried reading the code, but I don't
really understand the threading requirement. Could you explain a bit more on why
the pref thread is actually the file thread?

[mmenke, 2015/07/08 20:21:48]

I'm not sure.  The pref code doesn't say much about threading.  I had assumed it
was the thread we pass into PrefStore's constructor, I may be wrong.

Regardless, I'd rather not make URLRequestContextBuilder dependent on
HttpServerProperties rather than HttpServerPropertiesManager.  Disallowing use
of HttpServerPropertiesImpl in the interface seems a bit weird, and unnecessary.

[xunjieli, 2015/07/09 15:51:02]

Done. I made the builder take in a weak ptr of the parent class, and manage the
shutdown on cronet side. PTAL.

+  }
 
  private:
   // The thread should be torn down last.
   scoped_ptr<base::Thread> file_thread_;
   scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;
 
   URLRequestContextStorage storage_;
   scoped_ptr<TransportSecurityPersister> transport_security_persister_;
+  scoped_ptr<HttpServerPropertiesManager> http_server_properties_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(BasicURLRequestContext);
 };
 
 }  // namespace
 
 URLRequestContextBuilder::HttpCacheParams::HttpCacheParams()
     : type(IN_MEMORY),
       max_size(0) {}
 URLRequestContextBuilder::HttpCacheParams::~HttpCacheParams() {}
@@ skipping 63 matching lines @@
   DCHECK(cookie_store);
   cookie_store_ = cookie_store;
   channel_id_service_ = channel_id_service.Pass();
 }
 
 void URLRequestContextBuilder::SetFileTaskRunner(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   file_task_runner_ = task_runner;
 }
 
+void URLRequestContextBuilder::SetHttpServerPropertiesManager(
+    scoped_ptr<HttpServerPropertiesManager> http_server_properties_manager) {
+  http_server_properties_manager_ = http_server_properties_manager.Pass();
+}
+
 URLRequestContext* URLRequestContextBuilder::Build() {
   BasicURLRequestContext* context =
       new BasicURLRequestContext(file_task_runner_);
   URLRequestContextStorage* storage = context->storage();
 
   storage->set_http_user_agent_settings(new StaticHttpUserAgentSettings(
       accept_language_, user_agent_));
 
   if (!network_delegate_)
     network_delegate_.reset(new BasicNetworkDelegate);
@@ skipping 63 matching lines @@
   storage->set_transport_security_state(new TransportSecurityState());
   if (!transport_security_persister_path_.empty()) {
     context->set_transport_security_persister(
         make_scoped_ptr<TransportSecurityPersister>(
             new TransportSecurityPersister(context->transport_security_state(),
                                            transport_security_persister_path_,
                                            context->GetFileTaskRunner(),
                                            false)));
   }
 
-  storage->set_http_server_properties(
-      scoped_ptr<HttpServerProperties>(new HttpServerPropertiesImpl()));
+  if (http_server_properties_manager_) {
+    context->set_http_server_properties_manager(
+        http_server_properties_manager_.Pass());
+  } else {
+    storage->set_http_server_properties(
+        scoped_ptr<HttpServerProperties>(new HttpServerPropertiesImpl()));
+  }
+
   storage->set_cert_verifier(CertVerifier::CreateDefault());
 
   if (throttling_enabled_)
     storage->set_throttler_manager(new URLRequestThrottlerManager());
 
   HttpNetworkSession::Params network_session_params;
   network_session_params.host_resolver = context->host_resolver();
   network_session_params.cert_verifier = context->cert_verifier();
   network_session_params.transport_security_state =
       context->transport_security_state();
@@ skipping 80 matching lines @@
     }
     url_request_interceptors_.weak_clear();
   }
   storage->set_job_factory(top_job_factory.release());
   // TODO(willchan): Support sdch.
 
   return context;
 }
 
 }  // namespace net