Fix webview tag's using an on-disk directory with the network service enabled.

Bug: 769401
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ia9ad46583b00f88631eccbc65c23cf6864134f26
Reviewed-on: https://chromium-review.googlesource.com/965921Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543837}

chrome/browser/chrome_content_browser_client.cc

@@ -57,6 +57,8 @@
 #include "chrome/browser/memory/chrome_memory_coordinator_delegate.h"
 #include "chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.h"
 #include "chrome/browser/nacl_host/nacl_browser_delegate_impl.h"
+#include "chrome/browser/net/profile_network_context_service.h"
+#include "chrome/browser/net/profile_network_context_service_factory.h"
 #include "chrome/browser/net_benchmarking.h"
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "chrome/browser/page_load_metrics/metrics_navigation_throttle.h"
@@ -3926,6 +3928,7 @@ ChromeContentBrowserClient::CreateNetworkContext(
     content::BrowserContext* context,
     bool in_memory,
     const base::FilePath& relative_partition_path) {
+  Profile* profile = Profile::FromBrowserContext(context);
   // If the relative partition path is empty, this is creating the Profile's
   // main NetworkContext.
   if (relative_partition_path.empty()) {
@@ -3933,12 +3936,17 @@ ChromeContentBrowserClient::CreateNetworkContext(
     // ProfileIOData is removed. Currently, TestProfile (used in unit tests)
     // needs to be able to bypass ProfileNetworkContextServiceFactory, since
     // TestProfile bypasses ProfileIOData's URLRequestContext creation logic.
-    Profile* profile = Profile::FromBrowserContext(context);
     return profile->CreateMainNetworkContext();
   }
-  // TODO(mmenke):  Implement this once ProfileNetworkContextServiceFactory can
-  // create a fully functional NetworkContext for Apps when the network service
-  // is disabled.
+
+  // TODO(mmenke):  Share this with the non-network service code path once
+  // ProfileNetworkContextServiceFactory can create a fully functional
+  // NetworkContext for Apps when the network service is disabled.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    return ProfileNetworkContextServiceFactory::GetForContext(context)
+        ->CreateNetworkContextForPartition(in_memory, relative_partition_path);
+  }
+
   return ContentBrowserClient::CreateNetworkContext(context, in_memory,
                                                     relative_partition_path);
 }

chrome/browser/net/profile_network_context_service.cc

@@ -67,6 +67,18 @@ ProfileNetworkContextService::CreateMainNetworkContext() {
   return network_context;
 }
 
+network::mojom::NetworkContextPtr
+ProfileNetworkContextService::CreateNetworkContextForPartition(
+    bool in_memory,
+    const base::FilePath& relative_partition_path) {
+  DCHECK(base::FeatureList::IsEnabled(network::features::kNetworkService));
+  network::mojom::NetworkContextPtr network_context;
+  content::GetNetworkService()->CreateNetworkContext(
+      MakeRequest(&network_context),
+      CreateNetworkContextParams(in_memory, relative_partition_path));
+  return network_context;
+}
+
 void ProfileNetworkContextService::SetUpProfileIODataMainContext(
     network::mojom::NetworkContextRequest* network_context_request,
     network::mojom::NetworkContextParamsPtr* network_context_params) {
@@ -125,6 +137,14 @@ void ProfileNetworkContextService::FlushProxyConfigMonitorForTesting() {
 
 network::mojom::NetworkContextParamsPtr
 ProfileNetworkContextService::CreateMainNetworkContextParams() {
+  return CreateNetworkContextParams(profile_->IsOffTheRecord(),
+                                    base::FilePath());
+}
+
+network::mojom::NetworkContextParamsPtr
+ProfileNetworkContextService::CreateNetworkContextParams(
+    bool in_memory,
+    const base::FilePath& relative_partition_path) {
   // TODO(mmenke): Set up parameters here.
   network::mojom::NetworkContextParamsPtr network_context_params =
       CreateDefaultNetworkContextParams();
@@ -136,13 +156,17 @@ ProfileNetworkContextService::CreateMainNetworkContextParams() {
   // Always enable the HTTP cache.
   network_context_params->http_cache_enabled = true;
 
+  base::FilePath path = profile_->GetPath();
+  if (!relative_partition_path.empty())
+    path = path.Append(relative_partition_path);
+
   // Configure on-disk storage for non-OTR profiles. OTR profiles just use
   // default behavior (in memory storage, default sizes).
   PrefService* prefs = profile_->GetPrefs();
-  if (!profile_->IsOffTheRecord()) {
+  if (!in_memory) {
     // Configure the HTTP cache path and size.
     base::FilePath base_cache_path;
-    chrome::GetUserCacheDirectory(profile_->GetPath(), &base_cache_path);
+    chrome::GetUserCacheDirectory(path, &base_cache_path);
     base::FilePath disk_cache_dir = prefs->GetFilePath(prefs::kDiskCacheDir);
     if (!disk_cache_dir.empty())
       base_cache_path = disk_cache_dir.Append(base_cache_path.BaseName());
@@ -154,20 +178,26 @@ ProfileNetworkContextService::CreateMainNetworkContextParams() {
     // Currently this just contains HttpServerProperties, but that will likely
     // change.
     network_context_params->http_server_properties_path =
-        profile_->GetPath().Append(chrome::kNetworkPersistentStateFilename);
+        path.Append(chrome::kNetworkPersistentStateFilename);
 
-    base::FilePath cookie_path = profile_->GetPath();
+    base::FilePath cookie_path = path;
     cookie_path = cookie_path.Append(chrome::kCookieFilename);
     network_context_params->cookie_path = cookie_path;
 
-    base::FilePath channel_id_path = profile_->GetPath();
+    base::FilePath channel_id_path = path;
     channel_id_path = channel_id_path.Append(chrome::kChannelIDFilename);
     network_context_params->channel_id_path = channel_id_path;
 
-    network_context_params->restore_old_session_cookies =
-        profile_->ShouldRestoreOldSessionCookies();
-    network_context_params->persist_session_cookies =
-        profile_->ShouldPersistSessionCookies();
+    if (relative_partition_path.empty()) {
+      network_context_params->restore_old_session_cookies =
+          profile_->ShouldRestoreOldSessionCookies();
+      network_context_params->persist_session_cookies =
+          profile_->ShouldPersistSessionCookies();
+    } else {
+      // Copy behavior of ProfileImplIOData::InitializeAppRequestContext.
+      network_context_params->restore_old_session_cookies = false;
+      network_context_params->persist_session_cookies = false;
+    }
   }
 
   // NOTE(mmenke): Keep these protocol handlers and

chrome/browser/net/profile_network_context_service.h

@@ -30,6 +30,12 @@ class ProfileNetworkContextService : public KeyedService {
   // SetUpProfileIODataMainContext.
   network::mojom::NetworkContextPtr CreateMainNetworkContext();
 
+  // Create a network context for the given |relative_parition_path|. This is
+  // only used when the network service is enabled for now.
+  network::mojom::NetworkContextPtr CreateNetworkContextForPartition(
+      bool in_memory,
+      const base::FilePath& relative_partition_path);
+
   // Initializes |*network_context_params| to set up the ProfileIOData's
   // main URLRequestContext and |*network_context_request| to be one end of a
   // Mojo pipe to be bound to the NetworkContext for that URLRequestContext.
@@ -73,6 +79,13 @@ class ProfileNetworkContextService : public KeyedService {
   // it initializes some class members.
   network::mojom::NetworkContextParamsPtr CreateMainNetworkContextParams();
 
+  // Creates parameters for the NetworkContext. Use |in_memory| instead of
+  // |profile_->IsOffTheRecord()| because sometimes normal profiles want off the
+  // record partitions (e.g. for webview tag).
+  network::mojom::NetworkContextParamsPtr CreateNetworkContextParams(
+      bool in_memory,
+      const base::FilePath& relative_partition_path);
+
   Profile* const profile_;
 
   ProxyConfigMonitor proxy_config_monitor_;

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -60,8 +60,7 @@
 -WebViewTests/WebViewTest.ClearPersistentCookies/1
 -WebViewTests/WebViewTest.ClearSessionCookies/0
 -WebViewTests/WebViewTest.ClearSessionCookies/1
--WebViewTests/WebViewTest.DownloadCookieIsolation_CrossSession/0
--WebViewTests/WebViewTest.DownloadCookieIsolation_CrossSession/1
+# These two StoragePersistence tests flakily pass.
 -WebViewTests/WebViewTest.StoragePersistence/0
 -WebViewTests/WebViewTest.StoragePersistence/1
 -WebViewTests/WebViewTest.WebViewInBackgroundPage/0