Skip to content

Commit

Permalink
Revert "StoragePartition: Move shutdown to OnBrowserContextWillBeDest…
Browse files Browse the repository at this point in the history
…royed"

This reverts commit 0e67584.

Reason for revert: this is causing some crashes, most likely due to
async OTR profile deletion.

Original change's description:
> StoragePartition: Move shutdown to OnBrowserContextWillBeDestroyed
>
> Follow-up from crrev.com/c/4869214. This moves shutting down of
> services in ~StoragePartitionImpl to OnBrowserContextWillBeDestroyed.
> Doing this earlier will prevent other dangling pointer issues related
> to browser context (including fixing one in StoragePartitionImpl), and
> it consolidates shutdown.
>
> Bug: 1492282
> Change-Id: I57e1e439ad715df6625ef011c1a99a76051fac2e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4968658
> Commit-Queue: Scott Haseley <shaseley@chromium.org>
> Reviewed-by: Ayu Ishii <ayui@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1215818}

(cherry picked from commit b9f4336)

Bug: 1492282, 1497693
Change-Id: Id8b4fdc3989299886caa13277d1b7311cef79007
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4990213
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Scott Haseley <shaseley@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1217729}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4994750
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Owners-Override: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/6099@{#12}
Cr-Branched-From: e6ee450-refs/heads/main@{#1217362}
  • Loading branch information
shaseley authored and Chromium LUCI CQ committed Oct 31, 2023
1 parent 1008145 commit c9c51cb
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 28 deletions.
53 changes: 26 additions & 27 deletions content/browser/storage_partition_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1267,32 +1267,7 @@ StoragePartitionImpl::~StoragePartitionImpl() {
#if DCHECK_IS_ON()
DCHECK(on_browser_context_will_be_destroyed_called_);
#endif
}

void StoragePartitionImpl::OnBrowserContextWillBeDestroyed() {
#if DCHECK_IS_ON()
on_browser_context_will_be_destroyed_called_ = true;
#endif
// Shut down service worker and shared worker machinery because these can keep
// RenderProcessHosts and SiteInstances alive, and the codebase assumes these
// are destroyed before the BrowserContext is destroyed.
GetServiceWorkerContext()->Shutdown();
GetSharedWorkerService()->Shutdown();

// These may hold raw pointers to objects that are about to be destroyed,
// before this object is destroyed. Shut them down now to avoid dangling
// pointers.
if (GetFileSystemAccessManager()) {
GetFileSystemAccessManager()->Shutdown();
}

if (GetContentIndexContext()) {
GetContentIndexContext()->Shutdown();
}

if (keep_alive_url_loader_service_) {
keep_alive_url_loader_service_->Shutdown();
}
browser_context_ = nullptr;

if (url_loader_factory_getter_) {
url_loader_factory_getter_->OnStoragePartitionDestroyed();
Expand Down Expand Up @@ -1338,8 +1313,32 @@ void StoragePartitionImpl::OnBrowserContextWillBeDestroyed() {
if (GetGeneratedCodeCacheContext()) {
GetGeneratedCodeCacheContext()->Shutdown();
}
}

browser_context_ = nullptr;
void StoragePartitionImpl::OnBrowserContextWillBeDestroyed() {
#if DCHECK_IS_ON()
on_browser_context_will_be_destroyed_called_ = true;
#endif

// Shut down service worker and shared worker machinery because these can keep
// RenderProcessHosts and SiteInstances alive, and the codebase assumes these
// are destroyed before the BrowserContext is destroyed.
GetServiceWorkerContext()->Shutdown();
GetSharedWorkerService()->Shutdown();

// These hold raw pointers to objects that are about to be destroyed, before
// this object is destroyed. Shut them down now to avoid dangling pointers.
if (GetFileSystemAccessManager()) {
GetFileSystemAccessManager()->Shutdown();
}

if (GetContentIndexContext()) {
GetContentIndexContext()->Shutdown();
}

if (keep_alive_url_loader_service_) {
keep_alive_url_loader_service_->Shutdown();
}
}

// static
Expand Down
2 changes: 1 addition & 1 deletion content/browser/storage_partition_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ class CONTENT_EXPORT StoragePartitionImpl
// Raw pointer that should always be valid. The BrowserContext owns the
// StoragePartitionImplMap which then owns StoragePartitionImpl. When the
// BrowserContext is destroyed, `this` will be destroyed too.
raw_ptr<BrowserContext> browser_context_;
raw_ptr<BrowserContext, DanglingUntriaged> browser_context_;

const base::FilePath partition_path_;

Expand Down

0 comments on commit c9c51cb

Please sign in to comment.