[Cronet] Set up HttpServerPropertiesManager

components/cronet/android/cronet_url_request_context_adapter.cc

Index: components/cronet/android/cronet_url_request_context_adapter.cc
diff --git a/components/cronet/android/cronet_url_request_context_adapter.cc b/components/cronet/android/cronet_url_request_context_adapter.cc
index 5ece786594214f7dd171a4351c9a1633c0841499..4d641ba84299a957bd11bba6bab46819be151d13 100644
--- a/components/cronet/android/cronet_url_request_context_adapter.cc
+++ b/components/cronet/android/cronet_url_request_context_adapter.cc
@@ -11,8 +11,13 @@
 #include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_vector.h"
+#include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_filter.h"
+#include "base/prefs/pref_registry_simple.h"
+#include "base/prefs/pref_service.h"
+#include "base/prefs/pref_service_factory.h"
 #include "base/single_thread_task_runner.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "components/cronet/url_request_context_config.h"
 #include "jni/CronetUrlRequestContext_jni.h"
@@ -20,6 +25,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate_impl.h"
 #include "net/http/http_auth_handler_factory.h"
+#include "net/http/http_server_properties_manager.h"
 #include "net/log/write_to_file_net_log_observer.h"
 #include "net/proxy/proxy_service.h"
 #include "net/sdch/sdch_owner.h"
@@ -33,6 +39,8 @@
 
 namespace {
 
+const char kHttpServerProperties[] = "net.http_server_properties";
+
 class BasicNetworkDelegate : public net::NetworkDelegateImpl {
  public:
   BasicNetworkDelegate() {}
@@ -128,6 +136,11 @@ CronetURLRequestContextAdapter::CronetURLRequestContextAdapter(
 
 CronetURLRequestContextAdapter::~CronetURLRequestContextAdapter() {
   DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
+
+  if (http_server_prperties_manager_)
+    http_server_properties_manager_->ShutdownOnPrefThread();
+  if (pref_service_)
+    pref_service_->CommitPendingWrite();
   StopNetLogOnNetworkThread();
 }
 
@@ -184,14 +197,27 @@ void CronetURLRequestContextAdapter::InitializeOnNetworkThread(
   config->ConfigureURLRequestContextBuilder(&context_builder);
 
   // Set up pref file if storage path is specified.
-  // TODO(xunjieli): maybe get rid of the condition on sdch.
-  if (!config->storage_path.empty() && config->enable_sdch) {
+  if (!config->storage_path.empty()) {
     base::FilePath filepath(config->storage_path);
     filepath = filepath.Append(FILE_PATH_LITERAL("local_prefs.json"));
     json_pref_store_ = new JsonPrefStore(
         filepath, GetFileThread()->task_runner(), scoped_ptr<PrefFilter>());
-    json_pref_store_->ReadPrefsAsync(nullptr);
     context_builder.SetFileTaskRunner(GetFileThread()->task_runner());
+
+    // Set up HttpServerPropertiesManager.
+    base::PrefServiceFactory factory;
+    factory.set_user_prefs(json_pref_store_);
+    scoped_refptr<PrefRegistrySimple> registry = new PrefRegistrySimple;

[mmenke, 2015/07/09 19:08:10]

optional:  Suggest using:
scoped_refptr<PrefRegistrySimple> registry(...);

syntax instead (Just because that's what has to be done with scoped_ptrs, and I
like to delude myself into thinking they're more similar than they really are.

[xunjieli, 2015/07/09 19:37:01]

Done.

+    registry->RegisterDictionaryPref(kHttpServerProperties,
+                                     new base::DictionaryValue());
+    pref_service_ = factory.Create(registry.get()).Pass();
+
+    http_server_properties_manager_ = new net::HttpServerPropertiesManager(
+        pref_service_.get(), kHttpServerProperties,
+        base::ThreadTaskRunnerHandle::Get());

[mmenke, 2015/07/09 19:08:10]

Should either use GetNetworkTaskRunner() instead of
base::ThreadTaskRunnerHandle::Get(), or modify the CronetDataReductionProxy
constructor call to use base::ThreadTaskRunnerHandle::Get().  I don't have
strong feelings on the subject - GetNetworkTaskRunner() emphasizes it uses the
network thread, while ThreadTaskRunnerHandle::Get() emphasizes it uses the
current thread.

[mmenke, 2015/07/09 19:08:10]

I'd suggest this:

scoped_ptr<net::HttpServerPropertiesManager> http_properties_manager(new ...);
http_server_properties_manager->InitializeOnNetworkThread();
http_server_properties_manager_ = http_server_properties_manager.get();
context_builder.SetHttpServerProperties(http_server_properties_manager.get());

Keeping something in a scoped_ptr is a bit more robust if error cases are added
or something, and just ownership semantics a bit clearer .

[xunjieli, 2015/07/09 19:37:02]

Done.

[xunjieli, 2015/07/09 19:37:02]

Done.

+    http_server_properties_manager_->InitializeOnNetworkThread();
+    context_builder.SetHttpServerProperties(
+        make_scoped_ptr(http_server_properties_manager_));
   }
 
   context_.reset(context_builder.Build());