Skip to content

Commit

Permalink
Flip default on FrameSinkManagerImpl.
Browse files Browse the repository at this point in the history
Make FrameSinkManagerImpl by default use surface references. Most
instances of FrameSinkManagerImpl are using surface references now so it
makes the most sense to be the default.

Bug: 676384
Change-Id: I935e89ec137f1a41f6409e0e2b045a4b6105f2b9
Reviewed-on: https://chromium-review.googlesource.com/617310
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495587}
  • Loading branch information
kylechar authored and Commit Bot committed Aug 18, 2017
1 parent 45ecde9 commit 848a09e
Show file tree
Hide file tree
Showing 20 changed files with 27 additions and 48 deletions.
3 changes: 2 additions & 1 deletion android_webview/browser/surfaces_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ SurfacesInstance::SurfacesInstance()
// Webview does not own the surface so should not clear it.
settings.should_clear_root_render_pass = false;

frame_sink_manager_.reset(new viz::FrameSinkManagerImpl);
frame_sink_manager_ = std::make_unique<viz::FrameSinkManagerImpl>(
viz::SurfaceManager::LifetimeType::SEQUENCES);
local_surface_id_allocator_.reset(new viz::LocalSurfaceIdAllocator());

constexpr bool is_root = true;
Expand Down
4 changes: 1 addition & 3 deletions components/viz/host/host_frame_sink_manager_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ class FakeHostFrameSinkClient : public HostFrameSinkClient {
// A mock implementation of mojom::FrameSinkManager.
class MockFrameSinkManagerImpl : public FrameSinkManagerImpl {
public:
MockFrameSinkManagerImpl()
: FrameSinkManagerImpl(nullptr,
SurfaceManager::LifetimeType::REFERENCES) {}
MockFrameSinkManagerImpl() = default;
~MockFrameSinkManagerImpl() override = default;

// mojom::FrameSinkManager:
Expand Down
3 changes: 1 addition & 2 deletions components/viz/service/display/display_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ class TestDisplayScheduler : public DisplayScheduler {
class DisplayTest : public testing::Test {
public:
DisplayTest()
: manager_(nullptr, SurfaceManager::LifetimeType::REFERENCES),
support_(
: support_(
CompositorFrameSinkSupport::Create(nullptr,
&manager_,
kArbitraryFrameSinkId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ const base::UnguessableToken kArbitraryToken = base::UnguessableToken::Create();

class SurfaceAggregatorPerfTest : public testing::Test {
public:
SurfaceAggregatorPerfTest()
: manager_(nullptr, SurfaceManager::LifetimeType::REFERENCES) {
SurfaceAggregatorPerfTest() {
context_provider_ = cc::TestContextProvider::Create();
context_provider_->BindToCurrentThread();
shared_bitmap_manager_ = base::MakeUnique<cc::TestSharedBitmapManager>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ constexpr bool kNeedsSyncPoints = true;
class SurfaceAggregatorPixelTest : public cc::RendererPixelTest<GLRenderer> {
public:
SurfaceAggregatorPixelTest()
: manager_(nullptr, SurfaceManager::LifetimeType::REFERENCES),
support_(CompositorFrameSinkSupport::Create(nullptr,
: support_(CompositorFrameSinkSupport::Create(nullptr,
&manager_,
kArbitraryRootFrameSinkId,
kIsRoot,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ gfx::Size SurfaceSize() {
class SurfaceAggregatorTest : public testing::Test {
public:
explicit SurfaceAggregatorTest(bool use_damage_rect)
: manager_(nullptr, SurfaceManager::LifetimeType::REFERENCES),
observer_(false),
: observer_(false),
support_(CompositorFrameSinkSupport::Create(&fake_client_,
&manager_,
kArbitraryRootFrameSinkId,
Expand Down Expand Up @@ -2127,9 +2126,6 @@ TEST_F(SurfaceAggregatorPartialSwapTest, IgnoreOutside) {

class SurfaceAggregatorWithResourcesTest : public testing::Test {
public:
SurfaceAggregatorWithResourcesTest()
: manager_(nullptr, SurfaceManager::LifetimeType::REFERENCES) {}

void SetUp() override {
shared_bitmap_manager_ = base::MakeUnique<cc::TestSharedBitmapManager>();
resource_provider_ =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ class FakeCompositorFrameSinkSupportClient
class CompositorFrameSinkSupportTest : public testing::Test {
public:
CompositorFrameSinkSupportTest()
: manager_(nullptr, SurfaceManager::LifetimeType::REFERENCES),
frame_sink_manager_client_(&manager_),
: frame_sink_manager_client_(&manager_),
begin_frame_source_(0.f, false),
local_surface_id_(3, kArbitraryToken),
frame_sync_token_(GenTestSyncToken(4)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class DirectLayerTreeFrameSinkTest : public testing::Test {
task_runner_(new cc::OrderedSimpleTaskRunner(now_src_.get(), true)),
display_size_(1920, 1080),
display_rect_(display_size_),
frame_sink_manager_(nullptr, SurfaceManager::LifetimeType::REFERENCES),
support_manager_(&frame_sink_manager_),
context_provider_(cc::TestContextProvider::Create()) {
auto display_output_surface = cc::FakeOutputSurface::Create3d();
Expand Down
4 changes: 2 additions & 2 deletions components/viz/service/frame_sinks/frame_sink_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ FrameSinkManagerImpl::FrameSinkSourceMapping::~FrameSinkSourceMapping() =
default;

FrameSinkManagerImpl::FrameSinkManagerImpl(
DisplayProvider* display_provider,
SurfaceManager::LifetimeType lifetime_type)
SurfaceManager::LifetimeType lifetime_type,
DisplayProvider* display_provider)
: display_provider_(display_provider),
surface_manager_(lifetime_type),
binding_(this) {
Expand Down
6 changes: 3 additions & 3 deletions components/viz/service/frame_sinks/frame_sink_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ class FrameSinkManagerClient;
class VIZ_SERVICE_EXPORT FrameSinkManagerImpl : public SurfaceObserver,
public mojom::FrameSinkManager {
public:
FrameSinkManagerImpl(DisplayProvider* display_provider = nullptr,
SurfaceManager::LifetimeType lifetime_type =
SurfaceManager::LifetimeType::SEQUENCES);
FrameSinkManagerImpl(SurfaceManager::LifetimeType lifetime_type =
SurfaceManager::LifetimeType::REFERENCES,
DisplayProvider* display_provider = nullptr);
~FrameSinkManagerImpl() override;

// Binds |this| as a FrameSinkManagerImpl for |request| on |task_runner|. On
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ class FakeFrameSinkManagerClient : public FrameSinkManagerClient {

class FrameSinkManagerTest : public testing::Test {
public:
FrameSinkManagerTest()
: manager_(nullptr, SurfaceManager::LifetimeType::REFERENCES) {}

FrameSinkManagerTest() = default;
~FrameSinkManagerTest() override = default;

protected:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ class SurfaceReferencesTest : public testing::Test {
// testing::Test:
void SetUp() override {
// Start each test with a fresh SurfaceManager instance.
manager_ = base::MakeUnique<FrameSinkManagerImpl>(
nullptr /* display_provider */,
SurfaceManager::LifetimeType::REFERENCES);
manager_ = std::make_unique<FrameSinkManagerImpl>();
}
void TearDown() override {
for (auto& support : supports_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,7 @@ class FakeExternalBeginFrameSourceClient

class SurfaceSynchronizationTest : public testing::Test {
public:
SurfaceSynchronizationTest()
: frame_sink_manager_(nullptr /* display_provider */,
SurfaceManager::LifetimeType::REFERENCES),
surface_observer_(false) {}
SurfaceSynchronizationTest() : surface_observer_(false) {}
~SurfaceSynchronizationTest() override {}

CompositorFrameSinkSupport& display_support() {
Expand Down
7 changes: 2 additions & 5 deletions components/viz/service/surfaces/surface_hittest_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,8 @@ using namespace test;

class SurfaceHittestTest : public testing::Test {
public:
SurfaceHittestTest()
: frame_sink_manager_(nullptr /* display_provider */,
SurfaceManager::LifetimeType::REFERENCES) {}

~SurfaceHittestTest() override {}
SurfaceHittestTest() = default;
~SurfaceHittestTest() override = default;

CompositorFrameSinkSupport& root_support() { return *supports_[0]; }

Expand Down
2 changes: 1 addition & 1 deletion components/viz/service/surfaces/surface_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ constexpr bool kNeedsSyncPoints = true;

TEST(SurfaceTest, SurfaceLifetime) {
FrameSinkManagerImpl frame_sink_manager(
nullptr /* display_provider */, SurfaceManager::LifetimeType::SEQUENCES);
SurfaceManager::LifetimeType::SEQUENCES);
SurfaceManager* surface_manager = frame_sink_manager.surface_manager();
auto support = CompositorFrameSinkSupport::Create(
nullptr, &frame_sink_manager, kArbitraryFrameSinkId, kIsRoot,
Expand Down
4 changes: 2 additions & 2 deletions content/browser/browser_main_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1487,8 +1487,8 @@ int BrowserMainLoop::BrowserThreadsStarted() {
switches::kDisableSurfaceReferences)
? viz::SurfaceManager::LifetimeType::SEQUENCES
: viz::SurfaceManager::LifetimeType::REFERENCES;
frame_sink_manager_impl_ = base::MakeUnique<viz::FrameSinkManagerImpl>(
nullptr /* display_provider */, surface_lifetime_type);
frame_sink_manager_impl_ =
std::make_unique<viz::FrameSinkManagerImpl>(surface_lifetime_type);

host_frame_sink_manager_ = base::MakeUnique<viz::HostFrameSinkManager>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
namespace content {

NoTransportImageTransportFactory::NoTransportImageTransportFactory()
: frame_sink_manager_(nullptr,
viz::SurfaceManager::LifetimeType::REFERENCES),
context_factory_(&host_frame_sink_manager_, &frame_sink_manager_) {
: context_factory_(&host_frame_sink_manager_, &frame_sink_manager_) {
surface_utils::ConnectWithLocalFrameSinkManager(&host_frame_sink_manager_,
&frame_sink_manager_);

Expand Down
4 changes: 2 additions & 2 deletions content/browser/renderer_host/compositor_impl_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ struct CompositorDependencies {

// TODO(danakj): Don't make a FrameSinkManagerImpl when display is in the
// Gpu process, instead get the mojo pointer from the Gpu process.
frame_sink_manager_impl = base::MakeUnique<viz::FrameSinkManagerImpl>(
nullptr /* display_provider */, surface_lifetime_type);
frame_sink_manager_impl =
std::make_unique<viz::FrameSinkManagerImpl>(surface_lifetime_type);
surface_utils::ConnectWithLocalFrameSinkManager(
&host_frame_sink_manager, frame_sink_manager_impl.get());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ bool SynchronousLayerTreeFrameSink::BindToClient(
if (!cc::LayerTreeFrameSink::BindToClient(sink_client))
return false;

frame_sink_manager_ = base::MakeUnique<viz::FrameSinkManagerImpl>();
frame_sink_manager_ = std::make_unique<viz::FrameSinkManagerImpl>(
viz::SurfaceManager::LifetimeType::SEQUENCES);

DCHECK(begin_frame_source_);
client_->SetBeginFrameSource(begin_frame_source_.get());
Expand Down
2 changes: 1 addition & 1 deletion services/ui/gpu/gpu_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ void GpuMain::CreateFrameSinkManagerOnCompositorThread(
gpu_command_service_, gpu_service_->gpu_channel_manager());

frame_sink_manager_ = base::MakeUnique<viz::FrameSinkManagerImpl>(
display_provider_.get(), viz::SurfaceManager::LifetimeType::REFERENCES);
viz::SurfaceManager::LifetimeType::REFERENCES, display_provider_.get());
frame_sink_manager_->BindAndSetClient(std::move(request), nullptr,
std::move(client));
}
Expand Down

0 comments on commit 848a09e

Please sign in to comment.