[scheduling] Experiment to prioritize before unload handlers.

This CL is the first step in a series of experiments around prioritizing the unload pathway in the scheduler. The design doc[1] has more details.

This CL adds a new mojo interface 'UserBlockingLocalFrame' which allows the browser to send high priority messages to a local frame. These messages will run at a higher priority than legacy IPC messages, but lower than input. We will use this channel to send calls to LocalFrame::BeforeUnload so that they will be prioritized in the renderer.

All changes are implemented behind a flag so we can experiment with finch. This CL also contains some browser test fixes around timing that popped up when the feature was enabled by default. Also note that this only impacts browser-initiated navigations since renderer-initiated navigations dispatch the before unloads synchronously with the script that triggers the navigation, already effectively running as soon as they can.

[1] https://docs.google.com/document/d/1srzGhb1eCS66mCl2y8CqjN2o3iTPXI2WfySIKoXZQKo/edit?usp=sharing

Change-Id: Ia131bb65087f2612a1dd162f11cafe53862d787b
Bug: 1042118
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2173637
Commit-Queue: Katie Dillon <kdillon@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Scott Haseley <shaseley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785556}

content/browser/frame_host/render_frame_host_impl.cc

@@ -6472,6 +6472,11 @@ void RenderFrameHostImpl::SetUpMojoIfNeeded() {
       remote_interfaces.InitWithNewPipeAndPassReceiver());
   remote_interfaces_.reset(new service_manager::InterfaceProvider);
   remote_interfaces_->Bind(std::move(remote_interfaces));
+
+  // Called to bind the receiver for this interface to the local frame. We need
+  // to eagarly bind here because binding happens at normal priority on the main
+  // thread and future calls to this interface need to be high priority.
+  GetHighPriorityLocalFrame();
 }
 
 void RenderFrameHostImpl::InvalidateMojoConnection() {
@@ -6480,6 +6485,7 @@ void RenderFrameHostImpl::InvalidateMojoConnection() {
   frame_host_associated_receiver_.reset();
   local_frame_.reset();
   local_main_frame_.reset();
+  high_priority_local_frame_.reset();
   navigation_control_.reset();
   find_in_page_.reset();
   render_accessibility_.reset();
@@ -6597,6 +6603,15 @@ RenderFrameHostImpl::GetAssociatedLocalMainFrame() {
   return local_main_frame_;
 }
 
+const mojo::Remote<blink::mojom::HighPriorityLocalFrame>&
+RenderFrameHostImpl::GetHighPriorityLocalFrame() {
+  if (!high_priority_local_frame_.is_bound()) {
+    GetRemoteInterfaces()->GetInterface(
+        high_priority_local_frame_.BindNewPipeAndPassReceiver());
+  }
+  return high_priority_local_frame_;
+}
+
 void RenderFrameHostImpl::ResetLoadingState() {
   if (is_loading()) {
     // When pending deletion, just set the loading state to not loading.
@@ -8553,8 +8568,15 @@ void RenderFrameHostImpl::SendBeforeUnload(
             renderer_before_unload_start_time, renderer_before_unload_end_time);
       },
       rfh);
-  rfh->GetAssociatedLocalFrame()->BeforeUnload(
-      is_reload, std::move(before_unload_closure));
+  // Experiment to run beforeunload handlers at a higher priority in the
+  // renderer. See crubug.com/1042118.
+  if (base::FeatureList::IsEnabled(features::kHighPriorityBeforeUnload)) {
+    rfh->GetHighPriorityLocalFrame()->DispatchBeforeUnload(
+        is_reload, std::move(before_unload_closure));
+  } else {
+    rfh->GetAssociatedLocalFrame()->BeforeUnload(
+        is_reload, std::move(before_unload_closure));
+  }
 }
 
 void RenderFrameHostImpl::AddServiceWorkerContainerHost(

content/browser/frame_host/render_frame_host_impl.h

@@ -971,6 +971,15 @@ class CONTENT_EXPORT RenderFrameHostImpl
   const mojo::AssociatedRemote<blink::mojom::LocalMainFrame>&
   GetAssociatedLocalMainFrame();
 
+  // Returns remote to blink::mojom::HighPriorityLocalFrame Mojo interface. Note
+  // this interface is highly experimental and is being tested to address
+  // crbug.com/1042118. It is not an associated interface and may be actively
+  // reordered. GetAssociatedLocalFrame() should be used in most cases and any
+  // additional use cases of this interface should probably consider discussing
+  // with navigation-dev@chromium.org first.
+  const mojo::Remote<blink::mojom::HighPriorityLocalFrame>&
+  GetHighPriorityLocalFrame();
+
   // Resets the loading state. Following this call, the RenderFrameHost will be
   // in a non-loading state.
   void ResetLoadingState();
@@ -2626,6 +2635,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // remote will be valid when the frame is the active main frame.
   mojo::AssociatedRemote<blink::mojom::LocalMainFrame> local_main_frame_;
 
+  // Holder of Mojo connection with the HighPriorityLocalFrame in blink.
+  mojo::Remote<blink::mojom::HighPriorityLocalFrame> high_priority_local_frame_;
+
   // Holds a NavigationRequest when it's about to commit, ie. after
   // OnCrossDocumentCommitProcessed has returned a positive answer for this
   // NavigationRequest but before receiving DidCommitProvisionalLoad. This

content/public/common/content_features.cc

@@ -451,6 +451,11 @@ const base::Feature kProcessSharingWithDefaultSiteInstances{
 const base::Feature kProcessSharingWithStrictSiteInstances{
     "ProcessSharingWithStrictSiteInstances", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Tells the RenderFrameHost to send beforeunload messages on a different
+// local frame interface which will handle the messages at a higher priority.
+const base::Feature kHighPriorityBeforeUnload{
+    "HighPriorityBeforeUnload", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Under this flag bootstrap (aka startup) tasks will be prioritized. This flag
 // is used by various modules to determine whether special scheduling
 // arrangements need to be made to prioritize certain tasks.

content/public/common/content_features.h

@@ -99,6 +99,7 @@ CONTENT_EXPORT extern const base::Feature kPermissionsPolicyHeader;
 CONTENT_EXPORT extern const base::Feature kPepper3DImageChromium;
 CONTENT_EXPORT extern const base::Feature kPepperCrossOriginRedirectRestriction;
 CONTENT_EXPORT extern const base::Feature kPreferCompositingToLCDText;
+CONTENT_EXPORT extern const base::Feature kHighPriorityBeforeUnload;
 CONTENT_EXPORT extern const base::Feature kPrioritizeBootstrapTasks;
 CONTENT_EXPORT extern const base::Feature kProactivelySwapBrowsingInstance;
 CONTENT_EXPORT extern const base::Feature

third_party/blink/public/mojom/frame/frame.mojom

@@ -578,6 +578,26 @@ interface LocalFrame {
   GetSavableResourceLinks() => (GetSavableResourceLinksReply? reply);
 };
 
+// Also implemented in Blink, this interface defines frame-specific methods
+// that will be invoked from the browser process but processed at a higher
+// priority than methods invoked on the LocalFrame interface.
+//
+// Note this interface does not use legacy IPC channels and as such there are
+// no ordering guarantees for messages sent on this interface. This interface is
+// for experimental purposes.
+interface HighPriorityLocalFrame {
+
+  // Instructs the frame to invoke the beforeunload event handler.
+  //
+  // The closure callback is invoked to acknowledge the browser that
+  // the beforeunload event is handled. |proceed| matches the return value
+  // of the frame's beforeunload handler: true if the user decided to proceed
+  // with leaving the page.
+  DispatchBeforeUnload(bool is_reload)
+      => (bool proceed, mojo_base.mojom.TimeTicks before_unload_start_time,
+          mojo_base.mojom.TimeTicks before_unload_end_time);
+};
+
 // Implemented in Browser, this interface defines frame-specific methods that
 // will be invoked from the render process (e.g. blink::RemoteFrame).
 //

third_party/blink/public/platform/task_type.h

@@ -237,6 +237,9 @@ enum class TaskType : unsigned char {
   // Tasks used for find-in-page.
   kInternalFindInPage = 70,
 
+  // Tasks that come in on the HighPriorityLocalFrame interface.
+  kInternalHighPriorityLocalFrame = 71,
+
   ///////////////////////////////////////
   // The following task types are only for thread-local queues.
   ///////////////////////////////////////
@@ -261,7 +264,7 @@ enum class TaskType : unsigned char {
   kWorkerThreadTaskQueueV8 = 47,
   kWorkerThreadTaskQueueCompositor = 48,
 
-  kCount = 71,
+  kCount = 72,
 };
 
 }  // namespace blink

third_party/blink/renderer/core/frame/local_frame.cc

@@ -334,7 +334,10 @@ void LocalFrame::Init() {
           GetTaskRunner(blink::TaskType::kInternalDefault)));
   GetInterfaceRegistry()->AddAssociatedInterface(WTF::BindRepeating(
       &LocalFrame::BindToReceiver, WrapWeakPersistent(this)));
-
+  GetInterfaceRegistry()->AddInterface(
+      WTF::BindRepeating(&LocalFrame::BindToHighPriorityReceiver,
+                         WrapWeakPersistent(this)),
+      GetTaskRunner(blink::TaskType::kInternalHighPriorityLocalFrame));
   loader_.Init();
 }
 
@@ -2626,6 +2629,11 @@ void LocalFrame::BeforeUnload(bool is_reload, BeforeUnloadCallback callback) {
                           before_unload_end_time);
 }
 
+void LocalFrame::DispatchBeforeUnload(bool is_reload,
+                                      BeforeUnloadCallback callback) {
+  BeforeUnload(is_reload, std::move(callback));
+}
+
 void LocalFrame::MediaPlayerActionAtViewportPoint(
     const IntPoint& viewport_position,
     const blink::mojom::blink::MediaPlayerActionType type,
@@ -2944,6 +2952,13 @@ void LocalFrame::BindToMainFrameReceiver(
       frame->GetTaskRunner(blink::TaskType::kInternalDefault));
 }
 
+void LocalFrame::BindToHighPriorityReceiver(
+    mojo::PendingReceiver<mojom::blink::HighPriorityLocalFrame> receiver) {
+  high_priority_frame_receiver_.Bind(
+      std::move(receiver),
+      GetTaskRunner(blink::TaskType::kInternalHighPriorityLocalFrame));
+}
+
 SpellChecker& LocalFrame::GetSpellChecker() const {
   DCHECK(DomWindow());
   return DomWindow()->GetSpellChecker();

third_party/blink/renderer/core/frame/local_frame.h

@@ -135,11 +135,13 @@ class WebURLLoaderFactory;
 
 extern template class CORE_EXTERN_TEMPLATE_EXPORT Supplement<LocalFrame>;
 
-class CORE_EXPORT LocalFrame final : public Frame,
-                                     public FrameScheduler::Delegate,
-                                     public Supplementable<LocalFrame>,
-                                     public mojom::blink::LocalFrame,
-                                     public mojom::blink::LocalMainFrame {
+class CORE_EXPORT LocalFrame final
+    : public Frame,
+      public FrameScheduler::Delegate,
+      public Supplementable<LocalFrame>,
+      public mojom::blink::LocalFrame,
+      public mojom::blink::LocalMainFrame,
+      public mojom::blink::HighPriorityLocalFrame {
   USING_GARBAGE_COLLECTED_MIXIN(LocalFrame);
 
  public:
@@ -563,6 +565,8 @@ class CORE_EXPORT LocalFrame final : public Frame,
   void ReportBlinkFeatureUsage(const Vector<mojom::blink::WebFeature>&) final;
   void RenderFallbackContent() final;
   void BeforeUnload(bool is_reload, BeforeUnloadCallback callback) final;
+  void DispatchBeforeUnload(bool is_reload,
+                            BeforeUnloadCallback callback) final;
   void MediaPlayerActionAt(
       const gfx::Point& window_point,
       blink::mojom::blink::MediaPlayerActionPtr action) final;
@@ -688,6 +692,8 @@ class CORE_EXPORT LocalFrame final : public Frame,
   static void BindToMainFrameReceiver(
       blink::LocalFrame* frame,
       mojo::PendingAssociatedReceiver<mojom::blink::LocalMainFrame> receiver);
+  void BindToHighPriorityReceiver(
+      mojo::PendingReceiver<mojom::blink::HighPriorityLocalFrame> receiver);
 
   std::unique_ptr<FrameScheduler> frame_scheduler_;
 
@@ -811,6 +817,9 @@ class CORE_EXPORT LocalFrame final : public Frame,
                              HeapMojoWrapperMode::kWithoutContextObserver>
       main_frame_receiver_{this, nullptr};
 
+  mojo::Receiver<mojom::blink::HighPriorityLocalFrame>
+      high_priority_frame_receiver_{this};
+
   // Variable to control burst of download requests.
   int num_burst_download_requests_ = 0;
   base::TimeTicks burst_download_start_time_;

third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc

@@ -381,7 +381,7 @@ QueueTraits FrameSchedulerImpl::CreateQueueTraitsForTaskType(TaskType type) {
   switch (type) {
     case TaskType::kInternalContentCapture:
       return ThrottleableTaskQueueTraits().SetPrioritisationType(
-            QueueTraits::PrioritisationType::kBestEffort);
+          QueueTraits::PrioritisationType::kBestEffort);
     case TaskType::kJavascriptTimer:
       return ThrottleableTaskQueueTraits().SetPrioritisationType(
           QueueTraits::PrioritisationType::kJavaScriptTimer);
@@ -436,9 +436,12 @@ QueueTraits FrameSchedulerImpl::CreateQueueTraitsForTaskType(TaskType type) {
       return PausableTaskQueueTraits();
     case TaskType::kInternalFindInPage:
       return FindInPageTaskQueueTraits();
+    case TaskType::kInternalHighPriorityLocalFrame:
+      return QueueTraits().SetPrioritisationType(
+          QueueTraits::PrioritisationType::kHighPriorityLocalFrame);
     case TaskType::kInternalContinueScriptLoading:
       return PausableTaskQueueTraits().SetPrioritisationType(
-            QueueTraits::PrioritisationType::kVeryHigh);
+          QueueTraits::PrioritisationType::kVeryHigh);
     case TaskType::kDatabaseAccess:
       if (base::FeatureList::IsEnabled(kHighPriorityDatabaseTaskType)) {
         return PausableTaskQueueTraits().SetPrioritisationType(
@@ -530,8 +533,8 @@ FrameSchedulerImpl::CreateResourceLoadingTaskRunnerHandleImpl() {
     return ResourceLoadingTaskRunnerHandleImpl::WrapTaskRunner(task_queue);
   }
 
-  return ResourceLoadingTaskRunnerHandleImpl::WrapTaskRunner(GetTaskQueue
-      (TaskType::kNetworkingWithURLLoaderAnnotation));
+  return ResourceLoadingTaskRunnerHandleImpl::WrapTaskRunner(
+      GetTaskQueue(TaskType::kNetworkingWithURLLoaderAnnotation));
 }
 
 void FrameSchedulerImpl::DidChangeResourceLoadingPriority(
@@ -1075,14 +1078,13 @@ TaskQueue::QueuePriority FrameSchedulerImpl::ComputePriority(
 
   if (task_queue->GetPrioritisationType() ==
       MainThreadTaskQueue::QueueTraits::PrioritisationType::kLoadingControl) {
-    return main_thread_scheduler_
-                   ->should_prioritize_loading_with_compositing()
+    return main_thread_scheduler_->should_prioritize_loading_with_compositing()
                ? TaskQueue::QueuePriority::kVeryHighPriority
                : TaskQueue::QueuePriority::kHighPriority;
   }
 
   if (task_queue->GetPrioritisationType() ==
-      MainThreadTaskQueue::QueueTraits::PrioritisationType::kLoading &&
+          MainThreadTaskQueue::QueueTraits::PrioritisationType::kLoading &&
       main_thread_scheduler_->should_prioritize_loading_with_compositing()) {
     return main_thread_scheduler_->compositor_priority();
   }
@@ -1092,6 +1094,12 @@ TaskQueue::QueuePriority FrameSchedulerImpl::ComputePriority(
     return main_thread_scheduler_->find_in_page_priority();
   }
 
+  if (task_queue->GetPrioritisationType() ==
+      MainThreadTaskQueue::QueueTraits::PrioritisationType::
+          kHighPriorityLocalFrame) {
+    return TaskQueue::QueuePriority::kHighestPriority;
+  }
+
   if (task_queue->GetPrioritisationType() ==
       MainThreadTaskQueue::QueueTraits::PrioritisationType::
           kExperimentalDatabase) {
@@ -1242,8 +1250,7 @@ FrameSchedulerImpl::DeferrableTaskQueueTraits() {
 }
 
 // static
-MainThreadTaskQueue::QueueTraits
-FrameSchedulerImpl::PausableTaskQueueTraits() {
+MainThreadTaskQueue::QueueTraits FrameSchedulerImpl::PausableTaskQueueTraits() {
   return QueueTraits()
       .SetCanBeFrozen(base::FeatureList::IsEnabled(
           blink::features::kStopNonTimersInBackground))
@@ -1284,14 +1291,12 @@ void FrameSchedulerImpl::SetPreemptedForCooperativeScheduling(
   UpdatePolicy();
 }
 
-MainThreadTaskQueue::QueueTraits
-FrameSchedulerImpl::LoadingTaskQueueTraits() {
+MainThreadTaskQueue::QueueTraits FrameSchedulerImpl::LoadingTaskQueueTraits() {
   return QueueTraits()
       .SetCanBePaused(true)
       .SetCanBeFrozen(true)
       .SetCanBeDeferred(true)
-      .SetPrioritisationType(
-          QueueTraits::PrioritisationType::kLoading);
+      .SetPrioritisationType(QueueTraits::PrioritisationType::kLoading);
 }
 
 MainThreadTaskQueue::QueueTraits
@@ -1300,8 +1305,7 @@ FrameSchedulerImpl::LoadingControlTaskQueueTraits() {
       .SetCanBePaused(true)
       .SetCanBeFrozen(true)
       .SetCanBeDeferred(true)
-      .SetPrioritisationType(
-          QueueTraits::PrioritisationType::kLoadingControl);
+      .SetPrioritisationType(QueueTraits::PrioritisationType::kLoadingControl);
 }
 
 MainThreadTaskQueue::QueueTraits

third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl_unittest.cc

@@ -126,10 +126,11 @@ constexpr TaskType kAllFrameTaskTypes[] = {
     TaskType::kInternalFrameLifecycleControl,
     TaskType::kInternalTranslation,
     TaskType::kInternalInspector,
-    TaskType::kInternalNavigationAssociatedUnfreezable};
+    TaskType::kInternalNavigationAssociatedUnfreezable,
+    TaskType::kInternalHighPriorityLocalFrame};
 
 static_assert(
-    static_cast<int>(TaskType::kCount) == 71,
+    static_cast<int>(TaskType::kCount) == 72,
     "When adding a TaskType, make sure that kAllFrameTaskTypes is updated.");
 
 void AppendToVectorTestTask(Vector<String>* vector, String value) {
@@ -301,13 +302,12 @@ class FrameSchedulerImplTest : public testing::Test {
 
   scoped_refptr<MainThreadTaskQueue> GetTaskQueue(
       MainThreadTaskQueue::QueueTraits queue_traits) {
-    return frame_scheduler_->FrameTaskQueueControllerForTest()
-        ->GetTaskQueue(queue_traits);
+    return frame_scheduler_->FrameTaskQueueControllerForTest()->GetTaskQueue(
+        queue_traits);
   }
 
   scoped_refptr<TaskQueue> ThrottleableTaskQueue() {
-    return GetTaskQueue(
-        FrameSchedulerImpl::ThrottleableTaskQueueTraits());
+    return GetTaskQueue(FrameSchedulerImpl::ThrottleableTaskQueueTraits());
   }
 
   scoped_refptr<TaskQueue> JavaScriptTimerTaskQueue() {
@@ -317,13 +317,11 @@ class FrameSchedulerImplTest : public testing::Test {
   }
 
   scoped_refptr<TaskQueue> LoadingTaskQueue() {
-    return GetTaskQueue(
-        FrameSchedulerImpl::LoadingTaskQueueTraits());
+    return GetTaskQueue(FrameSchedulerImpl::LoadingTaskQueueTraits());
   }
 
   scoped_refptr<TaskQueue> LoadingControlTaskQueue() {
-    return GetTaskQueue(
-        FrameSchedulerImpl::LoadingControlTaskQueueTraits());
+    return GetTaskQueue(FrameSchedulerImpl::LoadingControlTaskQueueTraits());
   }
 
   scoped_refptr<TaskQueue> DeferrableTaskQueue() {
@@ -339,8 +337,7 @@ class FrameSchedulerImplTest : public testing::Test {
   }
 
   scoped_refptr<TaskQueue> ForegroundOnlyTaskQueue() {
-    return GetTaskQueue(
-        FrameSchedulerImpl::ForegroundOnlyTaskQueueTraits());
+    return GetTaskQueue(FrameSchedulerImpl::ForegroundOnlyTaskQueueTraits());
   }
 
   scoped_refptr<MainThreadTaskQueue> GetTaskQueue(TaskType type) {