Revert "[Paint Preview] Pre-warm compositor"

This reverts commit 3480cbb1.

Reason for revert: This caused some performance/memory regressions that
are far larger than expected.

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}

TBR=mahmoudi@chromium.org,ckitagawa@chromium.org

Change-Id: I7cd5067906f3833f9d927f9b3be1fe2600dbb741
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1126180, 1126665, 1126734, 1126728, 1126648
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2403389Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805718}

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

@@ -13,7 +13,6 @@ import org.chromium.chrome.browser.metrics.PageLoadMetrics;
 import org.chromium.chrome.browser.metrics.UmaUtils;
 import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
 import org.chromium.chrome.browser.paint_preview.services.PaintPreviewTabServiceFactory;
-import org.chromium.chrome.browser.paint_preview.services.PaintPreviewTabServiceNativeInitObserver;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.EmptyTabModelSelectorObserver;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
@@ -50,9 +49,6 @@ public class PaintPreviewHelper {
         }
         sActivityCreationTimeMs = activity.getOnCreateTimestampMs();
 
-        activity.getLifecycleDispatcher().register(
-                new PaintPreviewTabServiceNativeInitObserver(activity.getLifecycleDispatcher()));
-
         // TODO(crbug/1074428): verify this doesn't cause a memory leak if the user exits Chrome
         // prior to onTabStateInitialized being called.
         tabModelSelector.addObserver(new EmptyTabModelSelectorObserver() {

chrome/browser/paint_preview/android/BUILD.gn

@@ -25,14 +25,12 @@ android_library("java") {
     "java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewDemoServiceFactory.java",
     "java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewTabService.java",
     "java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewTabServiceFactory.java",
-    "java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewTabServiceNativeInitObserver.java",
   ]
 
   deps = [
     ":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/TabbedPaintPreviewPlayer.java

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

chrome/browser/paint_preview/android/java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewTabService.java

@@ -115,17 +115,6 @@ public class PaintPreviewTabService implements NativePaintPreviewServiceProvider
                 mNativePaintPreviewTabService, tabId);
     }
 
-    /**
-     * Stops the pre-warmed compositor service if it isn't needed.
-     * @return Whether a pre-warmed compositor service was actually stopped.
-     */
-    public boolean stopWarmCompositor() {
-        if (mNativePaintPreviewTabService == 0) return false;
-
-        return PaintPreviewTabServiceJni.get().stopWarmCompositorAndroid(
-                mNativePaintPreviewTabService);
-    }
-
     /**
      * Should be called when all tabs are restored. Registers a {@link TabModelSelectorTabObserver}
      * for the regular to capture and delete paint previews as needed. Audits restored tabs to
@@ -236,6 +225,5 @@ public class PaintPreviewTabService implements NativePaintPreviewServiceProvider
         void auditArtifactsAndroid(long nativePaintPreviewTabService, int[] activeTabIds);
         boolean isCacheInitializedAndroid(long nativePaintPreviewTabService);
         String getPathAndroid(long nativePaintPreviewTabService);
-        boolean stopWarmCompositorAndroid(long nativePaintPreviewTabService);
     }
 }

chrome/browser/paint_preview/android/java/src/org/chromium/chrome/browser/paint_preview/services/PaintPreviewTabServiceNativeInitObserver.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.services;
-
-import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
-import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
-
-/**
- * Watches for native init to pre-warm the compositor.
- */
-public class PaintPreviewTabServiceNativeInitObserver implements NativeInitObserver {
-    private ActivityLifecycleDispatcher mActvityLifecycleDispatcher;
-
-    public PaintPreviewTabServiceNativeInitObserver(
-            ActivityLifecycleDispatcher activityLifecycleDispatcher) {
-        mActvityLifecycleDispatcher = activityLifecycleDispatcher;
-    }
-
-    @Override
-    public void onFinishNativeInitialization() {
-        // Warms-up the service and prepares the compositor service.
-        PaintPreviewTabServiceFactory.getServiceInstance();
-        mActvityLifecycleDispatcher.unregister(this);
-    }
-}

chrome/browser/paint_preview/paint_preview_compositor_browsertest.cc

@@ -18,7 +18,6 @@
 #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"
@@ -199,28 +198,4 @@ 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/services/paint_preview_tab_service.cc

@@ -13,7 +13,6 @@
 #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)
@@ -56,16 +55,12 @@ PaintPreviewTabService::PaintPreviewTabService(
     const base::FilePath& profile_dir,
     base::StringPiece ascii_feature_name,
     std::unique_ptr<PaintPreviewPolicy> policy,
-    bool is_off_the_record,
-    bool prewarm_compositor)
+    bool is_off_the_record)
     : PaintPreviewBaseService(profile_dir,
                               ascii_feature_name,
                               std::move(policy),
                               is_off_the_record),
       cache_ready_(false) {
-  if (prewarm_compositor)
-    WarmCompositor::GetInstance()->WarmupCompositor();
-
   GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE, base::BindOnce(&FileManager::ListUsedKeys, GetFileManager()),
       base::BindOnce(&PaintPreviewTabService::InitializeCache,
@@ -160,11 +155,6 @@ void PaintPreviewTabService::AuditArtifacts(
                      weak_ptr_factory_.GetWeakPtr(), active_tab_ids));
 }
 
-bool PaintPreviewTabService::StopWarmCompositor() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return WarmCompositor::GetInstance()->StopCompositor();
-}
-
 void PaintPreviewTabService::GetCapturedPaintPreviewProto(
     const DirectoryKey& key,
     base::Optional<base::TimeDelta> expiry_horizon,
@@ -219,10 +209,6 @@ PaintPreviewTabService::GetPathAndroid(JNIEnv* env) {
   return base::android::ConvertUTF8ToJavaString(
       env, GetFileManager()->GetPath().AsUTF8Unsafe());
 }
-
-jboolean PaintPreviewTabService::StopWarmCompositorAndroid(JNIEnv* env) {
-  return static_cast<jboolean>(StopWarmCompositor());
-}
 #endif  // defined(OS_ANDROID)
 
 void PaintPreviewTabService::InitializeCache(

chrome/browser/paint_preview/services/paint_preview_tab_service.h

@@ -41,8 +41,7 @@ class PaintPreviewTabService : public PaintPreviewBaseService {
   PaintPreviewTabService(const base::FilePath& profile_dir,
                          base::StringPiece ascii_feature_name,
                          std::unique_ptr<PaintPreviewPolicy> policy,
-                         bool is_off_the_record,
-                         bool prewarm_compositor = true);
+                         bool is_off_the_record);
   ~PaintPreviewTabService() override;
 
   enum Status {
@@ -79,9 +78,6 @@ class PaintPreviewTabService : public PaintPreviewBaseService {
   // occurred.
   void AuditArtifacts(const std::vector<int>& active_tab_ids);
 
-  // Stops the pre-warmed compositor service.
-  bool StopWarmCompositor();
-
   // Override for GetCapturedPaintPreviewProto. Defaults expiry horizon to 72
   // hrs if not specified.
   void GetCapturedPaintPreviewProto(
@@ -104,7 +100,6 @@ class PaintPreviewTabService : public PaintPreviewBaseService {
       const base::android::JavaParamRef<jintArray>& j_tab_ids);
   jboolean IsCacheInitializedAndroid(JNIEnv* env);
   base::android::ScopedJavaLocalRef<jstring> GetPathAndroid(JNIEnv* env);
-  jboolean StopWarmCompositorAndroid(JNIEnv* env);
 
   base::android::ScopedJavaGlobalRef<jobject> GetJavaRef() { return java_ref_; }
 #endif  // defined(OS_ANDROID)

chrome/browser/paint_preview/services/paint_preview_tab_service_unittest.cc

@@ -85,7 +85,7 @@ class PaintPreviewTabServiceTest : public ChromeRenderViewHostTestHarness {
     ChromeRenderViewHostTestHarness::SetUp();
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
     service_ = std::make_unique<PaintPreviewTabService>(
-        temp_dir_.GetPath(), kFeatureName, nullptr, false, false);
+        temp_dir_.GetPath(), kFeatureName, nullptr, false);
     task_environment()->RunUntilIdle();
     EXPECT_TRUE(service_->CacheInitialized());
   }
@@ -119,7 +119,7 @@ class PaintPreviewTabServiceTest : public ChromeRenderViewHostTestHarness {
     }
 
     return std::make_unique<PaintPreviewTabService>(GetPath(), kFeatureName,
-                                                    nullptr, false, false);
+                                                    nullptr, false);
   }
 
  private:

components/paint_preview/browser/BUILD.gn

@@ -26,8 +26,6 @@ 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,12 +101,6 @@ 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());
@@ -127,8 +121,7 @@ void PaintPreviewCompositorServiceImpl::OnCompositorCreated(
 
 void PaintPreviewCompositorServiceImpl::DisconnectHandler() {
   DCHECK(default_task_runner_->RunsTasksInCurrentSequence());
-  if (user_disconnect_closure_)
-    std::move(user_disconnect_closure_).Run();
+  std::move(user_disconnect_closure_).Run();
   compositor_service_.reset();
 }
 

components/paint_preview/browser/paint_preview_compositor_service_impl.h

@@ -38,9 +38,6 @@ 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,10 +120,9 @@ PlayerCompositorDelegate::PlayerCompositorDelegate(
   TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("paint_preview",
                                     "PlayerCompositorDelegate CreateCompositor",
                                     TRACE_ID_LOCAL(this));
-  paint_preview_compositor_service_ =
-      WarmCompositor::GetInstance()->GetOrStartCompositorService(base::BindOnce(
-          &PlayerCompositorDelegate::OnCompositorServiceDisconnected,
-          weak_factory_.GetWeakPtr()));
+  paint_preview_compositor_service_ = StartCompositorService(
+      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,9 +35,6 @@ 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;