Add StoragePartition::GetPartitionDomain().

Adding GetPartitionDomain() method to make it easier to determine the
partition domain and reduce the need for calls to
GetStoragePartitionConfigForSite().

There are a few cases where it is unclear whether the domain computed
from the site URL actually matches what was returned by the new method,
so these cases now call a new GetPartitionDomain() helper function on
SiteInstance. This function preserves legacy behavior and will generate
crash dumps if we encounter any situations where the config and storage
partition disagree.


Bug: 1085275
Change-Id: I4fd02a91cf702f3849010d3723bf7a223b723785
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252470
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791345}

content/browser/download/download_manager_impl.cc

@@ -1316,16 +1316,12 @@ void DownloadManagerImpl::BeginResourceDownloadOnChecksComplete(
             BrowserContext::GetStoragePartitionForSite(browser_context_,
                                                        site_url));
 
-    auto storage_partition_config =
-        GetContentClient()->browser()->GetStoragePartitionConfigForSite(
-            browser_context_, site_url);
-
     pending_url_loader_factory =
         CreatePendingSharedURLLoaderFactoryFromURLLoaderFactory(
             CreateFileSystemURLLoaderFactory(
                 rfh->GetProcess()->GetID(), rfh->GetFrameTreeNodeId(),
                 storage_partition->GetFileSystemContext(),
-                storage_partition_config.partition_domain()));
+                storage_partition->GetPartitionDomain()));
   } else if (params->url().SchemeIs(url::kDataScheme)) {
     pending_url_loader_factory =
         CreatePendingSharedURLLoaderFactoryFromURLLoaderFactory(

content/browser/download/save_file_manager.cc

@@ -19,6 +19,7 @@
 #include "content/browser/file_system/file_system_url_loader_factory.h"
 #include "content/browser/loader/file_url_loader_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/storage_partition_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -252,8 +253,8 @@ void SaveFileManager::SaveURL(SaveItemId save_item_id,
 
     network::mojom::URLLoaderFactory* factory = nullptr;
     std::unique_ptr<network::mojom::URLLoaderFactory> url_loader_factory;
-    RenderFrameHost* rfh = RenderFrameHost::FromID(render_process_host_id,
-                                                   render_frame_routing_id);
+    auto* rfh = RenderFrameHostImpl::FromID(render_process_host_id,
+                                            render_frame_routing_id);
 
     // TODO(qinmin): should this match the if statements in
     // DownloadManagerImpl::BeginResourceDownloadOnChecksComplete so that it
@@ -268,16 +269,13 @@ void SaveFileManager::SaveURL(SaveItemId save_item_id,
           base::TaskPriority::USER_VISIBLE);
       factory = url_loader_factory.get();
     } else if (url.SchemeIsFileSystem() && rfh) {
-      std::string storage_domain;
-      auto* site_instance = rfh->GetSiteInstance();
-      auto storage_partition_config =
-          GetContentClient()->browser()->GetStoragePartitionConfigForSite(
-              context, site_instance->GetSiteURL());
-
+      auto* storage_partition_impl =
+          static_cast<StoragePartitionImpl*>(storage_partition);
+      auto partition_domain =
+          rfh->GetSiteInstance()->GetPartitionDomain(storage_partition_impl);
       url_loader_factory = CreateFileSystemURLLoaderFactory(
           rfh->GetProcess()->GetID(), rfh->GetFrameTreeNodeId(),
-          storage_partition->GetFileSystemContext(),
-          storage_partition_config.partition_domain());
+          storage_partition->GetFileSystemContext(), partition_domain);
       factory = url_loader_factory.get();
     } else if (rfh && url.SchemeIs(content::kChromeUIScheme)) {
       url_loader_factory = CreateWebUIURLLoader(rfh, url.scheme(),

content/browser/frame_host/render_frame_host_impl.cc

@@ -5968,17 +5968,14 @@ void RenderFrameHostImpl::CommitNavigation(
     }
 #endif
 
-    StoragePartition* partition =
-        BrowserContext::GetStoragePartition(browser_context, GetSiteInstance());
-    auto storage_partition_config =
-        GetContentClient()->browser()->GetStoragePartitionConfigForSite(
-            browser_context, site_instance_->GetSiteInfo().site_url());
+    auto* partition =
+        static_cast<StoragePartitionImpl*>(BrowserContext::GetStoragePartition(
+            browser_context, GetSiteInstance()));
     non_network_url_loader_factories_.emplace(
-        url::kFileSystemScheme,
-        content::CreateFileSystemURLLoaderFactory(
-            process_->GetID(), GetFrameTreeNodeId(),
-            partition->GetFileSystemContext(),
-            storage_partition_config.partition_domain()));
+        url::kFileSystemScheme, content::CreateFileSystemURLLoaderFactory(
+                                    process_->GetID(), GetFrameTreeNodeId(),
+                                    partition->GetFileSystemContext(),
+                                    partition->GetPartitionDomain()));
 
     non_network_url_loader_factories_.emplace(
         url::kDataScheme, std::make_unique<DataURLLoaderFactory>());

content/browser/site_instance_impl.cc

@@ -9,6 +9,7 @@
 
 #include "base/command_line.h"
 #include "base/debug/crash_logging.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "content/browser/bad_message.h"
@@ -687,6 +688,42 @@ bool SiteInstanceImpl::IsGuest() {
   return is_guest_;
 }
 
+std::string SiteInstanceImpl::GetPartitionDomain(
+    StoragePartitionImpl* storage_partition) {
+  auto storage_partition_config =
+      GetContentClient()->browser()->GetStoragePartitionConfigForSite(
+          GetBrowserContext(), GetSiteURL());
+
+  // The DCHECK here is to allow the trybots to detect any attempt to introduce
+  // new code that violates this assumption.
+  DCHECK_EQ(storage_partition->GetPartitionDomain(),
+            storage_partition_config.partition_domain());
+
+  if (storage_partition->GetPartitionDomain() !=
+      storage_partition_config.partition_domain()) {
+    // Trigger crash logging if we encounter a case that violates our
+    // assumptions.
+    static auto* storage_partition_domain_key =
+        base::debug::AllocateCrashKeyString("storage_partition_domain",
+                                            base::debug::CrashKeySize::Size256);
+    static auto* storage_partition_config_domain_key =
+        base::debug::AllocateCrashKeyString(
+            "storage_partition_config_domain_key",
+            base::debug::CrashKeySize::Size256);
+    base::debug::SetCrashKeyString(storage_partition_domain_key,
+                                   storage_partition->GetPartitionDomain());
+    base::debug::SetCrashKeyString(storage_partition_config_domain_key,
+                                   storage_partition_config.partition_domain());
+
+    base::debug::DumpWithoutCrashing();
+
+    // Return the value from the config to preserve legacy behavior until we
+    // can land a fix.
+    return storage_partition_config.partition_domain();
+  }
+  return storage_partition->GetPartitionDomain();
+}
+
 bool SiteInstanceImpl::IsOriginalUrlSameSite(
     const GURL& dest_url,
     bool should_compare_effective_urls) {

content/browser/site_instance_impl.h

@@ -22,6 +22,7 @@ namespace content {
 class BrowsingInstance;
 class ProcessLock;
 class RenderProcessHostFactory;
+class StoragePartitionImpl;
 
 // SiteInfo represents the principal of a SiteInstance. All documents and
 // workers within a SiteInstance are considered part of this principal and will
@@ -268,6 +269,19 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
   // the SiteInfo's process_lock_url() for security decisions.
   const ProcessLock GetProcessLock() const;
 
+  // Helper function that returns the storage partition domain for this
+  // object.
+  // This is a temporary helper function used to verify that
+  // the partition domain computed using this SiteInstance's site URL matches
+  // the partition domain returned by storage_partition->GetPartitionDomain().
+  // If there is a mismatch, we call DumpWithoutCrashing() and return the value
+  // computed from the site URL since that is the legacy behavior.
+  //
+  // TODO(acolwell) : Remove this function and update callers to directly call
+  // storage_partition->GetPartitionDomain() once we've verified that this is
+  // safe.
+  std::string GetPartitionDomain(StoragePartitionImpl* storage_partition);
+
   // This function returns a SiteInfo with the appropriate site_url and
   // process_lock_url computed.
   // Note: eventually this function will replace GetSiteForURL().

content/browser/storage_partition_impl.cc

@@ -1391,6 +1391,10 @@ base::FilePath StoragePartitionImpl::GetPath() {
   return partition_path_;
 }
 
+std::string StoragePartitionImpl::GetPartitionDomain() {
+  return partition_domain_;
+}
+
 network::mojom::NetworkContext* StoragePartitionImpl::GetNetworkContext() {
   DCHECK(initialized_);
   if (!network_context_.is_bound())

content/browser/storage_partition_impl.h

@@ -198,6 +198,7 @@ class CONTENT_EXPORT StoragePartitionImpl
   QuotaContext* GetQuotaContext();
   NativeIOContext* GetNativeIOContext();
   ConversionManagerImpl* GetConversionManager();
+  std::string GetPartitionDomain();
 
   // blink::mojom::DomStorage interface.
   void OpenLocalStorage(

content/browser/worker_host/dedicated_worker_host.cc

@@ -135,14 +135,6 @@ void DedicatedWorkerHost::StartScriptLoad(
     return;
   }
 
-  // Get a storage domain.
-  SiteInstance* site_instance =
-      nearest_ancestor_render_frame_host->GetSiteInstance();
-  auto storage_partition_config =
-      GetContentClient()->browser()->GetStoragePartitionConfigForSite(
-          storage_partition_impl->browser_context(),
-          site_instance->GetSiteURL());
-
   scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory;
   if (script_url.SchemeIsBlob()) {
     if (!blob_url_token) {
@@ -189,6 +181,11 @@ void DedicatedWorkerHost::StartScriptLoad(
   service_worker_handle_ = std::make_unique<ServiceWorkerMainResourceHandle>(
       storage_partition_impl->GetServiceWorkerContext(), base::DoNothing());
 
+  // Get a storage domain.
+  auto partition_domain =
+      nearest_ancestor_render_frame_host->GetSiteInstance()->GetPartitionDomain(
+          storage_partition_impl);
+
   WorkerScriptFetchInitiator::Start(
       worker_process_host_->GetID(), token_, SharedWorkerId(), script_url,
       creator_render_frame_host,
@@ -201,7 +198,7 @@ void DedicatedWorkerHost::StartScriptLoad(
       service_worker_handle_.get(),
       appcache_host ? appcache_host->GetWeakPtr() : nullptr,
       std::move(blob_url_loader_factory), nullptr, storage_partition_impl,
-      storage_partition_config.partition_domain(),
+      partition_domain,
       base::BindOnce(&DedicatedWorkerHost::DidStartScriptLoad,
                      weak_factory_.GetWeakPtr()));
 }
@@ -500,13 +497,10 @@ void DedicatedWorkerHost::UpdateSubresourceLoaderFactories() {
   if (!ancestor_render_frame_host)
     return;
 
-  SiteInstance* site_instance = ancestor_render_frame_host->GetSiteInstance();
-
   // Get a storage domain.
-  auto storage_partition_config =
-      GetContentClient()->browser()->GetStoragePartitionConfigForSite(
-          storage_partition_impl->browser_context(),
-          site_instance->GetSiteURL());
+  auto partition_domain =
+      ancestor_render_frame_host->GetSiteInstance()->GetPartitionDomain(
+          storage_partition_impl);
 
   // Start observing Network Service crash again.
   ObserveNetworkServiceCrash(storage_partition_impl);
@@ -518,7 +512,7 @@ void DedicatedWorkerHost::UpdateSubresourceLoaderFactories() {
           WorkerScriptFetchInitiator::CreateFactoryBundle(
               WorkerScriptFetchInitiator::LoaderType::kSubResource,
               worker_process_host_->GetID(), storage_partition_impl,
-              storage_partition_config.partition_domain(), file_url_support_,
+              partition_domain, file_url_support_,
               /*filesystem_url_support=*/true);
 
   bool bypass_redirect_checks = false;

content/browser/worker_host/shared_worker_service_impl.cc

@@ -182,25 +182,21 @@ void SharedWorkerServiceImpl::ConnectToWorker(
   // Could not find an existing SharedWorkerHost to reuse. Create a new one.
 
   // Get a storage domain.
-  SiteInstance* site_instance = render_frame_host->GetSiteInstance();
+  auto* site_instance = render_frame_host->GetSiteInstance();
   if (!site_instance) {
     ScriptLoadFailed(std::move(client), /*error_message=*/"");
     return;
   }
-  auto storage_partition_config =
-      GetContentClient()->browser()->GetStoragePartitionConfigForSite(
-          storage_partition_->browser_context(), site_instance->GetSiteURL());
-
+  auto partition_domain = site_instance->GetPartitionDomain(storage_partition_);
   SharedWorkerInstance instance(
       info->url, info->options->type, info->options->credentials,
       info->options->name, constructor_origin, info->content_security_policy,
       info->content_security_policy_type, info->creation_address_space,
       creation_context_type);
   host = CreateWorker(shared_worker_id_generator_.GenerateNextId(), instance,
                       std::move(info->outside_fetch_client_settings_object),
-                      client_render_frame_host_id,
-                      storage_partition_config.partition_domain(), message_port,
-                      std::move(blob_url_loader_factory));
+                      client_render_frame_host_id, partition_domain,
+                      message_port, std::move(blob_url_loader_factory));
   host->AddClient(std::move(client), client_render_frame_host_id, message_port,
                   client_ukm_source_id);
 }