From 64c011ab17217365520868c76895a7fe5d416e99 Mon Sep 17 00:00:00 2001
From: Jelle Foks <jfoks@google.com>
Date: Wed, 4 Dec 2024 15:49:48 -0800
Subject: [PATCH] Ensure correct destruction order for FileProxy.

This ensures that the base::FileProxy in FileFetcher is destroyed before
the TaskRunner that it's posting to from the destructor.

This may help with some of the ANRs.

b/304601532
---
 cobalt/loader/file_fetcher.cc | 16 ++++++++++------
 cobalt/loader/file_fetcher.h  |  2 +-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/cobalt/loader/file_fetcher.cc b/cobalt/loader/file_fetcher.cc
index c12130ffb5ef..19ad777117f4 100644
--- a/cobalt/loader/file_fetcher.cc
+++ b/cobalt/loader/file_fetcher.cc
@@ -85,7 +85,7 @@ FileFetcher::FileFetcher(const base::FilePath& file_path, Handler* handler,
       task_runner_(options.task_runner_proxy),
       file_path_(file_path),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
-      file_proxy_(task_runner_.get()) {
+      file_proxy_(new base::FileProxy(task_runner_.get())) {
   DCHECK_GT(buffer_size_, 0);
 
   // Ensure the request does not attempt to navigate outside the whitelisted
@@ -107,6 +107,10 @@ FileFetcher::FileFetcher(const base::FilePath& file_path, Handler* handler,
 FileFetcher::~FileFetcher() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
+  // Ensure FileProxy runs its destructor before the TaskRunner that it posts
+  // to is destroyes.
+  file_proxy_.reset();
+
   if (task_runner_ != base::SequencedTaskRunner::GetCurrentDefault()) {
     // In case we are currently in the middle of a fetch (in which case it will
     // be aborted), invalidate the weak pointers to this FileFetcher object to
@@ -145,10 +149,10 @@ void FileFetcher::TryFileOpen() {
   base::FilePath actual_file_path;
   actual_file_path = curr_search_path_iter_->Append(file_path_);
 
-  file_proxy_.CreateOrOpen(actual_file_path,
-                           base::File::FLAG_OPEN | base::File::FLAG_READ,
-                           base::Bind(&FileFetcher::DidCreateOrOpen,
-                                      weak_ptr_factory_.GetWeakPtr()));
+  file_proxy_->CreateOrOpen(actual_file_path,
+                            base::File::FLAG_OPEN | base::File::FLAG_READ,
+                            base::Bind(&FileFetcher::DidCreateOrOpen,
+                                       weak_ptr_factory_.GetWeakPtr()));
 }
 
 void FileFetcher::ReadNextChunk() {
@@ -156,7 +160,7 @@ void FileFetcher::ReadNextChunk() {
   if (bytes_to_read > bytes_left_to_read_) {
     bytes_to_read = static_cast<int32>(bytes_left_to_read_);
   }
-  file_proxy_.Read(
+  file_proxy_->Read(
       file_offset_, bytes_to_read,
       base::Bind(&FileFetcher::DidRead, weak_ptr_factory_.GetWeakPtr()));
 }
diff --git a/cobalt/loader/file_fetcher.h b/cobalt/loader/file_fetcher.h
index 3beed174c756..aafeeb6ad0fc 100644
--- a/cobalt/loader/file_fetcher.h
+++ b/cobalt/loader/file_fetcher.h
@@ -104,7 +104,7 @@ class FileFetcher : public Fetcher {
   base::WeakPtrFactory<FileFetcher> weak_ptr_factory_;
   // Current iterator into the search path vector.
   std::vector<base::FilePath>::const_iterator curr_search_path_iter_;
-  base::FileProxy file_proxy_;
+  std::unique_ptr<base::FileProxy> file_proxy_;
 };
 
 }  // namespace loader