Reland "[Paint Preview] Pre-warm compositor"

This is a reland of 3480cbb1

I had to revert the original due to small time regressions in time to
navigation commit time and a larger regression in memory.

I've deferred the initialization of the warm compositor to after we know
we likely have something to show to avoid this regression as much as
possible. This improves things by O(10 ms) still, but is not as
significant of an improvement as before.

Original change's description:
> [Paint Preview] Pre-warm compositor
>
> This CL adds the ability to pre-warm a compositor process for use by
> paint preview show-on-startup.
>
> 1. The compositor is started when native is initialized if the
>    experiment is enabled.
> 2. Depending on the situation the compositor is then used or killed.
>    a. Compositor is used: disconnect handler is updated and it is used
>       as if there was a compositor already.
>    b. Compositor is not used: the process is killed immediately.
>
> This moves initialization of the compositor service up to O(100 ms)
> earlier (in local tests).
>
> Bug: 1126180
> Change-Id: I1e6f627403e8c020b6551c510ad908bba0fc24a2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2398562
> Reviewed-by: Mehran Mahmoudi <mahmoudi@chromium.org>
> Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#805317}

Bug: 1126180
Change-Id: I7f7764bb080622b990dfadb7a99957cc9d992668
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2403580Reviewed-by: Mehran Mahmoudi <mahmoudi@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806675}

chrome/browser/paint_preview/BUILD.gn

@@ -22,6 +22,8 @@ source_set("services") {
   ]
 
   if (is_android) {
+    sources += [ "paint_preview_compositor_utils.cc" ]
+
     deps += [ "//chrome/browser/paint_preview/android:jni_headers" ]
   }
 }

chrome/browser/paint_preview/android/BUILD.gn

@@ -6,6 +6,7 @@ import("//build/config/android/rules.gni")
 
 generate_jni("jni_headers") {
   sources = [
+    "java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewCompositorUtils.java",
     "java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewDemoService.java",
     "java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewDemoServiceFactory.java",
     "java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewTabService.java",
@@ -17,6 +18,7 @@ android_library("java") {
   annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
 
   sources = [
+    "java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewCompositorUtils.java",
     "java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewDemoManager.java",
     "java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewTabHelper.java",
     "java/src/org/chromium/chrome/browser/paint_preview/TabbedPaintPreviewMetricsHelper.java",
@@ -31,6 +33,7 @@ android_library("java") {
     ":java_resources",
     "//base:base_java",
     "//base:jni_java",
+    "//chrome/browser/android/lifecycle:java",
     "//chrome/browser/flags:java",
     "//chrome/browser/tab:java",
     "//chrome/browser/tabmodel:java",

chrome/browser/paint_preview/android/java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewCompositorUtils.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.paint_preview;
+
+import org.chromium.base.annotations.JNINamespace;
+import org.chromium.base.annotations.NativeMethods;
+
+/**
+ * Utilities for warming up a compositor process for paint previews.
+ */
+@JNINamespace("paint_preview")
+public class PaintPreviewCompositorUtils {
+    /**
+     * Warms up the compositor process.
+     */
+    static void warmupCompositor() {
+        PaintPreviewCompositorUtilsJni.get().warmupCompositor();
+    }
+
+    /**
+     * Stops the warm warm compositor process if one exists.
+     */
+    static boolean stopWarmCompositor() {
+        return PaintPreviewCompositorUtilsJni.get().stopWarmCompositor();
+    }
+
+    @NativeMethods
+    interface Natives {
+        void warmupCompositor();
+        boolean stopWarmCompositor();
+    }
+
+    private PaintPreviewCompositorUtils() {}
+}
+
+

chrome/browser/paint_preview/android/java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewDemoManager.java

@@ -74,6 +74,7 @@ public class PaintPreviewDemoManager implements TabViewProvider {
     }
 
     void removePaintPreviewDemo() {
+        PaintPreviewCompositorUtils.stopWarmCompositor();
         if (mTab == null || mPlayerManager == null) {
             return;
         }

chrome/browser/paint_preview/android/java/src/org/chromium/chrome/browser/paint_preview/PaintPreviewTabHelper.java

@@ -40,6 +40,7 @@ public class PaintPreviewTabHelper extends EmptyTabObserver implements UserData
             return;
         }
 
+        PaintPreviewCompositorUtils.warmupCompositor();
         mPaintPreviewDemoManager.showPaintPreviewDemo();
     }
 

chrome/browser/paint_preview/android/java/src/org/chromium/chrome/browser/paint_preview/TabbedPaintPreviewPlayer.java

@@ -154,6 +154,8 @@ public class TabbedPaintPreviewPlayer implements TabViewProvider, UserData {
         mInitializing = hasCapture;
         mMetricsHelper.recordHadCapture(hasCapture);
         if (!hasCapture) return false;
+
+        PaintPreviewCompositorUtils.warmupCompositor();
         mFirstMeaningfulPaintHappened = false;
 
         mPlayerManager = new PlayerManager(mTab.getUrl(), mTab.getContext(),
@@ -187,6 +189,7 @@ public class TabbedPaintPreviewPlayer implements TabViewProvider, UserData {
      * nothing if there is no view showing.
      */
     private void removePaintPreview(@ExitCause int exitCause) {
+        PaintPreviewCompositorUtils.stopWarmCompositor();
         mOnDismissed = null;
         mInitializing = false;
         if (mTab == null || mPlayerManager == null) return;

chrome/browser/paint_preview/paint_preview_compositor_browsertest.cc

@@ -18,6 +18,7 @@
 #include "components/paint_preview/browser/paint_preview_base_service_test_factory.h"
 #include "components/paint_preview/browser/paint_preview_compositor_client_impl.h"
 #include "components/paint_preview/browser/paint_preview_compositor_service_impl.h"
+#include "components/paint_preview/browser/warm_compositor.h"
 #include "components/paint_preview/public/paint_preview_compositor_client.h"
 #include "components/paint_preview/public/paint_preview_compositor_service.h"
 #include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h"
@@ -198,4 +199,28 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewCompositorBrowserTest,
   EXPECT_FALSE(IsBoundAndConnected(compositor.get()));
 }
 
+IN_PROC_BROWSER_TEST_F(PaintPreviewCompositorBrowserTest, PreWarmCompositor) {
+  // Start with warm compositor.
+  WarmCompositor* warm_compositor = WarmCompositor::GetInstance();
+  warm_compositor->WarmupCompositor();
+  auto compositor_service = ToCompositorServiceImpl(
+      warm_compositor->GetOrStartCompositorService(base::DoNothing()));
+  EXPECT_FALSE(warm_compositor->StopCompositor());
+  EXPECT_NE(compositor_service, nullptr);
+  compositor_service.reset();
+  EXPECT_EQ(compositor_service, nullptr);
+
+  // Start and stop.
+  warm_compositor->WarmupCompositor();
+  EXPECT_TRUE(warm_compositor->StopCompositor());
+
+  // Verify it is still possible to start if the compositor was prematurely
+  // stopped.
+  compositor_service = ToCompositorServiceImpl(
+      warm_compositor->GetOrStartCompositorService(base::DoNothing()));
+  EXPECT_NE(compositor_service, nullptr);
+  compositor_service.reset();
+  EXPECT_EQ(compositor_service, nullptr);
+}
+
 }  // namespace paint_preview

chrome/browser/paint_preview/paint_preview_compositor_utils.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/paint_preview/android/jni_headers/PaintPreviewCompositorUtils_jni.h"
+#include "components/paint_preview/browser/warm_compositor.h"
+
+namespace paint_preview {
+
+void JNI_PaintPreviewCompositorUtils_WarmupCompositor(JNIEnv* env) {
+  WarmCompositor::GetInstance()->WarmupCompositor();
+}
+
+jboolean JNI_PaintPreviewCompositorUtils_StopWarmCompositor(JNIEnv* env) {
+  return static_cast<jboolean>(WarmCompositor::GetInstance()->StopCompositor());
+}
+
+}  // namespace paint_preview

chrome/browser/paint_preview/services/paint_preview_tab_service.cc

@@ -13,6 +13,7 @@
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "components/paint_preview/browser/file_manager.h"
+#include "components/paint_preview/browser/warm_compositor.h"
 #include "ui/gfx/geometry/rect.h"
 
 #if defined(OS_ANDROID)

components/paint_preview/browser/BUILD.gn

@@ -26,6 +26,8 @@ source_set("browser") {
     "paint_preview_compositor_service_impl.h",
     "paint_preview_policy.h",
     "service_sandbox_type.h",
+    "warm_compositor.cc",
+    "warm_compositor.h",
   ]
 
   deps = [

components/paint_preview/browser/paint_preview_compositor_service_impl.cc

@@ -101,6 +101,12 @@ bool PaintPreviewCompositorServiceImpl::HasActiveClients() const {
   return !active_clients_.empty();
 }
 
+void PaintPreviewCompositorServiceImpl::SetDisconnectHandler(
+    base::OnceClosure disconnect_handler) {
+  DCHECK(default_task_runner_->RunsTasksInCurrentSequence());
+  user_disconnect_closure_ = std::move(disconnect_handler);
+}
+
 void PaintPreviewCompositorServiceImpl::MarkCompositorAsDeleted(
     const base::UnguessableToken& token) {
   DCHECK(default_task_runner_->RunsTasksInCurrentSequence());
@@ -121,7 +127,8 @@ void PaintPreviewCompositorServiceImpl::OnCompositorCreated(
 
 void PaintPreviewCompositorServiceImpl::DisconnectHandler() {
   DCHECK(default_task_runner_->RunsTasksInCurrentSequence());
-  std::move(user_disconnect_closure_).Run();
+  if (user_disconnect_closure_)
+    std::move(user_disconnect_closure_).Run();
   compositor_service_.reset();
 }
 

components/paint_preview/browser/paint_preview_compositor_service_impl.h

@@ -38,6 +38,9 @@ class PaintPreviewCompositorServiceImpl : public PaintPreviewCompositorService {
   CreateCompositor(base::OnceClosure connected_closure) override;
 
   bool HasActiveClients() const override;
+  // NOTE: this is set by the constructor. However, in some cases it may need to
+  // be changed.
+  void SetDisconnectHandler(base::OnceClosure disconnect_handler) override;
 
   // Marks the compositor associated with |token| as deleted in the
   // |active_clients_| set.

components/paint_preview/browser/warm_compositor.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/paint_preview/browser/warm_compositor.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/memory/singleton.h"
+#include "components/paint_preview/browser/compositor_utils.h"
+
+namespace paint_preview {
+
+WarmCompositor::WarmCompositor()
+    : compositor_service_(nullptr, base::OnTaskRunnerDeleter(nullptr)) {}
+WarmCompositor::~WarmCompositor() = default;
+
+// static
+WarmCompositor* WarmCompositor::GetInstance() {
+  return base::Singleton<WarmCompositor,
+                         base::LeakySingletonTraits<WarmCompositor>>::get();
+}
+
+void WarmCompositor::WarmupCompositor() {
+  if (compositor_service_)
+    return;
+
+  compositor_service_ = StartCompositorService(base::BindOnce(
+      &WarmCompositor::OnDisconnect, weak_ptr_factory_.GetWeakPtr()));
+}
+
+bool WarmCompositor::StopCompositor() {
+  if (!compositor_service_)
+    return false;
+
+  compositor_service_.reset();
+  return true;
+}
+
+std::unique_ptr<PaintPreviewCompositorService, base::OnTaskRunnerDeleter>
+WarmCompositor::GetOrStartCompositorService(
+    base::OnceClosure disconnect_handler) {
+  if (!compositor_service_)
+    return StartCompositorService(std::move(disconnect_handler));
+
+  compositor_service_->SetDisconnectHandler(std::move(disconnect_handler));
+  return std::move(compositor_service_);
+}
+
+void WarmCompositor::OnDisconnect() {
+  compositor_service_.reset();
+}
+
+}  // namespace paint_preview

components/paint_preview/browser/warm_compositor.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_PAINT_PREVIEW_BROWSER_WARM_COMPOSITOR_H_
+#define COMPONENTS_PAINT_PREVIEW_BROWSER_WARM_COMPOSITOR_H_
+
+#include <memory>
+
+#include "base/task_runner.h"
+#include "components/paint_preview/public/paint_preview_compositor_service.h"
+
+namespace base {
+template <typename T>
+struct DefaultSingletonTraits;
+}
+
+namespace paint_preview {
+
+// A class that can hold a pre-warmed compositor service for use.
+class WarmCompositor {
+ public:
+  ~WarmCompositor();
+
+  WarmCompositor(const WarmCompositor&) = delete;
+  WarmCompositor& operator=(const WarmCompositor&) = delete;
+
+  static WarmCompositor* GetInstance();
+
+  // Warms up the compositor service.
+  void WarmupCompositor();
+
+  // Releases the warmed compositor service if there is one. Returns true if a
+  // compositor was released.
+  bool StopCompositor();
+
+  // Passes the pre-warmed compositor to the caller if one is present.
+  // Otherwise starts a new compositor.
+  std::unique_ptr<PaintPreviewCompositorService, base::OnTaskRunnerDeleter>
+  GetOrStartCompositorService(base::OnceClosure disconnect_handler);
+
+ private:
+  WarmCompositor();
+  friend struct base::DefaultSingletonTraits<WarmCompositor>;
+
+  void OnDisconnect();
+
+  std::unique_ptr<PaintPreviewCompositorService, base::OnTaskRunnerDeleter>
+      compositor_service_;
+
+  base::WeakPtrFactory<WarmCompositor> weak_ptr_factory_{this};
+};
+
+}  // namespace paint_preview
+
+#endif  // COMPONENTS_PAINT_PREVIEW_BROWSER_WARM_COMPOSITOR_H_

components/paint_preview/player/player_compositor_delegate.cc

@@ -21,8 +21,8 @@
 #include "base/trace_event/common/trace_event_common.h"
 #include "base/trace_event/trace_event.h"
 #include "base/unguessable_token.h"
-#include "components/paint_preview/browser/compositor_utils.h"
 #include "components/paint_preview/browser/paint_preview_base_service.h"
+#include "components/paint_preview/browser/warm_compositor.h"
 #include "components/paint_preview/common/proto/paint_preview.pb.h"
 #include "components/paint_preview/common/recording_map.h"
 #include "components/paint_preview/common/serialized_recording.h"
@@ -120,9 +120,10 @@ PlayerCompositorDelegate::PlayerCompositorDelegate(
   TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("paint_preview",
                                     "PlayerCompositorDelegate CreateCompositor",
                                     TRACE_ID_LOCAL(this));
-  paint_preview_compositor_service_ = StartCompositorService(
-      base::BindOnce(&PlayerCompositorDelegate::OnCompositorServiceDisconnected,
-                     weak_factory_.GetWeakPtr()));
+  paint_preview_compositor_service_ =
+      WarmCompositor::GetInstance()->GetOrStartCompositorService(base::BindOnce(
+          &PlayerCompositorDelegate::OnCompositorServiceDisconnected,
+          weak_factory_.GetWeakPtr()));
 
   paint_preview_compositor_client_ =
       paint_preview_compositor_service_->CreateCompositor(

components/paint_preview/public/paint_preview_compositor_service.h

@@ -35,6 +35,9 @@ class PaintPreviewCompositorService {
   // check if killing this service is safe (i.e. won't drop messages).
   virtual bool HasActiveClients() const = 0;
 
+  // Sets the disconnect handler for this service.
+  virtual void SetDisconnectHandler(base::OnceClosure disconnect_handler) = 0;
+
   PaintPreviewCompositorService(const PaintPreviewCompositorService&) = delete;
   PaintPreviewCompositorService& operator=(
       const PaintPreviewCompositorService&) = delete;