Initialize ios/web_view translate with a system-wide URLRequestContext.

ios/web_view/internal/app/application_context_impl.cc

+// Copyright 2017 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 "ios/web_view/internal/app/application_context_impl.h"
+
+#include "base/command_line.h"
+#include "base/path_service.h"
+#include "base/tracked_objects.h"
+#include "components/flags_ui/pref_service_flags_storage.h"
+#include "components/net_log/chrome_net_log.h"
+#include "components/prefs/json_pref_store.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/pref_service_factory.h"
+#include "components/proxy_config/pref_proxy_config_tracker_impl.h"
+#include "components/ssl_config/ssl_config_service_manager.h"
+#include "components/translate/core/browser/translate_download_manager.h"
+#include "ios/web/public/web_thread.h"
+#include "ios/web_view/internal/app/web_view_io_thread.h"
+#include "net/socket/client_socket_pool_manager.h"
+
+base::FilePath ApplicationContextImpl::LocalStatePath() {

[Eugene But (OOO till 7-30), 2017/05/18 23:54:55]

Do you want to reorder the method to follow the order from header?

[michaeldo, 2017/05/23 22:38:36]

Done.

+  base::FilePath local_state_path;
+  PathService::Get(base::DIR_APP_DATA, &local_state_path);
+  local_state_path =
+      local_state_path.Append(FILE_PATH_LITERAL("ChromeWebView"));
+  local_state_path = local_state_path.Append(FILE_PATH_LITERAL("Local State"));
+  return local_state_path;
+}
+
+ApplicationContextImpl::ApplicationContextImpl(
+    const base::CommandLine& command_line,
+    const std::string& locale)
+    : local_state_task_runner_(JsonPrefStore::GetTaskRunnerForFile(
+          LocalStatePath(),
+          web::WebThread::GetBlockingPool())),
+      created_local_state_(false) {
+  DCHECK(!GetApplicationContext());
+  SetApplicationContext(this);
+
+  net_log_.reset(new net_log::ChromeNetLog(
+      base::FilePath(), net::NetLogCaptureMode::Default(),
+      command_line.GetCommandLineString(), std::string()));
+
+  SetApplicationLocale(locale);
+}
+
+ApplicationContextImpl::~ApplicationContextImpl() {
+  DCHECK_EQ(this, GetApplicationContext());
+  tracked_objects::ThreadData::EnsureCleanupWasCalled(4);

[Hiroshi Ichikawa, 2017/05/19 02:37:51]

Why is this 4?

[michaeldo, 2017/05/23 22:38:36]

The 4 is for the threads that get spun up, but the value is never used (has been
unsused for 5.5 years ever since it was added). Other platforms use 4 here, but
I think the parameter should be deleted or the method needs to be re-enabled as
stated in the comments "until it is working on XP".

The bug referenced in the below CL (crbug.com/103209)is marked fixed even though
the code in this method has never been executed. I'll ping an owner of //base
about this.

https://codereview.chromium.org/8606001

https://cs.chromium.org/chromium/src/chrome/browser/browser_process_impl.cc?t...

https://cs.chromium.org/chromium/src/base/tracked_objects.cc?l=939&gsn=Ensure...

[Hiroshi Ichikawa, 2017/05/24 07:25:46]

Thanks! Feel free to go ahead without waiting for the cleanup, but can you add a
comment saying this 4 is not used?

+  SetApplicationContext(nullptr);
+}
+
+void ApplicationContextImpl::PreCreateThreads() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  web_view_io_thread_.reset(new WebViewIOThread(GetLocalState(), GetNetLog()));

[Eugene But (OOO till 7-30), 2017/05/18 23:54:55]

s/new/MakeUnique ?

[michaeldo, 2017/05/23 22:38:36]

Done.

+}
+
+void ApplicationContextImpl::StartTearDown() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  // TODO(crbug.com/723854): Commit prefs when entering background.
+  if (local_state_) {
+    local_state_->CommitPendingWrite();
+  }
+}
+
+void ApplicationContextImpl::PostDestroyThreads() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  // Resets associated state right after actual thread is stopped as
+  // WebViewIOThread::Globals cleanup happens in CleanUp on the IO
+  // thread, i.e. as the thread exits its message loop.
+  //
+  // This is important because in various places, the WebViewIOThread
+  // object being NULL is considered synonymous with the IO thread

[Eugene But (OOO till 7-30), 2017/05/18 23:54:55]

nit: s/NULL/null

[michaeldo, 2017/05/23 22:38:36]

Done.

+  // having stopped.
+  web_view_io_thread_.reset();
+}
+
+PrefService* ApplicationContextImpl::GetLocalState() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  if (!created_local_state_)
+    CreateLocalState();

[Eugene But (OOO till 7-30), 2017/05/18 23:54:54]

Do you want to fold |CreateLocalState| into this method?

[michaeldo, 2017/05/23 22:38:36]

Yes, I had the same thought. Done.

+  return local_state_.get();
+}
+
+net::URLRequestContextGetter*
+ApplicationContextImpl::GetSystemURLRequestContext() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  return web_view_io_thread_->system_url_request_context_getter();
+}
+
+const std::string& ApplicationContextImpl::GetApplicationLocale() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(!application_locale_.empty());
+  return application_locale_;
+}
+
+net_log::ChromeNetLog* ApplicationContextImpl::GetNetLog() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  return net_log_.get();
+}
+
+WebViewIOThread* ApplicationContextImpl::GetWebViewIOThread() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(web_view_io_thread_.get());
+  return web_view_io_thread_.get();
+}
+
+void ApplicationContextImpl::SetApplicationLocale(const std::string& locale) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  application_locale_ = locale;
+  translate::TranslateDownloadManager::GetInstance()->set_application_locale(
+      application_locale_);
+}
+
+void ApplicationContextImpl::CreateLocalState() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(!created_local_state_ && !local_state_);
+  created_local_state_ = true;
+
+  // Register local state preferences.
+  scoped_refptr<PrefRegistrySimple> pref_registry(new PrefRegistrySimple);

[Eugene But (OOO till 7-30), 2017/05/18 23:54:55]

s/new/MakeUnique ?

[michaeldo, 2017/05/23 22:38:36]

I can't use MakeUnique because the destructor is protected.

+  flags_ui::PrefServiceFlagsStorage::RegisterPrefs(pref_registry.get());
+  PrefProxyConfigTrackerImpl::RegisterPrefs(pref_registry.get());
+  ssl_config::SSLConfigServiceManager::RegisterPrefs(pref_registry.get());
+
+  scoped_refptr<PersistentPrefStore> user_pref_store =
+      new JsonPrefStore(LocalStatePath(), local_state_task_runner_, nullptr);
+
+  PrefServiceFactory factory;
+  factory.set_user_prefs(user_pref_store);
+  local_state_ = factory.Create(pref_registry.get());
+
+  net::ClientSocketPoolManager::set_max_sockets_per_proxy_server(
+      net::HttpNetworkSession::NORMAL_SOCKET_POOL,
+      std::max(std::min<int>(net::kDefaultMaxSocketsPerProxyServer, 99),

[Hiroshi Ichikawa, 2017/05/19 02:37:51]

Any reason to cap this value with 99? Maybe define a constant for 99?

[michaeldo, 2017/05/23 22:38:36]

No reason except consistency, which I agree is a bad reason. I think don't think
a constant helps readability here because I don't have any good name for the
value and my naming could end up being misleading to the real meaning of the
value.

[Hiroshi Ichikawa, 2017/05/24 07:25:46]

Can you maybe just leave a comment saying this is inherited from somewhere else?

+               net::ClientSocketPoolManager::max_sockets_per_group(
+                   net::HttpNetworkSession::NORMAL_SOCKET_POOL)));
+}