[RUN-3037] WIP wait for recording to finish before shutting renderers down

chrome/browser/lifetime/browser_shutdown.cc

@@ -68,11 +68,13 @@
 #include "components/rlz/rlz_tracker.h"  // nogncheck crbug.com/1125897
 #endif
 
-#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
 #include "base/run_loop.h"
+#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
 #include "content/public/browser/profiling_utils.h"
 #endif
 
+#include "content/public/browser/recording_utils.h"
+
 namespace browser_shutdown {
 namespace {
 
@@ -200,6 +202,12 @@ void OnShutdownStarting(ShutdownType type) {
   nested_run_loop.Run();
 #endif  // BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
 
+  // Wait for all recording child processes to finish their recording, so
+  // we don't pollute it with actual process shutdown.
+  base::RunLoop nested_run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+  content::RecordReplayAskAllChildrenToFinishRecording(nested_run_loop.QuitClosure());
+  nested_run_loop.Run();
+
   // Call FastShutdown on all of the RenderProcessHosts.  This will be
   // a no-op in some cases, so we still need to go through the normal
   // shutdown path for the ones that didn't exit here.

chrome/browser/ui/tabs/tab_strip_model.cc

@@ -19,6 +19,7 @@
 #include "base/metrics/user_metrics.h"
 #include "base/observer_list.h"
 #include "base/ranges/algorithm.h"
+#include "base/run_loop.h"
 #include "base/scoped_observation.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -58,6 +59,7 @@
 #include "components/tab_groups/tab_group_id.h"
 #include "components/tab_groups/tab_group_visual_data.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
+#include "content/public/browser/recording_utils.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_observer.h"
@@ -178,6 +180,23 @@ class RenderWidgetHostVisibilityTracker final
 
 }  // namespace
 
+class RefCountedScopedClosureRunner
+    : public base::RefCounted<RefCountedScopedClosureRunner> {
+ public:
+  RefCountedScopedClosureRunner(base::OnceClosure callback);
+
+ private:
+  friend class base::RefCounted<RefCountedScopedClosureRunner>;
+  ~RefCountedScopedClosureRunner() = default;
+
+  base::ScopedClosureRunner destruction_callback_;
+};
+
+RefCountedScopedClosureRunner::RefCountedScopedClosureRunner(
+    base::OnceClosure callback)
+    : destruction_callback_(std::move(callback)) {}
+
+
 TabGroupModelFactory::TabGroupModelFactory() {
   DCHECK(!factory_instance);
   factory_instance = this;
@@ -1867,6 +1886,21 @@ bool TabStripModel::CloseWebContentses(
       ++processes[process];
     }
 
+    // Synchronously stop recording the tabs we're closing.
+    // TODO(toshok) doing it here and this way is wrong since there might be an unload
+    // handler that cancels the close, and we'll want to record that (and continue the
+    // recording.)
+    std::vector<content::RenderProcessHost*> closing_renderers;
+    for (const auto& pair : processes) {
+      closing_renderers.push_back(pair.first);
+    }
+
+    base::RunLoop nested_run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+    content::RecordReplayAskChildrenToFinishRecording(closing_renderers, nested_run_loop.QuitClosure());
+    nested_run_loop.Run();
+    // At this point, all renderers have stopped their recordings and we can continue
+    // shutting them down.
+
     // Try to fast shutdown the tabs that can close.
     for (const auto& pair : processes)
       pair.first->FastShutdownIfPossible(pair.second, false);

content/browser/BUILD.gn

@@ -1520,6 +1520,7 @@ source_set("browser") {
     "quota/quota_internals_ui.h",
     "quota/quota_manager_host.cc",
     "quota/quota_manager_host.h",
+    "recording_utils.cc",
     "reduce_accept_language/reduce_accept_language_throttle.cc",
     "reduce_accept_language/reduce_accept_language_throttle.h",
     "reduce_accept_language/reduce_accept_language_utils.cc",

content/browser/recording_utils.cc

@@ -0,0 +1,81 @@
+#include <memory>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/callback_forward.h"
+#include "base/callback_helpers.h"
+#include "base/command_line.h"
+#include "base/memory/ref_counted.h"
+#include "content/public/browser/browser_child_process_host_iterator.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/recording_utils.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/common/content_switches.h"
+
+namespace content {
+
+namespace {
+
+// A refcounted class that runs a closure once it's destroyed.
+class RefCountedScopedClosureRunner
+    : public base::RefCounted<RefCountedScopedClosureRunner> {
+ public:
+  RefCountedScopedClosureRunner(base::OnceClosure callback);
+
+ private:
+  friend class base::RefCounted<RefCountedScopedClosureRunner>;
+  ~RefCountedScopedClosureRunner() = default;
+
+  base::ScopedClosureRunner destruction_callback_;
+};
+
+RefCountedScopedClosureRunner::RefCountedScopedClosureRunner(
+    base::OnceClosure callback)
+    : destruction_callback_(std::move(callback)) {}
+
+}  // namespace
+
+void RecordReplayAskAllChildrenToFinishRecording(base::OnceClosure callback) {
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSingleProcess)) {
+    // TODO(toshok) not sure - maybe we call recordreplay::FinishRecording directly?
+    // do we support single process mode at all?
+    return;
+  }
+
+  auto closure_runner =
+      base::MakeRefCounted<RefCountedScopedClosureRunner>(std::move(callback));
+
+  // Ask all the renderer processes to finish their recordings.
+  for (content::RenderProcessHost::iterator i(content::RenderProcessHost::AllHostsIterator());
+       !i.IsAtEnd(); i.Advance()) {
+    DCHECK(!i.GetCurrentValue()->GetProcess().is_current());
+    if (!i.GetCurrentValue()->IsInitializedAndNotDead())
+      continue;
+    i.GetCurrentValue()->FinishRecording(base::BindOnce(
+        [](scoped_refptr<RefCountedScopedClosureRunner>) {}, closure_runner));
+  }
+}
+
+void RecordReplayAskChildrenToFinishRecording(base::span<RenderProcessHost*> hosts, base::OnceClosure callback) {
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSingleProcess)) {
+    // TODO(toshok) not sure - maybe we call recordreplay::FinishRecording directly?
+    // do we support single process mode at all?
+    return;
+  }
+
+  auto closure_runner =
+      base::MakeRefCounted<RefCountedScopedClosureRunner>(std::move(callback));
+
+  for (RenderProcessHost* host : hosts) {
+        DCHECK(!host->GetProcess().is_current());
+        if (!host->IsInitializedAndNotDead())
+        continue;
+        host->FinishRecording(base::BindOnce(
+            [](scoped_refptr<RefCountedScopedClosureRunner>) {}, closure_runner));
+    }
+}
+
+}  // namespace content

content/browser/renderer_host/render_process_host_impl.cc

@@ -2231,6 +2231,11 @@ void RenderProcessHostImpl::DumpProfilingData(base::OnceClosure callback) {
 }
 #endif
 
+void RenderProcessHostImpl::FinishRecording(base::OnceClosure callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  GetRendererInterface()->FinishRecording(std::move(callback));
+}
+
 void RenderProcessHostImpl::WriteIntoTrace(
     perfetto::TracedProto<perfetto::protos::pbzero::RenderProcessHost> proto)
     const {

content/browser/renderer_host/render_process_host_impl.h

@@ -318,6 +318,7 @@ class CONTENT_EXPORT RenderProcessHostImpl
   void DumpProfilingData(base::OnceClosure callback) override;
 #endif
 
+  void FinishRecording(base::OnceClosure callback) override;
   void PauseSocketManagerForRenderFrameHost(
       const GlobalRenderFrameHostId& render_frame_host_id) override;
   void ResumeSocketManagerForRenderFrameHost(

content/common/renderer.mojom

@@ -146,6 +146,8 @@ interface Renderer {
   [EnableIf=clang_profiling_inside_sandbox]
   WriteClangProfilingProfile() => ();
 
+  FinishRecording() => ();
+
   // Set whether this renderer process is "cross-origin isolated". This
   // corresponds to agent cluster's "cross-origin isolated" concept.
   // TODO(yhirano): Have the spec URL.

content/public/browser/BUILD.gn

@@ -314,6 +314,7 @@ source_set("browser_sources") {
     "push_messaging_service.cc",
     "push_messaging_service.h",
     "quota_permission_context.h",
+    "recording_utils.h",
     "reduce_accept_language_controller_delegate.h",
     "reload_type.h",
     "render_frame_host.h",

content/public/browser/recording_utils.h

@@ -0,0 +1,27 @@
+// Copyright 2021 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_PUBLIC_BROWSER_RECORDING_UTILS_H_
+#define CONTENT_PUBLIC_BROWSER_RECORDING_UTILS_H_
+
+#include "base/callback_forward.h"
+#include "base/containers/span.h"
+#include "content/common/content_export.h"
+#include "content/public/browser/render_process_host.h"
+
+namespace content {
+
+// Ask all the child processes to stop recording, make sure
+// the recording is flushed, and calls |callback| once it's done.
+CONTENT_EXPORT void RecordReplayAskAllChildrenToFinishRecording(
+    base::OnceClosure callback);
+
+// Same as above, but a subset of renderer children
+CONTENT_EXPORT void RecordReplayAskChildrenToFinishRecording(
+    base::span<RenderProcessHost*> render_processes,
+    base::OnceClosure callback);
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_RECORDING_UTILS_H_

content/public/browser/render_process_host.h

@@ -659,6 +659,7 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender,
   virtual void DumpProfilingData(base::OnceClosure callback) {}
 #endif
 
+  virtual void FinishRecording(base::OnceClosure callback) {}
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   // Reinitializes the child process's logging with the given settings. This
   // is needed on Chrome OS, which switches to a log file in the user's home

content/renderer/render_thread_impl.cc

@@ -1384,6 +1384,19 @@ void RenderThreadImpl::WriteClangProfilingProfile(
 }
 #endif
 
+void RenderThreadImpl::FinishRecording(FinishRecordingCallback callback) {
+  if (recordreplay::IsRecordingOrReplaying()) {
+    recordreplay::FinishRecording();
+    // FinishRecording will cause the process to exit,
+    // though the _exit call may happen on another thread
+    // asynchronously. either way we don't want to call the
+    // callback here.
+  } else {
+    // If we're not recording, just call the callback.
+    std::move(callback).Run();
+  }
+}
+
 void RenderThreadImpl::SetIsCrossOriginIsolated(bool value) {
   blink::SetIsCrossOriginIsolated(value);
 }

content/renderer/render_thread_impl.h

@@ -441,6 +441,7 @@ class CONTENT_EXPORT RenderThreadImpl
   void WriteClangProfilingProfile(
       WriteClangProfilingProfileCallback callback) override;
 #endif
+  void FinishRecording(FinishRecordingCallback callback) override;
   void SetIsCrossOriginIsolated(bool value) override;
   void RecordReplayBrowserEvent(const std::string& name, base::Value::Dict value) override;
   void SetIsIsolatedApplication(bool value) override;