Skip to content

Commit

Permalink
Rename GetStoragePartitionForSite() and clean up some callers.
Browse files Browse the repository at this point in the history
This change renames GetStoragePartitionForSite() to
GetStoragePartitionForUrl() because most of the callers provide
normal URLs or origins and not site URLs. There are also some
minor changes that clean up the few cases that were actually
using site URLs. Site URLs have slightly different semantics than
normal URLs and this change ensures that callers are properly
indicating the type of URL they are passing in.

- Renamed BrowserContext::GetStoragePartitionForSite() to
  GetStoragePartitionForUrl() and updated all callers.
- Replaced a few calls that were passing SiteInfo::site_url()
  values to use SiteInfo::GetStoragePartitionConfig() instead.
- Introduced SiteInfo::GetStoragePartitionConfigForUrl() so that
  SiteInfo::GetStoragePartitionConfig(),
  BrowserContext::GetStoragePartitionForUrl(), and the
  DownloadManageImpl, code could all use the same logic.
- Added some crash dump logic to catch any cases where guest
  site URLs might be improperly marked as non-site URLs.

Bug: 1030932
Change-Id: Id0d9a074186beced1879664b85c5ce1ec5e378b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2762442
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Owners-Override: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#864458}
  • Loading branch information
acolwell authored and Chromium LUCI CQ committed Mar 18, 2021
1 parent 1d4b011 commit b54ca39
Show file tree
Hide file tree
Showing 41 changed files with 130 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ std::vector<blink::NotificationResources> ParseResources(
}

PlatformNotificationContext* GetContext(Profile* profile, const GURL& origin) {
auto* partition = BrowserContext::GetStoragePartitionForSite(profile, origin);
auto* partition = BrowserContext::GetStoragePartitionForUrl(profile, origin);
auto* context = partition->GetPlatformNotificationContext();
DCHECK(context);
return context;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ void BackgroundSyncDelegateImpl::OnEngagementEvent(

suspended_periodic_sync_origins_.erase(iter);

auto* storage_partition = content::BrowserContext::GetStoragePartitionForSite(
auto* storage_partition = content::BrowserContext::GetStoragePartitionForUrl(
profile_, url, /* can_create= */ false);
if (!storage_partition)
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ network::mojom::CookieManager*
AndroidSmsAppSetupControllerImpl::PwaDelegate::GetCookieManager(
const GURL& app_url,
Profile* profile) {
return content::BrowserContext::GetStoragePartitionForSite(profile, app_url)
return content::BrowserContext::GetStoragePartitionForUrl(profile, app_url)
->GetCookieManagerForBrowserProcess();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ GURL AndroidSmsPairingStateTrackerImpl::GetPairingUrl() {

network::mojom::CookieManager*
AndroidSmsPairingStateTrackerImpl::GetCookieManager() {
return content::BrowserContext::GetStoragePartitionForSite(profile_,
GetPairingUrl())
return content::BrowserContext::GetStoragePartitionForUrl(profile_,
GetPairingUrl())
->GetCookieManagerForBrowserProcess();
}

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/android_sms/connection_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ConnectionManager::ServiceWorkerProvider::~ServiceWorkerProvider() = default;
content::ServiceWorkerContext* ConnectionManager::ServiceWorkerProvider::Get(
const GURL& url,
Profile* profile) {
return content::BrowserContext::GetStoragePartitionForSite(profile, url)
return content::BrowserContext::GetStoragePartitionForUrl(profile, url)
->GetServiceWorkerContext();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ scoped_refptr<storage::FileSystemContext> GetFileSystemContext(
const GURL& url) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
content::StoragePartition* storage =
content::BrowserContext::GetStoragePartitionForSite(context, url);
content::BrowserContext::GetStoragePartitionForUrl(context, url);
return storage->GetFileSystemContext();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ TEST_F(FileManagerFileAPIUtilTest,

// Obtain the file system context.
content::StoragePartition* const partition =
content::BrowserContext::GetStoragePartitionForSite(
content::BrowserContext::GetStoragePartitionForUrl(
profile, GURL("http://example.com"));
ASSERT_TRUE(partition);
storage::FileSystemContext* const context = partition->GetFileSystemContext();
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/fileapi/external_file_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class URLHelper {
}
Profile* const profile = reinterpret_cast<Profile*>(profile_id);
content::StoragePartition* const storage =
content::BrowserContext::GetStoragePartitionForSite(profile, url);
content::BrowserContext::GetStoragePartitionForUrl(profile, url);
DCHECK(storage);

scoped_refptr<storage::FileSystemContext> context =
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/content_index/content_index_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ void ContentIndexProviderImpl::OpenItem(
const ContentId& id) {
auto components = GetEntryKeyComponents(id.id);

auto* storage_partition = content::BrowserContext::GetStoragePartitionForSite(
auto* storage_partition = content::BrowserContext::GetStoragePartitionForUrl(
profile_, components.origin.GetURL(), /* can_create= */ false);

if (!storage_partition || !storage_partition->GetContentIndexContext())
Expand Down Expand Up @@ -212,7 +212,7 @@ void ContentIndexProviderImpl::DidOpenTab(content::ContentIndexEntry entry,
void ContentIndexProviderImpl::RemoveItem(const ContentId& id) {
auto components = GetEntryKeyComponents(id.id);

auto* storage_partition = content::BrowserContext::GetStoragePartitionForSite(
auto* storage_partition = content::BrowserContext::GetStoragePartitionForUrl(
profile_, components.origin.GetURL(), /* can_create= */ false);

if (!storage_partition || !storage_partition->GetContentIndexContext())
Expand Down Expand Up @@ -242,7 +242,7 @@ void ContentIndexProviderImpl::GetItemById(const ContentId& id,
SingleItemCallback callback) {
auto components = GetEntryKeyComponents(id.id);

auto* storage_partition = content::BrowserContext::GetStoragePartitionForSite(
auto* storage_partition = content::BrowserContext::GetStoragePartitionForUrl(
profile_, components.origin.GetURL(), /* can_create= */ false);

if (!storage_partition || !storage_partition->GetContentIndexContext()) {
Expand Down Expand Up @@ -332,7 +332,7 @@ void ContentIndexProviderImpl::GetVisualsForItem(const ContentId& id,
VisualsCallback callback) {
auto components = GetEntryKeyComponents(id.id);

auto* storage_partition = content::BrowserContext::GetStoragePartitionForSite(
auto* storage_partition = content::BrowserContext::GetStoragePartitionForUrl(
profile_, components.origin.GetURL(), /* can_create= */ false);

if (!storage_partition || !storage_partition->GetContentIndexContext()) {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/devtools/devtools_ui_bindings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ void DevToolsUIBindings::LoadNetworkResource(DispatchCallback callback,
return;
}
} else {
auto* partition = content::BrowserContext::GetStoragePartitionForSite(
auto* partition = content::BrowserContext::GetStoragePartitionForUrl(
web_contents_->GetBrowserContext(), gurl);
url_loader_factory = partition->GetURLLoaderFactoryForBrowserProcess();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ void FileSystemRenameHandler::StartInternal() {
scoped_refptr<network::SharedURLLoaderFactory>
FileSystemRenameHandler::GetURLLoaderFactory(content::BrowserContext* context) {
content::StoragePartition* partition =
content::BrowserContext::GetStoragePartitionForSite(context,
settings_.home);
content::BrowserContext::GetStoragePartitionForUrl(context,
settings_.home);
return partition->GetURLLoaderFactoryForBrowserProcess();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ void FileSystemSigninDialogDelegate::DidFinishNavigation(
}

content::StoragePartition* partition =
content::BrowserContext::GetStoragePartitionForSite(
content::BrowserContext::GetStoragePartitionForUrl(
web_view_->GetBrowserContext(), GURL(kFileSystemBoxEndpointApi));
auto url_loader = partition->GetURLLoaderFactoryForBrowserProcess();
auto callback =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ TEST_F(NotificationTriggerSchedulerTest,
EXPECT_CALL(*data2.scheduler_, TriggerNotificationsForStoragePartition(_))
.Times(0);

auto* partition1 = content::BrowserContext::GetStoragePartitionForSite(
auto* partition1 = content::BrowserContext::GetStoragePartitionForUrl(
data1.profile_, GURL("http://example.com"));
auto* partition2 = content::BrowserContext::GetStoragePartitionForSite(
auto* partition2 = content::BrowserContext::GetStoragePartitionForUrl(
data2.profile_, GURL("http://example.com"));

auto now = base::Time::Now();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ void PrefetchProxyPageLoadMetricsObserver::CheckForCookiesOnURL(
content::BrowserContext* browser_context,
const GURL& url) {
content::StoragePartition* partition =
content::BrowserContext::GetStoragePartitionForSite(browser_context, url);
content::BrowserContext::GetStoragePartitionForUrl(browser_context, url);

partition->GetCookieManagerForBrowserProcess()->GetCookieList(
url, net::CookieOptions::MakeAllInclusive(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchCookie) {
PrefetchFromURL(url, FINAL_STATUS_NOSTATE_PREFETCH_FINISHED);

content::StoragePartition* storage_partition =
content::BrowserContext::GetStoragePartitionForSite(
content::BrowserContext::GetStoragePartitionForUrl(
current_browser()->profile(), url, false);
net::CookieOptions options = net::CookieOptions::MakeAllInclusive();
base::RunLoop loop;
Expand All @@ -856,7 +856,7 @@ IN_PROC_BROWSER_TEST_F(NoStatePrefetchBrowserTest, PrefetchCookieCrossDomain) {
// While the request is cross-site, it's permitted to set (implicitly) lax
// cookies on a cross-site navigation.
content::StoragePartition* storage_partition =
content::BrowserContext::GetStoragePartitionForSite(
content::BrowserContext::GetStoragePartitionForUrl(
current_browser()->profile(), cross_domain_url, false);
net::CookieOptions options = net::CookieOptions::MakeAllInclusive();
base::RunLoop loop;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1287,7 +1287,7 @@ void PrefetchProxyTabHelper::CheckEligibilityOfURL(
// place where service workers are observed by
// |PrefetchProxyServiceWorkersObserver|.
if (default_storage_partition !=
content::BrowserContext::GetStoragePartitionForSite(
content::BrowserContext::GetStoragePartitionForUrl(
profile, url,
/*can_create=*/false)) {
std::move(result_callback)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void RecordUserVisibleStatus(blink::mojom::PushUserVisibleStatus status) {

content::StoragePartition* GetStoragePartition(Profile* profile,
const GURL& origin) {
return content::BrowserContext::GetStoragePartitionForSite(profile, origin);
return content::BrowserContext::GetStoragePartitionForUrl(profile, origin);
}

NotificationDatabaseData CreateDatabaseData(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1620,7 +1620,7 @@ instance_id::InstanceIDDriver* PushMessagingServiceImpl::GetInstanceIDDriver()
content::DevToolsBackgroundServicesContext*
PushMessagingServiceImpl::GetDevToolsContext(const GURL& origin) const {
auto* storage_partition =
content::BrowserContext::GetStoragePartitionForSite(profile_, origin);
content::BrowserContext::GetStoragePartitionForUrl(profile_, origin);
if (!storage_partition)
return nullptr;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ void SyncFileSystemService::DumpFiles(const GURL& origin,
DCHECK(!origin.is_empty());

content::StoragePartition* storage_partition =
content::BrowserContext::GetStoragePartitionForSite(profile_, origin);
content::BrowserContext::GetStoragePartitionForUrl(profile_, origin);
storage::FileSystemContext* file_system_context =
storage_partition->GetFileSystemContext();
local_service_->MaybeInitializeFileSystemContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ ServiceWorkerRegistrationWaiter::ServiceWorkerRegistrationWaiter(
const GURL& url)
: url_(std::move(url)) {
content::StoragePartition* storage_partition =
content::BrowserContext::GetStoragePartitionForSite(browser_context,
url_);
content::BrowserContext::GetStoragePartitionForUrl(browser_context, url_);
DCHECK(storage_partition);

service_worker_context_ = storage_partition->GetServiceWorkerContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void BackgroundSyncControllerImpl::OnContentSettingChanged(
continue;

auto* storage_partition =
content::BrowserContext::GetStoragePartitionForSite(
content::BrowserContext::GetStoragePartitionForUrl(
browser_context_, origin.GetURL(), /* can_create= */ false);
if (!storage_partition)
continue;
Expand Down
2 changes: 1 addition & 1 deletion components/payments/content/payment_handler_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ content::DevToolsBackgroundServicesContext* GetDevTools(
if (!web_contents)
return nullptr;

auto* storage_partition = content::BrowserContext::GetStoragePartitionForSite(
auto* storage_partition = content::BrowserContext::GetStoragePartitionForUrl(
web_contents->GetBrowserContext(), sw_origin.GetURL(),
/*can_create=*/true);
if (!storage_partition)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class BackgroundSyncLauncherTest : public testing::Test {
DCHECK(!urls.empty());

for (const auto& url : urls) {
auto* storage_partition = BrowserContext::GetStoragePartitionForSite(
auto* storage_partition = BrowserContext::GetStoragePartitionForUrl(
&test_browser_context_, url);

auto iter = wakeup_deltas.find(url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ class BackgroundSyncManagerTest
// Create a StoragePartition with the correct BrowserContext so that the
// BackgroundSyncManager can find the BrowserContext through it.
storage_partition_impl_ = static_cast<StoragePartitionImpl*>(
BrowserContext::GetStoragePartitionForSite(
helper_->browser_context(), GURL("https://example.com")));
BrowserContext::GetStoragePartitionForUrl(helper_->browser_context(),
GURL("https://example.com")));
helper_->context_wrapper()->set_storage_partition(storage_partition_impl_);

SetMaxSyncAttemptsAndRestartManager(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ class BackgroundSyncSchedulerTest : public testing::Test {
auto* scheduler = BackgroundSyncScheduler::GetFor(&test_browser_context_);
DCHECK(scheduler);
auto* storage_partition = static_cast<StoragePartitionImpl*>(
BrowserContext::GetStoragePartitionForSite(&test_browser_context_,
url));
BrowserContext::GetStoragePartitionForUrl(&test_browser_context_, url));
DCHECK(storage_partition);

scheduler->ScheduleDelayedProcessing(storage_partition, sync_type, delay,
Expand All @@ -74,8 +73,7 @@ class BackgroundSyncSchedulerTest : public testing::Test {
auto* scheduler = BackgroundSyncScheduler::GetFor(&test_browser_context_);
DCHECK(scheduler);
auto* storage_partition = static_cast<StoragePartitionImpl*>(
BrowserContext::GetStoragePartitionForSite(&test_browser_context_,
url));
BrowserContext::GetStoragePartitionForUrl(&test_browser_context_, url));
DCHECK(storage_partition);

scheduler->CancelDelayedProcessing(storage_partition, sync_type);
Expand Down
24 changes: 11 additions & 13 deletions content/browser/browser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,13 @@ StoragePartition* BrowserContext::GetStoragePartition(
BrowserContext* browser_context,
SiteInstance* site_instance,
bool can_create) {
if (!site_instance) {
return GetStoragePartition(
browser_context, StoragePartitionConfig::CreateDefault(browser_context),
can_create);
}

return GetStoragePartitionForSite(browser_context,
site_instance->GetSiteURL(), can_create);
auto* site_instance_impl = static_cast<SiteInstanceImpl*>(site_instance);
auto partition_config =
site_instance_impl
? site_instance_impl->GetSiteInfo().GetStoragePartitionConfig(
browser_context)
: StoragePartitionConfig::CreateDefault(browser_context);
return GetStoragePartition(browser_context, partition_config, can_create);
}

StoragePartition* BrowserContext::GetStoragePartition(
Expand All @@ -245,13 +244,12 @@ StoragePartition* BrowserContext::GetStoragePartition(
return partition_map->Get(storage_partition_config, can_create);
}

StoragePartition* BrowserContext::GetStoragePartitionForSite(
StoragePartition* BrowserContext::GetStoragePartitionForUrl(
BrowserContext* browser_context,
const GURL& site,
const GURL& url,
bool can_create) {
auto storage_partition_config =
GetContentClient()->browser()->GetStoragePartitionConfigForSite(
browser_context, site);
auto storage_partition_config = SiteInfo::GetStoragePartitionConfigForUrl(
browser_context, url, /*is_site_url=*/false);

return GetStoragePartition(browser_context, storage_partition_config,
can_create);
Expand Down
20 changes: 13 additions & 7 deletions content/browser/download/download_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,16 @@ StoragePartitionImpl* GetStoragePartition(BrowserContext* context,
BrowserContext::GetStoragePartition(context, site_instance));
}

// TODO(acolwell): Update DownloadManager and related code to pass around
// StoragePartitionConfig objects instead of site URLs.
StoragePartitionImpl* GetStoragePartitionForSiteUrl(BrowserContext* context,
const GURL& site_url) {
auto partition_config = SiteInfo::GetStoragePartitionConfigForUrl(
context, site_url, /*is_site_url=*/true);
return static_cast<StoragePartitionImpl*>(
BrowserContext::GetStoragePartition(context, partition_config));
}

void OnDownloadStarted(
download::DownloadItemImpl* download,
download::DownloadUrlParameters::OnStartedCallback on_started) {
Expand Down Expand Up @@ -1301,9 +1311,7 @@ void DownloadManagerImpl::BeginResourceDownloadOnChecksComplete(
base::flat_set<std::string>()));
} else if (rfh && params->url().SchemeIsFileSystem()) {
StoragePartitionImpl* storage_partition =
static_cast<StoragePartitionImpl*>(
BrowserContext::GetStoragePartitionForSite(browser_context_,
site_url));
GetStoragePartitionForSiteUrl(browser_context_, site_url);

pending_url_loader_factory =
std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
Expand Down Expand Up @@ -1336,9 +1344,7 @@ void DownloadManagerImpl::BeginResourceDownloadOnChecksComplete(
}
} else {
StoragePartitionImpl* storage_partition =
static_cast<StoragePartitionImpl*>(
BrowserContext::GetStoragePartitionForSite(browser_context_,
site_url));
GetStoragePartitionForSiteUrl(browser_context_, site_url);
pending_url_loader_factory =
CreatePendingSharedURLLoaderFactory(storage_partition, rfh, true);
}
Expand Down Expand Up @@ -1369,7 +1375,7 @@ void DownloadManagerImpl::BeginDownloadInternal(
// a new factory if we don't have one already.
if (!blob_url_loader_factory && params->url().SchemeIsBlob()) {
blob_url_loader_factory = ChromeBlobStorageContext::URLLoaderFactoryForUrl(
BrowserContext::GetStoragePartitionForSite(browser_context_, site_url),
GetStoragePartitionForSiteUrl(browser_context_, site_url),
params->url());
}

Expand Down
2 changes: 1 addition & 1 deletion content/browser/notifications/devtools_event_logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ DevToolsBackgroundServicesContext* GetDevToolsContext(
BrowserContext* browser_context,
const GURL& origin) {
auto* storage_partition =
BrowserContext::GetStoragePartitionForSite(browser_context, origin);
BrowserContext::GetStoragePartitionForUrl(browser_context, origin);
if (!storage_partition)
return nullptr;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ void DispatchNotificationEvent(
DCHECK(origin.is_valid());

StoragePartition* partition =
BrowserContext::GetStoragePartitionForSite(browser_context, origin);
BrowserContext::GetStoragePartitionForUrl(browser_context, origin);

scoped_refptr<ServiceWorkerContextWrapper> service_worker_context =
static_cast<ServiceWorkerContextWrapper*>(
Expand Down
2 changes: 1 addition & 1 deletion content/browser/payments/payment_app_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ scoped_refptr<DevToolsBackgroundServicesContextImpl>
PaymentAppProviderImpl::GetDevTools(const url::Origin& sw_origin) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(payment_request_web_contents_);
auto* storage_partition = BrowserContext::GetStoragePartitionForSite(
auto* storage_partition = BrowserContext::GetStoragePartitionForUrl(
payment_request_web_contents_->GetBrowserContext(), sw_origin.GetURL(),
/*can_create=*/true);
if (!storage_partition)
Expand Down
Loading

0 comments on commit b54ca39

Please sign in to comment.