Fix cookie access for extensions and apps.

The first fix is to get the CookieManager from the render process' StoragePartition, instead of
using GetDefaultStoragePartition. The second fix is to not use the StoragePartition's CookieManager
(which is used for web requests) when ContentBrowserClient::OverrideRequestContextForURL returns a
different store (e.g. for chrome-extension scheme).

Bug: 769401

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Id61abda2cd38957867fb3848d06b1b18d9e89b79
Reviewed-on: https://chromium-review.googlesource.com/957554Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543186}

content/browser/frame_host/render_frame_message_filter.cc

@@ -23,6 +23,7 @@
 #include "content/browser/gpu/gpu_data_manager_impl.h"
 #include "content/browser/renderer_host/render_widget_helper.h"
 #include "content/browser/resource_context_impl.h"
+#include "content/browser/storage_partition_impl.h"
 #include "content/common/content_constants_internal.h"
 #include "content/common/frame_messages.h"
 #include "content/common/frame_owner_properties.h"
@@ -222,23 +223,22 @@ RenderFrameMessageFilter::RenderFrameMessageFilter(
     int render_process_id,
     PluginServiceImpl* plugin_service,
     BrowserContext* browser_context,
-    net::URLRequestContextGetter* request_context,
+    StoragePartition* storage_partition,
     RenderWidgetHelper* render_widget_helper)
     : BrowserMessageFilter(FrameMsgStart),
       BrowserAssociatedInterface<mojom::RenderFrameMessageFilter>(this, this),
 #if BUILDFLAG(ENABLE_PLUGINS)
       plugin_service_(plugin_service),
-      profile_data_directory_(browser_context->GetPath()),
+      profile_data_directory_(storage_partition->GetPath()),
 #endif  // ENABLE_PLUGINS
-      request_context_(request_context),
+      request_context_(storage_partition->GetURLRequestContext()),
       resource_context_(browser_context->GetResourceContext()),
       render_widget_helper_(render_widget_helper),
       incognito_(browser_context->IsOffTheRecord()),
       render_process_id_(render_process_id) {
   network::mojom::CookieManagerPtr cookie_manager;
-  BrowserContext::GetDefaultStoragePartition(browser_context)
-      ->GetNetworkContext()
-      ->GetCookieManager(mojo::MakeRequest(&cookie_manager));
+  storage_partition->GetNetworkContext()->GetCookieManager(
+      mojo::MakeRequest(&cookie_manager));
 
   // The PostTask below could finish before the constructor returns which would
   // lead to this object being destructed prematurely.
@@ -477,16 +477,18 @@ void RenderFrameMessageFilter::SetCookie(int32_t render_frame_id,
           render_frame_id, options))
     return;
 
-  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
-    // TODO(jam): modify GetRequestContextForURL to work with network service.
-    // Merge this with code path below for non-network service.
+  net::URLRequestContext* context = GetRequestContextForURL(url);
+  // If the embedder overrides the URLRequestContext then always use it, even if
+  // the network service is enabled, instead of the CookieManager associated
+  // this process' StoragePartition.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
+      context == request_context_->GetURLRequestContext()) {
     cookie_manager_->SetCanonicalCookie(*cookie, url.SchemeIsCryptographic(),
                                         !options.exclude_httponly(),
                                         net::CookieStore::SetCookiesCallback());
     return;
   }
 
-  net::URLRequestContext* context = GetRequestContextForURL(url);
   // Pass a null callback since we don't care about when the 'set' completes.
   context->cookie_store()->SetCanonicalCookieAsync(
       std::move(cookie), url.SchemeIsCryptographic(),
@@ -519,7 +521,12 @@ void RenderFrameMessageFilter::GetCookies(int render_frame_id,
         net::CookieOptions::SameSiteCookieMode::DO_NOT_INCLUDE);
   }
 
-  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+  net::URLRequestContext* context = GetRequestContextForURL(url);
+  // If the embedder overrides the URLRequestContext then always use it, even if
+  // the network service is enabled, instead of the CookieManager associated
+  // this process' StoragePartition.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&
+      context == request_context_->GetURLRequestContext()) {
     // TODO(jam): modify GetRequestContextForURL to work with network service.
     // Merge this with code path below for non-network service.
     cookie_manager_->GetCookieList(
@@ -534,7 +541,6 @@ void RenderFrameMessageFilter::GetCookies(int render_frame_id,
   // http://crbug.com/99242
   DEBUG_ALIAS_FOR_GURL(url_buf, url);
 
-  net::URLRequestContext* context = GetRequestContextForURL(url);
   context->cookie_store()->GetCookieListWithOptionsAsync(
       url, options,
       base::BindOnce(&RenderFrameMessageFilter::CheckPolicyForCookies, this,

content/browser/frame_host/render_frame_message_filter.h

@@ -48,6 +48,7 @@ class PluginServiceImpl;
 struct Referrer;
 class RenderWidgetHelper;
 class ResourceContext;
+class StoragePartition;
 struct WebPluginInfo;
 
 // RenderFrameMessageFilter intercepts FrameHost messages on the IO thread
@@ -64,7 +65,7 @@ class CONTENT_EXPORT RenderFrameMessageFilter
   RenderFrameMessageFilter(int render_process_id,
                            PluginServiceImpl* plugin_service,
                            BrowserContext* browser_context,
-                           net::URLRequestContextGetter* request_context,
+                           StoragePartition* storage_partition,
                            RenderWidgetHelper* render_widget_helper);
 
   // BrowserMessageFilter methods:

content/browser/renderer_host/render_process_host_impl.cc

@@ -1706,9 +1706,7 @@ void RenderProcessHostImpl::CreateMessageFilters() {
 #else
       nullptr,
 #endif
-      GetBrowserContext(),
-      request_context.get(),
-      widget_helper_.get());
+      GetBrowserContext(), storage_partition_impl_, widget_helper_.get());
   AddFilter(render_frame_message_filter_.get());
 
   BrowserContext* browser_context = GetBrowserContext();

content/browser/renderer_host/render_view_host_unittest.cc

@@ -256,8 +256,7 @@ class TestSaveImageFromDataURL : public RenderFrameMessageFilter {
             0,
             nullptr,
             context,
-            BrowserContext::GetDefaultStoragePartition(context)
-                ->GetURLRequestContext(),
+            BrowserContext::GetDefaultStoragePartition(context),
             nullptr) {
     Reset();
   }

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

@@ -14,13 +14,10 @@
 -EnabledSignInIsolationBrowserTest.SyntheticTrial
 -ExpectCTBrowserTest.TestDynamicExpectCTHeaderProcessing
 -ExpectCTBrowserTest.TestDynamicExpectCTReporting
--ExtensionApiTest.Cookies
 -ExtensionApiTest.Debugger
 -ExtensionApiTestWithSwitch.ExtensionDebugger
 -ExtensionUnloadBrowserTest.UnloadWithContentScripts
 -InstantThemeTest.ThemeBackgroundAccess
--IsolatedAppTest.CookieIsolation
--IsolatedAppTest.SubresourceCookieIsolation
 -LocalNTPJavascriptTest.SimpleJavascriptTests
 -LocalNTPVoiceJavascriptTest.MicrophoneTests
 -LocalNTPVoiceJavascriptTest.SpeechTests
@@ -42,7 +39,6 @@
 -PolicyTest.DefaultCookiesSetting
 -PrefetchBrowserTestPredictionDisabled.ExperimentDisabled
 -PreviewsOptimizationGuideBrowserTest.NoScriptPreviewsEnabledByWhitelist
--ProcessManagerBrowserTest.ExtensionProcessReuse
 -ProcessManagerBrowserTest.NestedURLNavigationsToExtensionAllowed
 -ProfileWindowBrowserTest.GuestClearsCookies
 -ProxySettingsApiTest.ProxyEventsInvalidProxy
@@ -64,8 +60,6 @@
 -WebViewTests/WebViewTest.ClearPersistentCookies/1
 -WebViewTests/WebViewTest.ClearSessionCookies/0
 -WebViewTests/WebViewTest.ClearSessionCookies/1
--WebViewTests/WebViewTest.CookieIsolation/0
--WebViewTests/WebViewTest.CookieIsolation/1
 -WebViewTests/WebViewTest.DownloadCookieIsolation/0
 -WebViewTests/WebViewTest.DownloadCookieIsolation/1
 -WebViewTests/WebViewTest.DownloadCookieIsolation_CrossSession/0