ProfileDestroyer: allow custom force-destruction timeout

This is useful for automation scenarios where slow computers
may not be able to close all processes in two seconds, but
will eventually succeed.

Currently, we hit CHECK in ~ProfileDestroyer, most likely because
we force-destroy upon timeout of ~2 seconds. This change allows
automation profiles to provide a custom timeout that would be
enough to destroy all render process hosts.

(cherry picked from commit bcc5a28)

Bug: 1349150
Change-Id: Iadbd45a6e23c90b1dae43c23b3c4de7930727c8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3821446
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1037431}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3846107
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Auto-Submit: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/branch-heads/5249@{#35}
Cr-Branched-From: 4f7bea5-refs/heads/main@{#1036826}

chrome/browser/devtools/devtools_browser_context_manager.cc

@@ -14,6 +14,12 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 
+namespace {
+
+const int64_t kDestroyProfileTimeoutSeconds = 60;
+
+}  // namespace
+
 DevToolsBrowserContextManager::DevToolsBrowserContextManager() {}
 
 DevToolsBrowserContextManager::~DevToolsBrowserContextManager() = default;
@@ -88,7 +94,8 @@ void DevToolsBrowserContextManager::DisposeBrowserContext(
   if (!has_opened_browser) {
     otr_profiles_.erase(it);
     profile->RemoveObserver(this);
-    ProfileDestroyer::DestroyProfileWhenAppropriate(profile);
+    ProfileDestroyer::DestroyProfileWhenAppropriateWithTimeout(
+        profile, base::Seconds(kDestroyProfileTimeoutSeconds));
     std::move(callback).Run(true, "");
     return;
   }
@@ -134,8 +141,10 @@ void DevToolsBrowserContextManager::OnBrowserRemoved(Browser* browser) {
   // during the browser tear-down process.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::BindOnce(&ProfileDestroyer::DestroyProfileWhenAppropriate,
-                     base::Unretained(otr_profile)));
+      base::BindOnce(
+          &ProfileDestroyer::DestroyProfileWhenAppropriateWithTimeout,
+          base::Unretained(otr_profile),
+          base::Seconds(kDestroyProfileTimeoutSeconds)));
 
   std::move(pending_disposal->second).Run(true, "");
   pending_context_disposals_.erase(pending_disposal);

chrome/browser/profiles/profile_destroyer.cc

@@ -53,6 +53,14 @@ DestroyerSet& PendingDestroyers() {
 
 // static
 void ProfileDestroyer::DestroyProfileWhenAppropriate(Profile* const profile) {
+  DestroyProfileWhenAppropriateWithTimeout(profile,
+                                           base::Seconds(kTimerDelaySeconds));
+}
+
+// static
+void ProfileDestroyer::DestroyProfileWhenAppropriateWithTimeout(
+    Profile* const profile,
+    base::TimeDelta timeout) {
   if (!profile)  // profile might have been reset in ResetPendingDestroyers();
     return;
 
@@ -96,7 +104,7 @@ void ProfileDestroyer::DestroyProfileWhenAppropriate(Profile* const profile) {
     // The instance will destroy itself once all (non-spare) render process
     // hosts referring to it are properly terminated. This happens in the two
     // "final" state: Retry() and Timeout().
-    new ProfileDestroyer(profile, profile_hosts);
+    new ProfileDestroyer(profile, profile_hosts, timeout);
     return;
   }
 
@@ -219,8 +227,12 @@ void ProfileDestroyer::ResetPendingDestroyers(Profile* const profile) {
   }
 }
 
-ProfileDestroyer::ProfileDestroyer(Profile* const profile, const HostSet& hosts)
-    : profile_(profile), profile_ptr_(reinterpret_cast<uint64_t>(profile)) {
+ProfileDestroyer::ProfileDestroyer(Profile* const profile,
+                                   const HostSet& hosts,
+                                   base::TimeDelta timeout)
+    : profile_(profile),
+      timeout_(timeout),
+      profile_ptr_(reinterpret_cast<uint64_t>(profile)) {
   TRACE_EVENT("shutdown", "ProfileDestroyer::ProfileDestroyer",
               [&](perfetto::EventContext ctx) {
                 auto* proto =
@@ -236,8 +248,8 @@ ProfileDestroyer::ProfileDestroyer(Profile* const profile, const HostSet& hosts)
   DCHECK(observations_.IsObservingAnySource());
 
   // We don't want to wait for RenderProcessHost to be destroyed longer than
-  // kTimerDelaySeconds.
-  timer_.Start(FROM_HERE, base::Seconds(kTimerDelaySeconds),
+  // timeout.
+  timer_.Start(FROM_HERE, timeout,
                base::BindOnce(&ProfileDestroyer::Timeout,
                               weak_ptr_factory_.GetWeakPtr()));
 }
@@ -305,7 +317,7 @@ void ProfileDestroyer::Timeout() {
 }
 
 void ProfileDestroyer::Retry() {
-  DestroyProfileWhenAppropriate(profile_);
+  DestroyProfileWhenAppropriateWithTimeout(profile_, timeout_);
   delete this;  // Final state.
 }
 

chrome/browser/profiles/profile_destroyer.h

@@ -15,6 +15,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_process_host_observer.h"
 
+class DevToolsBrowserContextManager;
 class Profile;
 class ProfileImpl;
 
@@ -39,9 +40,20 @@ class ProfileDestroyer : public content::RenderProcessHostObserver {
   friend class ProfileImpl;
   friend class base::RefCounted<ProfileDestroyer>;
 
+  // For custom timeout, see DestroyProfileWhenAppropriateWithTimeout.
+  friend class DevToolsBrowserContextManager;
+
   using HostSet = std::set<content::RenderProcessHost*>;
 
-  ProfileDestroyer(Profile* const profile, const HostSet& hosts);
+  // Same as DestroyProfileWhenAppropriate, but configures how long to wait
+  // for render process hosts to be destroyed. Intended for testing/automation
+  // scenarios, where default timeout is too short.
+  static void DestroyProfileWhenAppropriateWithTimeout(Profile* const profile,
+                                                       base::TimeDelta timeout);
+
+  ProfileDestroyer(Profile* const profile,
+                   const HostSet& hosts,
+                   base::TimeDelta timeout);
   ~ProfileDestroyer() override;
 
   // content::RenderProcessHostObserver override.
@@ -90,6 +102,9 @@ class ProfileDestroyer : public content::RenderProcessHostObserver {
   // another instance of ProfileDestroyer that this instance is canceled.
   Profile* profile_;
 
+  // Force-destruction timeout.
+  const base::TimeDelta timeout_;
+
   // The initial value of |profile_| stored as uint64_t for traces. It is useful
   // for use in the destructor, because at the end, |profile_| is nullptr.
   const uint64_t profile_ptr_;