 Chromium Code Reviews
 Chromium Code Reviews Issue 2493223002:
  Change exo::SurfaceFactoryOwner to exo::ExoCompositorFrameSink  (Closed)
    
  
    Issue 2493223002:
  Change exo::SurfaceFactoryOwner to exo::ExoCompositorFrameSink  (Closed) 
  | Index: components/exo/surface.cc | 
| diff --git a/components/exo/surface.cc b/components/exo/surface.cc | 
| index 5edc802f9635be0168c30c9bf197969867378ace..d717870792ef525645605fef9376601942e3fa7c 100644 | 
| --- a/components/exo/surface.cc | 
| +++ b/components/exo/surface.cc | 
| @@ -24,6 +24,7 @@ | 
| #include "components/exo/buffer.h" | 
| #include "components/exo/surface_delegate.h" | 
| #include "components/exo/surface_observer.h" | 
| +#include "mojo/public/cpp/bindings/strong_binding.h" | 
| #include "third_party/khronos/GLES2/gl2.h" | 
| #include "ui/aura/env.h" | 
| #include "ui/aura/window_delegate.h" | 
| @@ -162,52 +163,15 @@ void RequireCallback(cc::SurfaceManager* manager, | 
| } // namespace | 
| //////////////////////////////////////////////////////////////////////////////// | 
| -// SurfaceFactoryOwner, public: | 
| - | 
| -SurfaceFactoryOwner::SurfaceFactoryOwner() {} | 
| - | 
| -//////////////////////////////////////////////////////////////////////////////// | 
| -// cc::SurfaceFactoryClient overrides: | 
| - | 
| -void SurfaceFactoryOwner::ReturnResources( | 
| - const cc::ReturnedResourceArray& resources) { | 
| - scoped_refptr<SurfaceFactoryOwner> holder(this); | 
| - for (auto& resource : resources) { | 
| - auto it = release_callbacks_.find(resource.id); | 
| - DCHECK(it != release_callbacks_.end()); | 
| - it->second.second->Run(resource.sync_token, resource.lost); | 
| - release_callbacks_.erase(it); | 
| - } | 
| -} | 
| - | 
| -void SurfaceFactoryOwner::WillDrawSurface(const cc::LocalFrameId& id, | 
| - const gfx::Rect& damage_rect) { | 
| - if (surface_) | 
| - surface_->WillDraw(); | 
| -} | 
| - | 
| -void SurfaceFactoryOwner::SetBeginFrameSource( | 
| - cc::BeginFrameSource* begin_frame_source) { | 
| - if (surface_) | 
| - surface_->SetBeginFrameSource(begin_frame_source); | 
| -} | 
| - | 
| -//////////////////////////////////////////////////////////////////////////////// | 
| -// SurfaceFactoryOwner, private: | 
| - | 
| -SurfaceFactoryOwner::~SurfaceFactoryOwner() { | 
| - if (surface_factory_->manager()) | 
| - surface_factory_->manager()->InvalidateFrameSinkId(frame_sink_id_); | 
| -} | 
| - | 
| -//////////////////////////////////////////////////////////////////////////////// | 
| // Surface, public: | 
| Surface::Surface() | 
| : window_(new aura::Window(new CustomWindowDelegate(this))), | 
| - surface_manager_( | 
| - aura::Env::GetInstance()->context_factory()->GetSurfaceManager()), | 
| - factory_owner_(new SurfaceFactoryOwner) { | 
| + compositor_frame_sink_(new ExoCompositorFrameSink( | 
| + aura::Env::GetInstance()->context_factory()->AllocateFrameSinkId(), | 
| + aura::Env::GetInstance()->context_factory()->GetSurfaceManager())), | 
| + surface_id_(cc::SurfaceId(compositor_frame_sink_->frame_sink_id(), | 
| + cc::LocalFrameId())) { | 
| window_->SetType(ui::wm::WINDOW_TYPE_CONTROL); | 
| window_->SetName("ExoSurface"); | 
| window_->SetProperty(kSurfaceKey, this); | 
| @@ -215,15 +179,6 @@ Surface::Surface() | 
| window_->SetEventTargeter(base::WrapUnique(new CustomWindowTargeter)); | 
| window_->set_owned_by_parent(false); | 
| window_->AddObserver(this); | 
| - factory_owner_->surface_ = this; | 
| - factory_owner_->frame_sink_id_ = | 
| - aura::Env::GetInstance()->context_factory()->AllocateFrameSinkId(); | 
| - factory_owner_->id_allocator_.reset(new cc::SurfaceIdAllocator()); | 
| - surface_manager_->RegisterFrameSinkId(factory_owner_->frame_sink_id_); | 
| - surface_manager_->RegisterSurfaceFactoryClient(factory_owner_->frame_sink_id_, | 
| - factory_owner_.get()); | 
| - factory_owner_->surface_factory_.reset(new cc::SurfaceFactory( | 
| - factory_owner_->frame_sink_id_, surface_manager_, factory_owner_.get())); | 
| aura::Env::GetInstance()->context_factory()->AddObserver(this); | 
| } | 
| @@ -235,8 +190,6 @@ Surface::~Surface() { | 
| window_->RemoveObserver(this); | 
| window_->layer()->SetShowSolidColorContent(); | 
| - factory_owner_->surface_ = nullptr; | 
| - | 
| // Call pending frame callbacks with a null frame time to indicate that they | 
| // have been cancelled. | 
| frame_callbacks_.splice(frame_callbacks_.end(), pending_frame_callbacks_); | 
| @@ -248,11 +201,7 @@ Surface::~Surface() { | 
| if (begin_frame_source_ && needs_begin_frame_) | 
| begin_frame_source_->RemoveObserver(this); | 
| - if (local_frame_id_.is_valid()) | 
| - factory_owner_->surface_factory_->Destroy(local_frame_id_); | 
| - surface_manager_->UnregisterSurfaceFactoryClient( | 
| - factory_owner_->frame_sink_id_); | 
| } | 
| // static | 
| @@ -261,7 +210,7 @@ Surface* Surface::AsSurface(const aura::Window* window) { | 
| } | 
| cc::SurfaceId Surface::GetSurfaceId() const { | 
| - return cc::SurfaceId(factory_owner_->frame_sink_id_, local_frame_id_); | 
| + return surface_id_; | 
| } | 
| void Surface::Attach(Buffer* buffer) { | 
| @@ -474,22 +423,10 @@ void Surface::CommitSurfaceHierarchy() { | 
| UpdateResource(true); | 
| } | 
| - cc::LocalFrameId old_local_frame_id = local_frame_id_; | 
| - if (needs_commit_to_new_surface_ || !local_frame_id_.is_valid()) { | 
| - needs_commit_to_new_surface_ = false; | 
| - local_frame_id_ = factory_owner_->id_allocator_->GenerateId(); | 
| - factory_owner_->surface_factory_->Create(local_frame_id_); | 
| - } | 
| - | 
| UpdateSurface(true); | 
| - if (old_local_frame_id.is_valid() && old_local_frame_id != local_frame_id_) { | 
| - factory_owner_->surface_factory_->SetPreviousFrameSurface( | 
| - local_frame_id_, old_local_frame_id); | 
| - factory_owner_->surface_factory_->Destroy(old_local_frame_id); | 
| - } | 
| - | 
| - if (old_local_frame_id != local_frame_id_) { | 
| + if (needs_commit_to_new_surface_) { | 
| + needs_begin_frame_ = false; | 
| float contents_surface_to_layer_scale = 1.0; | 
| // The bounds must be updated before switching to the new surface, because | 
| // the layer may be mirrored, in which case a surface change causes the | 
| @@ -497,9 +434,11 @@ void Surface::CommitSurfaceHierarchy() { | 
| window_->layer()->SetBounds( | 
| gfx::Rect(window_->layer()->bounds().origin(), content_size_)); | 
| window_->layer()->SetShowSurface( | 
| - cc::SurfaceId(factory_owner_->frame_sink_id_, local_frame_id_), | 
| - base::Bind(&SatisfyCallback, base::Unretained(surface_manager_)), | 
| - base::Bind(&RequireCallback, base::Unretained(surface_manager_)), | 
| + surface_id_, | 
| + base::Bind(&SatisfyCallback, | 
| + base::Unretained(compositor_frame_sink_->surface_manager())), | 
| + base::Bind(&RequireCallback, | 
| + base::Unretained(compositor_frame_sink_->surface_manager())), | 
| content_size_, contents_surface_to_layer_scale, content_size_); | 
| window_->layer()->SetFillsBoundsOpaquely( | 
| state_.blend_mode == SkXfermode::kSrc_Mode || | 
| @@ -510,8 +449,9 @@ void Surface::CommitSurfaceHierarchy() { | 
| // Reset damage. | 
| pending_damage_.setEmpty(); | 
| - DCHECK(!current_resource_.id || | 
| - factory_owner_->release_callbacks_.count(current_resource_.id)); | 
| + DCHECK( | 
| + !current_resource_.id || | 
| + compositor_frame_sink_->release_callbacks_.count(current_resource_.id)); | 
| // Move pending frame callbacks to the end of frame_callbacks_. | 
| frame_callbacks_.splice(frame_callbacks_.end(), pending_frame_callbacks_); | 
| @@ -636,7 +576,7 @@ void Surface::CheckIfSurfaceHierarchyNeedsCommitToNewSurfaces() { | 
| } | 
| void Surface::OnLostResources() { | 
| - if (!local_frame_id_.is_valid()) | 
| + if (!surface_id_.is_valid()) | 
| return; | 
| UpdateResource(false); | 
| @@ -645,13 +585,13 @@ void Surface::OnLostResources() { | 
| void Surface::OnWindowAddedToRootWindow(aura::Window* window) { | 
| window->layer()->GetCompositor()->AddFrameSink( | 
| - factory_owner_->frame_sink_id_); | 
| + compositor_frame_sink_->frame_sink_id_); | 
| } | 
| void Surface::OnWindowRemovingFromRootWindow(aura::Window* window, | 
| aura::Window* new_root) { | 
| window->layer()->GetCompositor()->RemoveFrameSink( | 
| - factory_owner_->frame_sink_id_); | 
| + compositor_frame_sink_->frame_sink_id_); | 
| } | 
| void Surface::OnBeginFrame(const cc::BeginFrameArgs& args) { | 
| @@ -751,8 +691,8 @@ void Surface::UpdateResource(bool client_usage) { | 
| texture_mailbox.target()); | 
| resource.is_overlay_candidate = texture_mailbox.is_overlay_candidate(); | 
| - factory_owner_->release_callbacks_[resource.id] = std::make_pair( | 
| - factory_owner_, std::move(texture_mailbox_release_callback)); | 
| + compositor_frame_sink_->release_callbacks_[resource.id] = std::make_pair( | 
| 
Fady Samuel
2016/11/15 23:19:46
These release_callbacks are weird. I would investi
 | 
| + compositor_frame_sink_, std::move(texture_mailbox_release_callback)); | 
| current_resource_ = resource; | 
| } else { | 
| current_resource_.id = 0; | 
| @@ -842,8 +782,7 @@ void Surface::UpdateSurface(bool full_damage) { | 
| frame.render_pass_list.push_back(std::move(render_pass)); | 
| - factory_owner_->surface_factory_->SubmitCompositorFrame( | 
| - local_frame_id_, std::move(frame), cc::SurfaceFactory::DrawCallback()); | 
| + compositor_frame_sink_->SubmitCompositorFrame(std::move(frame)); | 
| } | 
| void Surface::UpdateNeedsBeginFrame() { |