Commit 3480cbb1 authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[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/+/2398562Reviewed-by: default avatarMehran Mahmoudi <mahmoudi@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805317}
parent 0251e808
......@@ -13,6 +13,7 @@ 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;
......@@ -49,6 +50,9 @@ 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() {
......
......@@ -25,12 +25,14 @@ 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",
......
......@@ -152,7 +152,10 @@ public class TabbedPaintPreviewPlayer implements TabViewProvider, UserData {
boolean hasCapture = mPaintPreviewTabService.hasCaptureForTab(mTab.getId());
mInitializing = hasCapture;
mMetricsHelper.recordHadCapture(hasCapture);
if (!hasCapture) return false;
if (!hasCapture) {
mPaintPreviewTabService.stopWarmCompositor();
return false;
}
mFirstMeaningfulPaintHappened = false;
mPlayerManager = new PlayerManager(mTab.getUrl(), mTab.getContext(),
......@@ -186,6 +189,7 @@ 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;
......
......@@ -115,6 +115,17 @@ 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
......@@ -225,5 +236,6 @@ public class PaintPreviewTabService implements NativePaintPreviewServiceProvider
void auditArtifactsAndroid(long nativePaintPreviewTabService, int[] activeTabIds);
boolean isCacheInitializedAndroid(long nativePaintPreviewTabService);
String getPathAndroid(long nativePaintPreviewTabService);
boolean stopWarmCompositorAndroid(long nativePaintPreviewTabService);
}
}
// 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);
}
}
......@@ -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
......@@ -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)
......@@ -55,12 +56,16 @@ PaintPreviewTabService::PaintPreviewTabService(
const base::FilePath& profile_dir,
base::StringPiece ascii_feature_name,
std::unique_ptr<PaintPreviewPolicy> policy,
bool is_off_the_record)
bool is_off_the_record,
bool prewarm_compositor)
: 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,
......@@ -155,6 +160,11 @@ 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,
......@@ -209,6 +219,10 @@ 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(
......
......@@ -41,7 +41,8 @@ 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 is_off_the_record,
bool prewarm_compositor = true);
~PaintPreviewTabService() override;
enum Status {
......@@ -78,6 +79,9 @@ 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(
......@@ -100,6 +104,7 @@ 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)
......
......@@ -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);
temp_dir_.GetPath(), kFeatureName, nullptr, false, 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);
nullptr, false, false);
}
private:
......
......@@ -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 = [
......
......@@ -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();
}
......
......@@ -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.
......
// 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
// 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_
......@@ -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(
......
......@@ -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;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment