fix: rework lifetime mgmt of ClearDataTask/ClearDataOperation (#47411)

* fix: rework lifetime mgmt of ClearDataTask/ClearDataOperation

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* Update shell/browser/api/electron_api_session.cc

Co-authored-by: Robo <hop2deep@gmail.com>

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* Update shell/browser/api/electron_api_session.cc

Co-authored-by: Robo <hop2deep@gmail.com>

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* Update shell/browser/api/electron_api_session.cc

Co-authored-by: Robo <hop2deep@gmail.com>

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

* Update shell/browser/api/electron_api_session.cc

Co-authored-by: Robo <hop2deep@gmail.com>

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

shell/browser/api/electron_api_session.cc

@@ -84,6 +84,7 @@
 #include "shell/common/gin_converters/time_converter.h"
 #include "shell/common/gin_converters/usb_protected_classes_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
+#include "shell/common/gin_helper/cleaned_up_at_exit.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/error_thrower.h"
 #include "shell/common/gin_helper/object_template_builder.h"
@@ -198,9 +199,11 @@ std::vector<std::string> GetDataTypesFromMask(
 
 // Represents a task to clear browsing data for the `clearData` API method.
 //
-// This type manages its own lifetime, deleting itself once the task finishes
+// This type manages its own lifetime,
-// completely.
+// 1) deleting itself once all the operations created by this task are
-class ClearDataTask {
+// completed. 2) through gin_helper::CleanedUpAtExit, ensuring it's destroyed
+// before the node environment shuts down.
+class ClearDataTask : public gin_helper::CleanedUpAtExit {
  public:
   // Starts running a task. This function will return before the task is
   // finished, but will resolve or reject the |promise| when it finishes.
@@ -211,7 +214,7 @@ class ClearDataTask {
       std::vector<url::Origin> origins,
       BrowsingDataFilterBuilder::Mode filter_mode,
       BrowsingDataFilterBuilder::OriginMatchingMode origin_matching_mode) {
-    std::shared_ptr<ClearDataTask> task(new ClearDataTask(std::move(promise)));
+    auto* task = new ClearDataTask(std::move(promise));
 
     // This method counts as an operation. This is important so we can call
     // `OnOperationFinished` at the end of this method as a fallback if all the
@@ -255,42 +258,36 @@ class ClearDataTask {
     }
 
     // This static method counts as an operation.
-    task->OnOperationFinished(std::nullopt);
+    task->OnOperationFinished(nullptr, std::nullopt);
   }
 
  private:
   // An individual |content::BrowsingDataRemover::Remove...| operation as part
-  // of a full |ClearDataTask|. This class manages its own lifetime, cleaning
+  // of a full |ClearDataTask|. This class is owned by ClearDataTask and cleaned
-  // itself up after the operation completes and notifies the task of the
+  // up either when the operation completes or when ClearDataTask is destroyed.
-  // result.
   class ClearDataOperation : private BrowsingDataRemover::Observer {
    public:
-    static void Run(std::shared_ptr<ClearDataTask> task,
+    ClearDataOperation(ClearDataTask* task, BrowsingDataRemover* remover)
-                    BrowsingDataRemover* remover,
+        : task_(task) {
-                    BrowsingDataRemover::DataType data_type_mask,
+      observation_.Observe(remover);
-                    std::unique_ptr<BrowsingDataFilterBuilder> filter_builder) {
+    }
-      auto* operation = new ClearDataOperation(task, remover);
 
+    void Start(BrowsingDataRemover* remover,
+               BrowsingDataRemover::DataType data_type_mask,
+               std::unique_ptr<BrowsingDataFilterBuilder> filter_builder) {
       remover->RemoveWithFilterAndReply(base::Time::Min(), base::Time::Max(),
                                         data_type_mask, kClearOriginTypeAll,
-                                        std::move(filter_builder), operation);
+                                        std::move(filter_builder), this);
     }
 
     // BrowsingDataRemover::Observer:
     void OnBrowsingDataRemoverDone(
         BrowsingDataRemover::DataType failed_data_types) override {
-      task_->OnOperationFinished(failed_data_types);
+      task_->OnOperationFinished(this, failed_data_types);
-      delete this;
     }
 
    private:
-    ClearDataOperation(std::shared_ptr<ClearDataTask> task,
+    raw_ptr<ClearDataTask> task_;
-                       BrowsingDataRemover* remover)
-        : task_(task) {
-      observation_.Observe(remover);
-    }
-
-    std::shared_ptr<ClearDataTask> task_;
     base::ScopedObservation<BrowsingDataRemover, BrowsingDataRemover::Observer>
         observation_{this};
   };
@@ -299,18 +296,20 @@ class ClearDataTask {
       : promise_(std::move(promise)) {}
 
   static void StartOperation(
-      std::shared_ptr<ClearDataTask> task,
+      ClearDataTask* task,
       BrowsingDataRemover* remover,
       BrowsingDataRemover::DataType data_type_mask,
       std::unique_ptr<BrowsingDataFilterBuilder> filter_builder) {
     // Track this operation
     task->operations_running_ += 1;
 
-    ClearDataOperation::Run(task, remover, data_type_mask,
+    auto& operation = task->operations_.emplace_back(
-                            std::move(filter_builder));
+        std::make_unique<ClearDataOperation>(task, remover));
+    operation->Start(remover, data_type_mask, std::move(filter_builder));
   }
 
   void OnOperationFinished(
+      ClearDataOperation* operation,
       std::optional<BrowsingDataRemover::DataType> failed_data_types) {
     DCHECK_GT(operations_running_, 0);
     operations_running_ -= 1;
@@ -319,6 +318,16 @@ class ClearDataTask {
       failed_data_types_ |= failed_data_types.value();
     }
 
+    if (operation) {
+      operations_.erase(
+          std::remove_if(
+              operations_.begin(), operations_.end(),
+              [operation](const std::unique_ptr<ClearDataOperation>& op) {
+                return op.get() == operation;
+              }),
+          operations_.end());
+    }
+
     // If this is the last operation, then the task is finished
     if (operations_running_ == 0) {
       OnTaskFinished();
@@ -346,11 +355,14 @@ class ClearDataTask {
 
       promise_.Reject(error);
     }
+
+    delete this;
   }
 
   int operations_running_ = 0;
   BrowsingDataRemover::DataType failed_data_types_ = 0ULL;
   gin_helper::Promise<void> promise_;
+  std::vector<std::unique_ptr<ClearDataOperation>> operations_;
 };
 
 base::Value::Dict createProxyConfig(ProxyPrefs::ProxyMode proxy_mode,