diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc index f20eee26097abb..ba2f687513a637 100644 --- a/content/browser/download/download_manager_impl.cc +++ b/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( diff --git a/content/browser/download/save_file_manager.cc b/content/browser/download/save_file_manager.cc index 4961d254ba2c93..a59ac47c50caae 100644 --- a/content/browser/download/save_file_manager.cc +++ b/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 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(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(), diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 56a2f8e88bd4f3..ab9fef18aa644b 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/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(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()); diff --git a/content/browser/site_instance_impl.cc b/content/browser/site_instance_impl.cc index 5a7ccbd331b4b6..280dd9e6144e43 100644 --- a/content/browser/site_instance_impl.cc +++ b/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) { diff --git a/content/browser/site_instance_impl.h b/content/browser/site_instance_impl.h index e5d26881312d42..0fecca6137cf97 100644 --- a/content/browser/site_instance_impl.h +++ b/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(). diff --git a/content/browser/storage_partition_impl.cc b/content/browser/storage_partition_impl.cc index f9112b70e2e0d0..0c25b235ef2130 100644 --- a/content/browser/storage_partition_impl.cc +++ b/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()) diff --git a/content/browser/storage_partition_impl.h b/content/browser/storage_partition_impl.h index e2a54854734761..0bd46421d0bf1f 100644 --- a/content/browser/storage_partition_impl.h +++ b/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( diff --git a/content/browser/worker_host/dedicated_worker_host.cc b/content/browser/worker_host/dedicated_worker_host.cc index d7c9d440a2e36c..899b314c8f3911 100644 --- a/content/browser/worker_host/dedicated_worker_host.cc +++ b/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 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( 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; diff --git a/content/browser/worker_host/shared_worker_service_impl.cc b/content/browser/worker_host/shared_worker_service_impl.cc index 54455eb7d25ab7..de421f33499cf1 100644 --- a/content/browser/worker_host/shared_worker_service_impl.cc +++ b/content/browser/worker_host/shared_worker_service_impl.cc @@ -182,15 +182,12 @@ 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, @@ -198,9 +195,8 @@ void SharedWorkerServiceImpl::ConnectToWorker( 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); }