Commit 5d570b7f authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

MacViews: Disable ui::Compositor recycling

Update logic for counting spare compositors (otherwise would be broken
by this change), and add optional "force new compositor" argument.

This is a speculative "revert", to determine the cause for GPU crashes
in GLFence::Create.

Bug: 863817
Change-Id: I47cae1fb78e73481d1272d4ce9d56b19dcb4079c
Reviewed-on: https://chromium-review.googlesource.com/1138872Reviewed-by: default avatarccameron <ccameron@chromium.org>
Reviewed-by: default avatarLeonard Grey <lgrey@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575441}
parent 58f43363
...@@ -16,6 +16,9 @@ namespace ui { ...@@ -16,6 +16,9 @@ namespace ui {
namespace { namespace {
// The number of RecyclableCompositorMacs in existence.
size_t g_recyclable_compositor_count = 0;
// Returns a task runner for creating a ui::Compositor. This allows compositor // Returns a task runner for creating a ui::Compositor. This allows compositor
// tasks to be funneled through ui::WindowResizeHelper's task runner to allow // tasks to be funneled through ui::WindowResizeHelper's task runner to allow
// resize operations to coordinate with frames provided by the GPU process. // resize operations to coordinate with frames provided by the GPU process.
...@@ -43,6 +46,7 @@ RecyclableCompositorMac::RecyclableCompositorMac( ...@@ -43,6 +46,7 @@ RecyclableCompositorMac::RecyclableCompositorMac(
GetCompositorTaskRunner(), GetCompositorTaskRunner(),
features::IsSurfaceSynchronizationEnabled(), features::IsSurfaceSynchronizationEnabled(),
ui::IsPixelCanvasRecordingEnabled()) { ui::IsPixelCanvasRecordingEnabled()) {
g_recyclable_compositor_count += 1;
compositor_.SetAcceleratedWidget( compositor_.SetAcceleratedWidget(
accelerated_widget_mac_->accelerated_widget()); accelerated_widget_mac_->accelerated_widget());
Suspend(); Suspend();
...@@ -51,6 +55,7 @@ RecyclableCompositorMac::RecyclableCompositorMac( ...@@ -51,6 +55,7 @@ RecyclableCompositorMac::RecyclableCompositorMac(
RecyclableCompositorMac::~RecyclableCompositorMac() { RecyclableCompositorMac::~RecyclableCompositorMac() {
compositor_.RemoveObserver(this); compositor_.RemoveObserver(this);
g_recyclable_compositor_count -= 1;
} }
void RecyclableCompositorMac::Suspend() { void RecyclableCompositorMac::Suspend() {
...@@ -100,9 +105,9 @@ RecyclableCompositorMacFactory* RecyclableCompositorMacFactory::Get() { ...@@ -100,9 +105,9 @@ RecyclableCompositorMacFactory* RecyclableCompositorMacFactory::Get() {
std::unique_ptr<RecyclableCompositorMac> std::unique_ptr<RecyclableCompositorMac>
RecyclableCompositorMacFactory::CreateCompositor( RecyclableCompositorMacFactory::CreateCompositor(
ui::ContextFactory* context_factory, ui::ContextFactory* context_factory,
ui::ContextFactoryPrivate* context_factory_private) { ui::ContextFactoryPrivate* context_factory_private,
active_compositor_count_ += 1; bool force_new_compositor) {
if (!compositors_.empty()) { if (!compositors_.empty() && !force_new_compositor) {
std::unique_ptr<RecyclableCompositorMac> result; std::unique_ptr<RecyclableCompositorMac> result;
result = std::move(compositors_.back()); result = std::move(compositors_.back());
compositors_.pop_back(); compositors_.pop_back();
...@@ -114,16 +119,6 @@ RecyclableCompositorMacFactory::CreateCompositor( ...@@ -114,16 +119,6 @@ RecyclableCompositorMacFactory::CreateCompositor(
void RecyclableCompositorMacFactory::RecycleCompositor( void RecyclableCompositorMacFactory::RecycleCompositor(
std::unique_ptr<RecyclableCompositorMac> compositor) { std::unique_ptr<RecyclableCompositorMac> compositor) {
// When we get to zero compositors in use, destroy all spare compositors.
// This is done to appease tests that rely on compositors being destroyed
// immediately (if the compositor is recycled and continues to exist, its
// subsequent initialization will crash).
active_compositor_count_ -= 1;
if (!active_compositor_count_) {
compositors_.clear();
return;
}
if (recycling_disabled_) if (recycling_disabled_)
return; return;
...@@ -132,6 +127,15 @@ void RecyclableCompositorMacFactory::RecycleCompositor( ...@@ -132,6 +127,15 @@ void RecyclableCompositorMacFactory::RecycleCompositor(
// Make this RecyclableCompositorMac recyclable for future instances. // Make this RecyclableCompositorMac recyclable for future instances.
compositors_.push_back(std::move(compositor)); compositors_.push_back(std::move(compositor));
// When we get to zero active compositors in use, destroy all spare
// compositors. This is done to appease tests that rely on compositors being
// destroyed immediately (if the compositor is recycled and continues to
// exist, its subsequent initialization will crash).
if (g_recyclable_compositor_count == compositors_.size()) {
compositors_.clear();
return;
}
// Post a task to free up the spare ui::Compositors when needed. Post this // Post a task to free up the spare ui::Compositors when needed. Post this
// to the browser main thread so that we won't free any compositors while // to the browser main thread so that we won't free any compositors while
// in a nested loop waiting to put up a new frame. // in a nested loop waiting to put up a new frame.
......
...@@ -82,7 +82,8 @@ class COMPOSITOR_EXPORT RecyclableCompositorMacFactory { ...@@ -82,7 +82,8 @@ class COMPOSITOR_EXPORT RecyclableCompositorMacFactory {
// Create a compositor, or recycle a preexisting one. // Create a compositor, or recycle a preexisting one.
std::unique_ptr<RecyclableCompositorMac> CreateCompositor( std::unique_ptr<RecyclableCompositorMac> CreateCompositor(
ui::ContextFactory* context_factory, ui::ContextFactory* context_factory,
ui::ContextFactoryPrivate* context_factory_private); ui::ContextFactoryPrivate* context_factory_private,
bool force_new_compositor = false);
// Delete a compositor, or allow it to be recycled. // Delete a compositor, or allow it to be recycled.
void RecycleCompositor(std::unique_ptr<RecyclableCompositorMac> compositor); void RecycleCompositor(std::unique_ptr<RecyclableCompositorMac> compositor);
...@@ -92,13 +93,11 @@ class COMPOSITOR_EXPORT RecyclableCompositorMacFactory { ...@@ -92,13 +93,11 @@ class COMPOSITOR_EXPORT RecyclableCompositorMacFactory {
private: private:
friend class base::NoDestructor<ui::RecyclableCompositorMacFactory>; friend class base::NoDestructor<ui::RecyclableCompositorMacFactory>;
friend class RecyclableCompositorMac;
RecyclableCompositorMacFactory(); RecyclableCompositorMacFactory();
~RecyclableCompositorMacFactory(); ~RecyclableCompositorMacFactory();
void ReduceSpareCompositors(); void ReduceSpareCompositors();
// The number of RecyclableCompositors that have been vended out and have
// not yet been recycled.
size_t active_compositor_count_ = 0;
bool recycling_disabled_ = false; bool recycling_disabled_ = false;
std::list<std::unique_ptr<RecyclableCompositorMac>> compositors_; std::list<std::unique_ptr<RecyclableCompositorMac>> compositors_;
base::WeakPtrFactory<RecyclableCompositorMacFactory> weak_factory_; base::WeakPtrFactory<RecyclableCompositorMacFactory> weak_factory_;
......
...@@ -1204,8 +1204,11 @@ void BridgedNativeWidget::CreateCompositor() { ...@@ -1204,8 +1204,11 @@ void BridgedNativeWidget::CreateCompositor() {
AddCompositorSuperview(); AddCompositorSuperview();
// TODO(ccameron): Re-enable compositor recycling here.
// https://crbug.com/863817
compositor_ = ui::RecyclableCompositorMacFactory::Get()->CreateCompositor( compositor_ = ui::RecyclableCompositorMacFactory::Get()->CreateCompositor(
context_factory, context_factory_private); context_factory, context_factory_private,
true /* force_new_compositor */);
compositor_->widget()->SetNSView(this); compositor_->widget()->SetNSView(this);
} }
...@@ -1236,8 +1239,9 @@ void BridgedNativeWidget::DestroyCompositor() { ...@@ -1236,8 +1239,9 @@ void BridgedNativeWidget::DestroyCompositor() {
return; return;
compositor_->widget()->ResetNSView(); compositor_->widget()->ResetNSView();
compositor_->compositor()->SetRootLayer(nullptr); compositor_->compositor()->SetRootLayer(nullptr);
ui::RecyclableCompositorMacFactory::Get()->RecycleCompositor( // TODO(ccameron): Re-enable compositor recycling here.
std::move(compositor_)); // https://crbug.com/863817
compositor_.reset();
} }
void BridgedNativeWidget::AddCompositorSuperview() { void BridgedNativeWidget::AddCompositorSuperview() {
......
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