diff options
author | danakj@chromium.org <danakj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-22 23:29:51 +0000 |
---|---|---|
committer | danakj@chromium.org <danakj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-22 23:29:51 +0000 |
commit | 819b9f5580eea88e9d1610a4b7903062cc5a89d4 (patch) | |
tree | b63d391d27dc4e8e63c2e4d96822059ee795bbb2 | |
parent | 29834b67f21ae4d72157d4374312a96cb265024b (diff) | |
download | chromium_src-819b9f5580eea88e9d1610a4b7903062cc5a89d4.zip chromium_src-819b9f5580eea88e9d1610a4b7903062cc5a89d4.tar.gz chromium_src-819b9f5580eea88e9d1610a4b7903062cc5a89d4.tar.bz2 |
cc: Return resources to child compositor via a Callback.
Currently resources in the ResourceProvider are returned for release
back to a child compositor by explicitly calling the method
ResourceProvider::PrepareSendReturnsToChild(). This is problematic
because we want to return resources immediately if the last reference
on the resource is released when a parent compositor gives the resource
back to us.
TextureMailboxes deal with this gracefully by using a ReleaseCallback
in the ResourceProvider that is called when the mailbox is no longer in
use. We follow this example to release resources back to child
compositors by introducing a ReturnCallback in the ResourceProvider.
The ReturnCallback receives an array of resources instead of a single
resource per call, allowing us to save on post tasks and Run()
invocations.
The PrepareSendReturnsToChild is completely replaced by the new callback
mechanism. Resources are returned when they are no longer in use in the
current frame, and their exported reference count reaches zero (or lost
if the child compositor's connection is destroyed).
When the DelegatedRendererLayerImpl recieves a new frame, it will always
add all the resources into the ResourceProvider so that it can map the
resource ids correctly into its own compositor's namespace. Then it will
decide if the frame is valid or not. At that point, the DRLI must decide
to use the new frame, or continue using the old frame. It does this by
calling DeclareUsedResourcesFromChild() on the provider. Any resource
not listed in the declaration will become unused and returned to the
child as soon as possible. This changes the contract on ResourceProvider
such that every call to ReceiveFromChild() should be followed by a
DeclareUsedResourcesFromChild() call.
For now, DelegatedRendererLayer receives the callbacks about returned
resources and just inserts them in its list of resources for the
embedder to grab via TakeUnusedResourcesForChildCompositor(). A
follow-up step will be to move this callback out into the embedder
itself, similar to ReleaseCallback for mailboxes, so that the
RenderWidgetHostViewAura can promptly return resources instead of
waiting until the next commit to know resources are available.
Since resources are now returned to the main thread during commit, we
no longer have an extra pending frame of resources sitting on the impl
side, and can eliminate all the TODOs in our unit tests about this.
Covered by our many existing tests, but also added some new ones:
ResourceProviderTest.DeleteExportedResources
ResourceProviderTest.DestroyChildWithExportedResources
R=piman
BUG=263069
Review URL: https://chromiumcodereview.appspot.com/24078024
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@224640 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | cc/layers/delegated_renderer_layer.cc | 40 | ||||
-rw-r--r-- | cc/layers/delegated_renderer_layer.h | 17 | ||||
-rw-r--r-- | cc/layers/delegated_renderer_layer_impl.cc | 68 | ||||
-rw-r--r-- | cc/layers/delegated_renderer_layer_impl.h | 14 | ||||
-rw-r--r-- | cc/resources/resource_provider.cc | 340 | ||||
-rw-r--r-- | cc/resources/resource_provider.h | 35 | ||||
-rw-r--r-- | cc/resources/resource_provider_unittest.cc | 317 | ||||
-rw-r--r-- | cc/resources/return_callback.h | 17 | ||||
-rw-r--r-- | cc/test/fake_delegated_renderer_layer_impl.cc | 15 | ||||
-rw-r--r-- | cc/test/fake_delegated_renderer_layer_impl.h | 4 | ||||
-rw-r--r-- | cc/test/layer_tree_test.h | 3 | ||||
-rw-r--r-- | cc/trees/layer_tree_host_unittest_delegated.cc | 225 | ||||
-rw-r--r-- | cc/trees/single_thread_proxy.cc | 22 |
13 files changed, 771 insertions, 346 deletions
diff --git a/cc/layers/delegated_renderer_layer.cc b/cc/layers/delegated_renderer_layer.cc index 5286874..cc49c75 100644 --- a/cc/layers/delegated_renderer_layer.cc +++ b/cc/layers/delegated_renderer_layer.cc @@ -8,6 +8,7 @@ #include "cc/layers/delegated_renderer_layer_impl.h" #include "cc/output/delegated_frame_data.h" #include "cc/quads/render_pass_draw_quad.h" +#include "cc/trees/blocking_task_runner.h" #include "cc/trees/layer_tree_host.h" namespace cc { @@ -22,7 +23,9 @@ DelegatedRendererLayer::DelegatedRendererLayer( DelegatedRendererLayerClient* client) : Layer(), client_(client), - needs_filter_context_(false) {} + needs_filter_context_(false), + main_thread_runner_(BlockingTaskRunner::current()), + weak_ptrs_(this) {} DelegatedRendererLayer::~DelegatedRendererLayer() {} @@ -64,19 +67,23 @@ void DelegatedRendererLayer::PushPropertiesTo(LayerImpl* impl) { delegated_impl->SetDisplaySize(display_size_); + delegated_impl->CreateChildIdIfNeeded( + base::Bind(&DelegatedRendererLayer::ReceiveUnusedResourcesOnImplThread, + main_thread_runner_, + weak_ptrs_.GetWeakPtr())); + if (frame_data_) delegated_impl->SetFrameData(frame_data_.Pass(), damage_in_frame_); frame_data_.reset(); damage_in_frame_ = gfx::RectF(); - delegated_impl->CollectUnusedResources( - &unused_resources_for_child_compositor_); - + // The ResourceProvider will have the new frame as soon as we push it to the + // pending tree. So unused resources will be returned as well. if (client_) client_->DidCommitFrameData(); - // TODO(danakj): TakeUnusedResourcesForChildCompositor requires a push - // properties to happen in order to collect unused resources returned + // TODO(danakj): The DidCommitFrameData() notification requires a push + // properties to happen in order to notify about unused resources returned // from the parent compositor. crbug.com/259090 needs_push_properties_ = true; } @@ -90,6 +97,8 @@ void DelegatedRendererLayer::SetDisplaySize(gfx::Size size) { void DelegatedRendererLayer::SetFrameData( scoped_ptr<DelegatedFrameData> new_frame_data) { + DCHECK(new_frame_data); + if (frame_data_) { // Copy the resources from the last provided frame into the unused resources // list, as the new frame will provide its own resources. @@ -139,4 +148,23 @@ void DelegatedRendererLayer::TakeUnusedResourcesForChildCompositor( array->swap(unused_resources_for_child_compositor_); } +void DelegatedRendererLayer::ReceiveUnusedResources( + const ReturnedResourceArray& unused) { + unused_resources_for_child_compositor_.insert( + unused_resources_for_child_compositor_.end(), + unused.begin(), + unused.end()); +} + +// static +void DelegatedRendererLayer::ReceiveUnusedResourcesOnImplThread( + scoped_refptr<BlockingTaskRunner> task_runner, + base::WeakPtr<DelegatedRendererLayer> self, + const ReturnedResourceArray& unused) { + task_runner->PostTask( + FROM_HERE, + base::Bind( + &DelegatedRendererLayer::ReceiveUnusedResources, self, unused)); +} + } // namespace cc diff --git a/cc/layers/delegated_renderer_layer.h b/cc/layers/delegated_renderer_layer.h index b3a078f..60e9c3c 100644 --- a/cc/layers/delegated_renderer_layer.h +++ b/cc/layers/delegated_renderer_layer.h @@ -5,12 +5,15 @@ #ifndef CC_LAYERS_DELEGATED_RENDERER_LAYER_H_ #define CC_LAYERS_DELEGATED_RENDERER_LAYER_H_ +#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" +#include "base/synchronization/lock.h" #include "cc/base/cc_export.h" #include "cc/layers/layer.h" #include "cc/resources/returned_resource.h" namespace cc { - +class BlockingTaskRunner; class DelegatedFrameData; class DelegatedRendererLayerClient; @@ -42,17 +45,27 @@ class CC_EXPORT DelegatedRendererLayer : public Layer { virtual ~DelegatedRendererLayer(); private: + void ReceiveUnusedResources(const ReturnedResourceArray& unused); + static void ReceiveUnusedResourcesOnImplThread( + scoped_refptr<BlockingTaskRunner> task_runner, + base::WeakPtr<DelegatedRendererLayer> self, + const ReturnedResourceArray& unused); + scoped_ptr<DelegatedFrameData> frame_data_; gfx::RectF damage_in_frame_; gfx::Size frame_size_; gfx::Size display_size_; - ReturnedResourceArray unused_resources_for_child_compositor_; DelegatedRendererLayerClient* client_; bool needs_filter_context_; + ReturnedResourceArray unused_resources_for_child_compositor_; + scoped_refptr<BlockingTaskRunner> main_thread_runner_; + base::WeakPtrFactory<DelegatedRendererLayer> weak_ptrs_; + DISALLOW_COPY_AND_ASSIGN(DelegatedRendererLayer); }; } // namespace cc + #endif // CC_LAYERS_DELEGATED_RENDERER_LAYER_H_ diff --git a/cc/layers/delegated_renderer_layer_impl.cc b/cc/layers/delegated_renderer_layer_impl.cc index b0a1b3f..34a1cd8 100644 --- a/cc/layers/delegated_renderer_layer_impl.cc +++ b/cc/layers/delegated_renderer_layer_impl.cc @@ -47,7 +47,7 @@ bool DelegatedRendererLayerImpl::HasContributingDelegatedRenderPasses() const { static ResourceProvider::ResourceId ResourceRemapHelper( bool* invalid_frame, const ResourceProvider::ResourceIdMap& child_to_parent_map, - ResourceProvider::ResourceIdSet *remapped_resources, + ResourceProvider::ResourceIdArray* resources_in_frame, ResourceProvider::ResourceId id) { ResourceProvider::ResourceIdMap::const_iterator it = @@ -59,7 +59,7 @@ static ResourceProvider::ResourceId ResourceRemapHelper( DCHECK_EQ(it->first, id); ResourceProvider::ResourceId remapped_id = it->second; - remapped_resources->insert(remapped_id); + resources_in_frame->push_back(id); return remapped_id; } @@ -85,23 +85,31 @@ void DelegatedRendererLayerImpl::PushPropertiesTo(LayerImpl* layer) { have_render_passes_to_push_ = false; } - // This is just a copy for testing since we keep the data on the pending layer - // for returning resources to the child for now. + // This is just a copy for testing, since resources are added to the + // ResourceProvider in the pending tree. delegated_layer->resources_ = resources_; } +void DelegatedRendererLayerImpl::CreateChildIdIfNeeded( + const ReturnCallback& return_callback) { + if (child_id_) + return; + + ResourceProvider* resource_provider = layer_tree_impl()->resource_provider(); + child_id_ = resource_provider->CreateChild(return_callback); + own_child_id_ = true; +} + void DelegatedRendererLayerImpl::SetFrameData( scoped_ptr<DelegatedFrameData> frame_data, gfx::RectF damage_in_frame) { DCHECK(frame_data); + DCHECK(child_id_) << "CreateChildIdIfNeeded must be called first."; // A frame with an empty root render pass is invalid. DCHECK(frame_data->render_pass_list.empty() || !frame_data->render_pass_list.back()->output_rect.IsEmpty()); - CreateChildIdIfNeeded(); - DCHECK(child_id_); - ResourceProvider* resource_provider = layer_tree_impl()->resource_provider(); const ResourceProvider::ResourceIdMap& resource_map = resource_provider->GetChildToParentMap(child_id_); @@ -109,12 +117,12 @@ void DelegatedRendererLayerImpl::SetFrameData( resource_provider->ReceiveFromChild(child_id_, frame_data->resource_list); bool invalid_frame = false; - ResourceProvider::ResourceIdSet used_resources; + ResourceProvider::ResourceIdArray resources_in_frame; DrawQuad::ResourceIteratorCallback remap_resources_to_parent_callback = base::Bind(&ResourceRemapHelper, &invalid_frame, resource_map, - &used_resources); + &resources_in_frame); for (size_t i = 0; i < frame_data->render_pass_list.size(); ++i) { RenderPass* pass = frame_data->render_pass_list[i]; for (size_t j = 0; j < pass->quad_list.size(); ++j) { @@ -123,8 +131,15 @@ void DelegatedRendererLayerImpl::SetFrameData( } } - if (invalid_frame) + if (invalid_frame) { + // Declare we are still using the last frame's resources. + resource_provider->DeclareUsedResourcesFromChild(child_id_, resources_); return; + } + + // Declare we are using the new frame's resources. + resources_.swap(resources_in_frame); + resource_provider->DeclareUsedResourcesFromChild(child_id_, resources_); // Display size is already set so we can compute what the damage rect // will be in layer space. @@ -140,33 +155,9 @@ void DelegatedRendererLayerImpl::SetFrameData( // Save the remapped quads on the layer. This steals the quads and render // passes from the frame_data. SetRenderPasses(&frame_data->render_pass_list); - resources_.swap(used_resources); have_render_passes_to_push_ = true; } -void DelegatedRendererLayerImpl::CollectUnusedResources( - ReturnedResourceArray* resources_for_ack) { - CreateChildIdIfNeeded(); - DCHECK(child_id_); - - ResourceProvider* resource_provider = layer_tree_impl()->resource_provider(); - const ResourceProvider::ResourceIdMap& resource_map = - resource_provider->GetChildToParentMap(child_id_); - - ResourceProvider::ResourceIdArray unused_resources; - for (ResourceProvider::ResourceIdMap::const_iterator it = - resource_map.begin(); - it != resource_map.end(); - ++it) { - bool resource_is_in_current_frame = resources_.count(it->second) > 0; - bool resource_is_in_use = resource_provider->InUseByConsumer(it->second); - if (!resource_is_in_current_frame && !resource_is_in_use) - unused_resources.push_back(it->second); - } - resource_provider->PrepareSendReturnsToChild( - child_id_, unused_resources, resources_for_ack); -} - void DelegatedRendererLayerImpl::SetDisplaySize(gfx::Size size) { if (display_size_ == size) return; @@ -477,15 +468,6 @@ const char* DelegatedRendererLayerImpl::LayerTypeAsString() const { return "cc::DelegatedRendererLayerImpl"; } -void DelegatedRendererLayerImpl::CreateChildIdIfNeeded() { - if (child_id_) - return; - - ResourceProvider* resource_provider = layer_tree_impl()->resource_provider(); - child_id_ = resource_provider->CreateChild(); - own_child_id_ = true; -} - void DelegatedRendererLayerImpl::ClearChildId() { if (!child_id_) return; diff --git a/cc/layers/delegated_renderer_layer_impl.h b/cc/layers/delegated_renderer_layer_impl.h index 5f55050..7d774f1 100644 --- a/cc/layers/delegated_renderer_layer_impl.h +++ b/cc/layers/delegated_renderer_layer_impl.h @@ -40,11 +40,14 @@ class CC_EXPORT DelegatedRendererLayerImpl : public LayerImpl { void AppendContributingRenderPasses(RenderPassSink* render_pass_sink); + // Creates an ID with the resource provider for the child renderer + // that will be sending quads to the layer. Registers the callback to + // inform when resources are no longer in use. + void CreateChildIdIfNeeded(const ReturnCallback& return_callback); + void SetFrameData(scoped_ptr<DelegatedFrameData> frame_data, gfx::RectF damage_in_frame); - void CollectUnusedResources(ReturnedResourceArray* resources_for_ack); - void SetDisplaySize(gfx::Size size); protected: @@ -54,14 +57,11 @@ class CC_EXPORT DelegatedRendererLayerImpl : public LayerImpl { const ScopedPtrVector<RenderPass>& RenderPassesInDrawOrderForTesting() const { return render_passes_in_draw_order_; } - const ResourceProvider::ResourceIdSet& ResourcesForTesting() const { + const ResourceProvider::ResourceIdArray& ResourcesForTesting() const { return resources_; } private: - // Creates an ID with the resource provider for the child renderer - // that will be sending quads to the layer. - void CreateChildIdIfNeeded(); void ClearChildId(); void AppendRainbowDebugBorder(QuadSink* quad_sink, @@ -92,7 +92,7 @@ class CC_EXPORT DelegatedRendererLayerImpl : public LayerImpl { bool have_render_passes_to_push_; ScopedPtrVector<RenderPass> render_passes_in_draw_order_; base::hash_map<RenderPass::Id, int> render_passes_index_by_id_; - ResourceProvider::ResourceIdSet resources_; + ResourceProvider::ResourceIdArray resources_; gfx::Size display_size_; int child_id_; diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc index e1cbba5..6ee8be0 100644 --- a/cc/resources/resource_provider.cc +++ b/cc/resources/resource_provider.cc @@ -86,7 +86,8 @@ class ScopedSetActiveTexture { } // namespace ResourceProvider::Resource::Resource() - : gl_id(0), + : child_id(0), + gl_id(0), gl_pixel_buffer_id(0), gl_upload_query_id(0), pixels(NULL), @@ -115,15 +116,15 @@ ResourceProvider::Resource::Resource() ResourceProvider::Resource::~Resource() {} -ResourceProvider::Resource::Resource( - unsigned texture_id, - gfx::Size size, - GLenum filter, - GLenum texture_pool, - GLint wrap_mode, - TextureUsageHint hint, - ResourceFormat format) - : gl_id(texture_id), +ResourceProvider::Resource::Resource(unsigned texture_id, + gfx::Size size, + GLenum filter, + GLenum texture_pool, + GLint wrap_mode, + TextureUsageHint hint, + ResourceFormat format) + : child_id(0), + gl_id(texture_id), gl_pixel_buffer_id(0), gl_upload_query_id(0), pixels(NULL), @@ -152,12 +153,12 @@ ResourceProvider::Resource::Resource( DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT); } -ResourceProvider::Resource::Resource( - uint8_t* pixels, - gfx::Size size, - GLenum filter, - GLint wrap_mode) - : gl_id(0), +ResourceProvider::Resource::Resource(uint8_t* pixels, + gfx::Size size, + GLenum filter, + GLint wrap_mode) + : child_id(0), + gl_id(0), gl_pixel_buffer_id(0), gl_upload_query_id(0), pixels(pixels), @@ -215,6 +216,8 @@ scoped_ptr<ResourceProvider> ResourceProvider::Create( } ResourceProvider::~ResourceProvider() { + while (!children_.empty()) + DestroyChild(children_.begin()->first); while (!resources_.empty()) DeleteResourceInternal(resources_.begin(), ForShutdown); @@ -810,9 +813,12 @@ void ResourceProvider::CleanUpGLIfNeeded() { Finish(); } -int ResourceProvider::CreateChild() { +int ResourceProvider::CreateChild(const ReturnCallback& return_callback) { DCHECK(thread_checker_.CalledOnValidThread()); + Child child_info; + child_info.return_callback = return_callback; + int child = next_child_++; children_[child] = child_info; return child; @@ -820,20 +826,26 @@ int ResourceProvider::CreateChild() { void ResourceProvider::DestroyChild(int child_id) { DCHECK(thread_checker_.CalledOnValidThread()); + ChildMap::iterator it = children_.find(child_id); DCHECK(it != children_.end()); Child& child = it->second; + + ResourceIdArray resources_for_child; + for (ResourceIdMap::iterator child_it = child.child_to_parent_map.begin(); child_it != child.child_to_parent_map.end(); ++child_it) { ResourceId id = child_it->second; - // We're abandoning this resource, it will not get recycled. - // crbug.com/224062 - ResourceMap::iterator resource_it = resources_.find(id); - CHECK(resource_it != resources_.end()); - resource_it->second.imported_count = 0; - DeleteResource(id); + resources_for_child.push_back(id); } + + // If the child is going away, don't consider any resources in use. + child.in_use_resources.clear(); + + DeleteAndReturnUnusedResourcesToChild( + &child, ForShutdown, resources_for_child); + children_.erase(it); } @@ -875,62 +887,6 @@ void ResourceProvider::PrepareSendToParent(const ResourceIdArray& resources, } } -void ResourceProvider::PrepareSendReturnsToChild( - int child, - const ResourceIdArray& resources, - ReturnedResourceArray* list) { - DCHECK(thread_checker_.CalledOnValidThread()); - WebGraphicsContext3D* context3d = Context3d(); - if (!context3d || !context3d->makeContextCurrent()) { - // TODO(skaslev): Implement this path for software compositing. - return; - } - Child& child_info = children_.find(child)->second; - bool need_sync_point = false; - for (ResourceIdArray::const_iterator it = resources.begin(); - it != resources.end(); ++it) { - Resource* resource = GetResource(*it); - DCHECK(!resource->locked_for_write); - DCHECK(!resource->lock_for_read_count); - DCHECK(child_info.parent_to_child_map.find(*it) != - child_info.parent_to_child_map.end()); - - if (resource->filter != resource->original_filter) { - DCHECK(resource->target); - DCHECK(resource->gl_id); - - GLC(context3d, context3d->bindTexture(resource->target, resource->gl_id)); - GLC(context3d, context3d->texParameteri(resource->target, - GL_TEXTURE_MIN_FILTER, - resource->original_filter)); - GLC(context3d, context3d->texParameteri(resource->target, - GL_TEXTURE_MAG_FILTER, - resource->original_filter)); - } - - ReturnedResource returned; - returned.id = child_info.parent_to_child_map[*it]; - returned.sync_point = resource->mailbox.sync_point(); - if (!returned.sync_point) - need_sync_point = true; - returned.count = resource->imported_count; - list->push_back(returned); - - child_info.parent_to_child_map.erase(*it); - child_info.child_to_parent_map.erase(returned.id); - resources_[*it].imported_count = 0; - DeleteResource(*it); - } - if (need_sync_point) { - unsigned int sync_point = context3d->insertSyncPoint(); - for (ReturnedResourceArray::iterator it = list->begin(); - it != list->end(); ++it) { - if (!it->sync_point) - it->sync_point = sync_point; - } - } -} - void ResourceProvider::ReceiveFromChild( int child, const TransferableResourceArray& resources) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -962,26 +918,65 @@ void ResourceProvider::ReceiveFromChild( GLC(context3d, context3d->bindTexture(GL_TEXTURE_2D, texture_id)); GLC(context3d, context3d->consumeTextureCHROMIUM(GL_TEXTURE_2D, it->mailbox.name)); - - ResourceId id = next_id_++; - Resource resource( - texture_id, - it->size, - it->filter, - 0, - GL_CLAMP_TO_EDGE, - TextureUsageAny, - it->format); + ResourceId local_id = next_id_++; + Resource resource(texture_id, + it->size, + it->filter, + 0, + GL_CLAMP_TO_EDGE, + TextureUsageAny, + it->format); resource.mailbox.SetName(it->mailbox); + resource.child_id = child; // Don't allocate a texture for a child. resource.allocated = true; resource.imported_count = 1; - resources_[id] = resource; - child_info.parent_to_child_map[id] = it->id; - child_info.child_to_parent_map[it->id] = id; + resources_[local_id] = resource; + child_info.parent_to_child_map[local_id] = it->id; + child_info.child_to_parent_map[it->id] = local_id; } } +void ResourceProvider::DeclareUsedResourcesFromChild( + int child, + const ResourceIdArray& resources_from_child) { + DCHECK(thread_checker_.CalledOnValidThread()); + + Child& child_info = children_.find(child)->second; + child_info.in_use_resources.clear(); + + for (size_t i = 0; i < resources_from_child.size(); ++i) { + ResourceIdMap::iterator it = + child_info.child_to_parent_map.find(resources_from_child[i]); + DCHECK(it != child_info.child_to_parent_map.end()); + + ResourceId local_id = it->second; + child_info.in_use_resources.insert(local_id); + } + + ResourceIdArray unused; + for (ResourceIdMap::iterator it = child_info.child_to_parent_map.begin(); + it != child_info.child_to_parent_map.end(); + ++it) { + ResourceId local_id = it->second; + bool resource_is_in_use = child_info.in_use_resources.count(local_id) > 0; + if (!resource_is_in_use) + unused.push_back(local_id); + } + DeleteAndReturnUnusedResourcesToChild(&child_info, Normal, unused); +} + +// static +bool ResourceProvider::CompareResourceMapIteratorsByChildId( + const std::pair<ReturnedResource, ResourceMap::iterator>& a, + const std::pair<ReturnedResource, ResourceMap::iterator>& b) { + const ResourceMap::iterator& a_it = a.second; + const ResourceMap::iterator& b_it = b.second; + const Resource& a_resource = a_it->second; + const Resource& b_resource = b_it->second; + return a_resource.child_id < b_resource.child_id; +} + void ResourceProvider::ReceiveReturnsFromParent( const ReturnedResourceArray& resources) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -990,25 +985,83 @@ void ResourceProvider::ReceiveReturnsFromParent( // TODO(skaslev): Implement this path for software compositing. return; } + + int child_id = 0; + Child* child_info = NULL; + ResourceIdArray resources_for_child; + + std::vector<std::pair<ReturnedResource, ResourceMap::iterator> > + sorted_resources; + for (ReturnedResourceArray::const_iterator it = resources.begin(); it != resources.end(); ++it) { - ResourceMap::iterator map_iterator = resources_.find(it->id); - DCHECK(map_iterator != resources_.end()); + ResourceId local_id = it->id; + ResourceMap::iterator map_iterator = resources_.find(local_id); + + // Resource was already lost (e.g. it belonged to a child that was + // destroyed). + if (map_iterator == resources_.end()) + continue; + + sorted_resources.push_back( + std::pair<ReturnedResource, ResourceMap::iterator>(*it, map_iterator)); + } + + std::sort(sorted_resources.begin(), + sorted_resources.end(), + CompareResourceMapIteratorsByChildId); + + for (size_t i = 0; i < sorted_resources.size(); ++i) { + ReturnedResource& returned = sorted_resources[i].first; + ResourceMap::iterator& map_iterator = sorted_resources[i].second; + ResourceId local_id = map_iterator->first; Resource* resource = &map_iterator->second; - CHECK_GE(resource->exported_count, it->count); - resource->exported_count -= it->count; + + CHECK_GE(resource->exported_count, returned.count); + resource->exported_count -= returned.count; if (resource->exported_count) continue; + if (resource->gl_id) { - if (it->sync_point) - GLC(context3d, context3d->waitSyncPoint(it->sync_point)); + if (returned.sync_point) + GLC(context3d, context3d->waitSyncPoint(returned.sync_point)); } else { resource->mailbox = - TextureMailbox(resource->mailbox.name(), it->sync_point); + TextureMailbox(resource->mailbox.name(), returned.sync_point); } - if (resource->marked_for_deletion) + + if (!resource->marked_for_deletion) + continue; + + if (!resource->child_id) { + // The resource belongs to this ResourceProvider, so it can be destroyed. DeleteResourceInternal(map_iterator, Normal); + continue; + } + + // Delete the resource and return it to the child it came from one. + if (resource->child_id != child_id) { + ChildMap::iterator child_it = children_.find(resource->child_id); + DCHECK(child_it != children_.end()); + + if (child_id) { + DCHECK_NE(resources_for_child.size(), 0u); + DeleteAndReturnUnusedResourcesToChild( + child_info, Normal, resources_for_child); + resources_for_child.clear(); + } + + child_info = &child_it->second; + child_id = resource->child_id; + } + resources_for_child.push_back(local_id); + } + + if (child_id) { + DCHECK_NE(resources_for_child.size(), 0u); + DeleteAndReturnUnusedResourcesToChild( + child_info, Normal, resources_for_child); } } @@ -1046,6 +1099,93 @@ void ResourceProvider::TransferResource(WebGraphicsContext3D* context, } } +void ResourceProvider::DeleteAndReturnUnusedResourcesToChild( + Child* child_info, + DeleteStyle style, + const ResourceIdArray& unused) { + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(child_info); + + if (unused.empty()) + return; + + WebGraphicsContext3D* context3d = Context3d(); + if (!context3d || !context3d->makeContextCurrent()) { + // TODO(skaslev): Implement this path for software compositing. + return; + } + + ReturnedResourceArray to_return; + + bool need_sync_point = false; + for (size_t i = 0; i < unused.size(); ++i) { + ResourceId local_id = unused[i]; + + ResourceMap::iterator it = resources_.find(local_id); + CHECK(it != resources_.end()); + Resource& resource = it->second; + + DCHECK(!resource.locked_for_write); + DCHECK(!resource.lock_for_read_count); + DCHECK_EQ(0u, child_info->in_use_resources.count(local_id)); + DCHECK(child_info->parent_to_child_map.count(local_id)); + + ResourceId child_id = child_info->parent_to_child_map[local_id]; + DCHECK(child_info->child_to_parent_map.count(child_id)); + + // TODO(danakj): bool is_lost = false; + if (resource.exported_count > 0) { + if (style != ForShutdown) { + // Defer this until we receive the resource back from the parent. + resource.marked_for_deletion = true; + continue; + } + + // We still have an exported_count, so we'll have to lose it. + // TODO(danakj): is_lost = true; + } + + if (resource.filter != resource.original_filter) { + DCHECK(resource.target); + DCHECK(resource.gl_id); + + GLC(context3d, context3d->bindTexture(resource.target, resource.gl_id)); + GLC(context3d, + context3d->texParameteri(resource.target, + GL_TEXTURE_MIN_FILTER, + resource.original_filter)); + GLC(context3d, + context3d->texParameteri(resource.target, + GL_TEXTURE_MAG_FILTER, + resource.original_filter)); + } + + ReturnedResource returned; + returned.id = child_id; + returned.sync_point = resource.mailbox.sync_point(); + if (!returned.sync_point) + need_sync_point = true; + returned.count = resource.imported_count; + // TODO(danakj): Save the |is_lost| bit. + to_return.push_back(returned); + + child_info->parent_to_child_map.erase(local_id); + child_info->child_to_parent_map.erase(child_id); + resource.imported_count = 0; + DeleteResourceInternal(it, style); + } + if (need_sync_point) { + unsigned int sync_point = context3d->insertSyncPoint(); + for (size_t i = 0; i < to_return.size(); ++i) { + if (!to_return[i].sync_point) + to_return[i].sync_point = sync_point; + } + } + + if (!to_return.empty()) + child_info->return_callback.Run(to_return); +} + void ResourceProvider::AcquirePixelBuffer(ResourceId id) { Resource* resource = GetResource(id); DCHECK(!resource->external); diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h index e73ea15..0a70b02 100644 --- a/cc/resources/resource_provider.h +++ b/cc/resources/resource_provider.h @@ -8,6 +8,7 @@ #include <deque> #include <set> #include <string> +#include <utility> #include <vector> #include "base/basictypes.h" @@ -20,6 +21,7 @@ #include "cc/output/output_surface.h" #include "cc/resources/release_callback.h" #include "cc/resources/resource_format.h" +#include "cc/resources/return_callback.h" #include "cc/resources/single_release_callback.h" #include "cc/resources/texture_mailbox.h" #include "cc/resources/transferable_resource.h" @@ -144,7 +146,7 @@ class CC_EXPORT ResourceProvider { bool ShallowFlushIfSupported(); // Creates accounting for a child. Returns a child ID. - int CreateChild(); + int CreateChild(const ReturnCallback& return_callback); // Destroys accounting for the child, deleting all accounted resources. void DestroyChild(int child); @@ -159,23 +161,25 @@ class CC_EXPORT ResourceProvider { void PrepareSendToParent(const ResourceIdArray& resources, TransferableResourceArray* transferable_resources); - // Prepares resources to be transfered back to the child, moving them to - // mailboxes and serializing meta-data into TransferableResources. - // Resources are removed from the ResourceProvider. Note: the resource IDs - // passed are in the parent namespace and will be translated to the child - // namespace when returned. - void PrepareSendReturnsToChild(int child, - const ResourceIdArray& resources, - ReturnedResourceArray* returned_resources); - // Receives resources from a child, moving them from mailboxes. Resource IDs // passed are in the child namespace, and will be translated to the parent // namespace, added to the child->parent map. + // This adds the resources to the working set in the ResourceProvider without + // declaring which resources are in use. Use DeclareUsedResourcesFromChild + // after calling this method to do that. All calls to ReceiveFromChild should + // be followed by a DeclareUsedResourcesFromChild. // NOTE: if the sync_point is set on any TransferableResource, this will // wait on it. void ReceiveFromChild( int child, const TransferableResourceArray& transferable_resources); + // Once a set of resources have been received, they may or may not be used. + // This declares what set of resources are currently in use from the child, + // releasing any other resources back to the child. + void DeclareUsedResourcesFromChild( + int child, + const ResourceIdArray& resources_from_child); + // Receives resources from the parent, moving them from mailboxes. Resource // IDs passed are in the child namespace. // NOTE: if the sync_point is set on any TransferableResource, this will @@ -359,6 +363,7 @@ class CC_EXPORT ResourceProvider { GLenum filter, GLint wrap_mode); + int child_id; unsigned gl_id; // Pixel buffer used for set pixels without unnecessary copying. unsigned gl_pixel_buffer_id; @@ -392,12 +397,19 @@ class CC_EXPORT ResourceProvider { ResourceFormat format; }; typedef base::hash_map<ResourceId, Resource> ResourceMap; + + static bool CompareResourceMapIteratorsByChildId( + const std::pair<ReturnedResource, ResourceMap::iterator>& a, + const std::pair<ReturnedResource, ResourceMap::iterator>& b); + struct Child { Child(); ~Child(); ResourceIdMap child_to_parent_map; ResourceIdMap parent_to_child_map; + ReturnCallback return_callback; + ResourceIdSet in_use_resources; }; typedef base::hash_map<int, Child> ChildMap; @@ -428,6 +440,9 @@ class CC_EXPORT ResourceProvider { ForShutdown, }; void DeleteResourceInternal(ResourceMap::iterator it, DeleteStyle style); + void DeleteAndReturnUnusedResourcesToChild(Child* child_info, + DeleteStyle style, + const ResourceIdArray& unused); void LazyCreate(Resource* resource); void LazyAllocate(Resource* resource); diff --git a/cc/resources/resource_provider_unittest.cc b/cc/resources/resource_provider_unittest.cc index 7df9412..e3e14c1 100644 --- a/cc/resources/resource_provider_unittest.cc +++ b/cc/resources/resource_provider_unittest.cc @@ -424,9 +424,18 @@ class ResourceProviderTest output_surface_.get(), 0, false); } + static void CollectResources(ReturnedResourceArray* array, + const ReturnedResourceArray& returned) { + array->insert(array->end(), returned.begin(), returned.end()); + } + + static ReturnCallback GetReturnCallback(ReturnedResourceArray* array) { + return base::Bind(&ResourceProviderTest::CollectResources, array); + } + static void SetResourceFilter(ResourceProvider* resource_provider, - ResourceProvider::ResourceId id, - WGC3Denum filter) { + ResourceProvider::ResourceId id, + WGC3Denum filter) { ResourceProvider::ScopedSamplerGL sampler( resource_provider, id, GL_TEXTURE_2D, filter); } @@ -578,7 +587,9 @@ TEST_P(ResourceProviderTest, TransferResources) { uint8_t data2[4] = { 5, 5, 5, 5 }; child_resource_provider->SetPixels(id2, data2, rect, rect, gfx::Vector2d()); - int child_id = resource_provider_->CreateChild(); + ReturnedResourceArray returned_to_child; + int child_id = + resource_provider_->CreateChild(GetReturnCallback(&returned_to_child)); { // Transfer some resources to the parent. ResourceProvider::ResourceIdArray resource_ids_to_transfer; @@ -593,6 +604,8 @@ TEST_P(ResourceProviderTest, TransferResources) { EXPECT_TRUE(child_resource_provider->InUseByConsumer(id1)); EXPECT_TRUE(child_resource_provider->InUseByConsumer(id2)); resource_provider_->ReceiveFromChild(child_id, list); + resource_provider_->DeclareUsedResourcesFromChild(child_id, + resource_ids_to_transfer); } EXPECT_EQ(2u, resource_provider_->num_resources()); @@ -631,17 +644,18 @@ TEST_P(ResourceProviderTest, TransferResources) { EXPECT_TRUE(child_resource_provider->InUseByConsumer(id1)); } { - // Transfer resources back from the parent to the child. - ResourceProvider::ResourceIdArray resource_ids_to_transfer; - resource_ids_to_transfer.push_back(mapped_id1); - resource_ids_to_transfer.push_back(mapped_id2); - ReturnedResourceArray list; - resource_provider_->PrepareSendReturnsToChild( - child_id, resource_ids_to_transfer, &list); - ASSERT_EQ(2u, list.size()); - EXPECT_NE(0u, list[0].sync_point); - EXPECT_NE(0u, list[1].sync_point); - child_resource_provider->ReceiveReturnsFromParent(list); + EXPECT_EQ(0u, returned_to_child.size()); + + // Transfer resources back from the parent to the child. Set no resources as + // being in use. + ResourceProvider::ResourceIdArray no_resources; + resource_provider_->DeclareUsedResourcesFromChild(child_id, no_resources); + + ASSERT_EQ(2u, returned_to_child.size()); + EXPECT_NE(0u, returned_to_child[0].sync_point); + EXPECT_NE(0u, returned_to_child[1].sync_point); + child_resource_provider->ReceiveReturnsFromParent(returned_to_child); + returned_to_child.clear(); } EXPECT_FALSE(child_resource_provider->InUseByConsumer(id1)); EXPECT_FALSE(child_resource_provider->InUseByConsumer(id2)); @@ -674,11 +688,231 @@ TEST_P(ResourceProviderTest, TransferResources) { EXPECT_TRUE(child_resource_provider->InUseByConsumer(id1)); EXPECT_TRUE(child_resource_provider->InUseByConsumer(id2)); resource_provider_->ReceiveFromChild(child_id, list); + resource_provider_->DeclareUsedResourcesFromChild(child_id, + resource_ids_to_transfer); } + EXPECT_EQ(0u, returned_to_child.size()); + EXPECT_EQ(2u, resource_provider_->num_resources()); resource_provider_->DestroyChild(child_id); EXPECT_EQ(0u, resource_provider_->num_resources()); + + ASSERT_EQ(2u, returned_to_child.size()); + // TODO(danakj): Verify the resources are not marked as lost. + EXPECT_NE(0u, returned_to_child[0].sync_point); + EXPECT_NE(0u, returned_to_child[1].sync_point); +} + +TEST_P(ResourceProviderTest, DeleteExportedResources) { + // Resource transfer is only supported with GL textures for now. + if (GetParam() != ResourceProvider::GLTexture) + return; + + scoped_ptr<ResourceProviderContext> child_context_owned( + ResourceProviderContext::Create(shared_data_.get())); + + FakeOutputSurfaceClient child_output_surface_client; + scoped_ptr<OutputSurface> child_output_surface(FakeOutputSurface::Create3d( + child_context_owned.PassAs<TestWebGraphicsContext3D>())); + CHECK(child_output_surface->BindToClient(&child_output_surface_client)); + + scoped_ptr<ResourceProvider> child_resource_provider( + ResourceProvider::Create(child_output_surface.get(), 0, false)); + + gfx::Size size(1, 1); + ResourceFormat format = RGBA_8888; + size_t pixel_size = TextureSize(size, format); + ASSERT_EQ(4U, pixel_size); + + ResourceProvider::ResourceId id1 = child_resource_provider->CreateResource( + size, GL_CLAMP_TO_EDGE, ResourceProvider::TextureUsageAny, format); + uint8_t data1[4] = {1, 2, 3, 4}; + gfx::Rect rect(size); + child_resource_provider->SetPixels(id1, data1, rect, rect, gfx::Vector2d()); + + ResourceProvider::ResourceId id2 = child_resource_provider->CreateResource( + size, GL_CLAMP_TO_EDGE, ResourceProvider::TextureUsageAny, format); + uint8_t data2[4] = {5, 5, 5, 5}; + child_resource_provider->SetPixels(id2, data2, rect, rect, gfx::Vector2d()); + + ReturnedResourceArray returned_to_child; + int child_id = + resource_provider_->CreateChild(GetReturnCallback(&returned_to_child)); + { + // Transfer some resources to the parent. + ResourceProvider::ResourceIdArray resource_ids_to_transfer; + resource_ids_to_transfer.push_back(id1); + resource_ids_to_transfer.push_back(id2); + TransferableResourceArray list; + child_resource_provider->PrepareSendToParent(resource_ids_to_transfer, + &list); + ASSERT_EQ(2u, list.size()); + EXPECT_NE(0u, list[0].sync_point); + EXPECT_NE(0u, list[1].sync_point); + EXPECT_TRUE(child_resource_provider->InUseByConsumer(id1)); + EXPECT_TRUE(child_resource_provider->InUseByConsumer(id2)); + resource_provider_->ReceiveFromChild(child_id, list); + resource_provider_->DeclareUsedResourcesFromChild(child_id, + resource_ids_to_transfer); + } + + EXPECT_EQ(2u, resource_provider_->num_resources()); + ResourceProvider::ResourceIdMap resource_map = + resource_provider_->GetChildToParentMap(child_id); + ResourceProvider::ResourceId mapped_id1 = resource_map[id1]; + ResourceProvider::ResourceId mapped_id2 = resource_map[id2]; + EXPECT_NE(0u, mapped_id1); + EXPECT_NE(0u, mapped_id2); + EXPECT_FALSE(resource_provider_->InUseByConsumer(id1)); + EXPECT_FALSE(resource_provider_->InUseByConsumer(id2)); + + { + // The parent transfers the resources to the grandparent. + ResourceProvider::ResourceIdArray resource_ids_to_transfer; + resource_ids_to_transfer.push_back(mapped_id1); + resource_ids_to_transfer.push_back(mapped_id2); + TransferableResourceArray list; + resource_provider_->PrepareSendToParent(resource_ids_to_transfer, &list); + + ASSERT_EQ(2u, list.size()); + EXPECT_NE(0u, list[0].sync_point); + EXPECT_NE(0u, list[1].sync_point); + EXPECT_TRUE(resource_provider_->InUseByConsumer(id1)); + EXPECT_TRUE(resource_provider_->InUseByConsumer(id2)); + + // Release the resource in the parent. Set no resources as being in use. The + // resources are exported so that can't be transferred back yet. + ResourceProvider::ResourceIdArray no_resources; + resource_provider_->DeclareUsedResourcesFromChild(child_id, no_resources); + + EXPECT_EQ(0u, returned_to_child.size()); + EXPECT_EQ(2u, resource_provider_->num_resources()); + + // Return the resources from the grandparent to the parent. They should be + // returned to the child then. + EXPECT_EQ(2u, list.size()); + EXPECT_EQ(mapped_id1, list[0].id); + EXPECT_EQ(mapped_id2, list[1].id); + ReturnedResourceArray returned; + TransferableResource::ReturnResources(list, &returned); + resource_provider_->ReceiveReturnsFromParent(returned); + + EXPECT_EQ(0u, resource_provider_->num_resources()); + ASSERT_EQ(2u, returned_to_child.size()); + // TODO(danakj): Verify the resources are not marked as lost. + EXPECT_NE(0u, returned_to_child[0].sync_point); + EXPECT_NE(0u, returned_to_child[1].sync_point); + } +} + +TEST_P(ResourceProviderTest, DestroyChildWithExportedResources) { + // Resource transfer is only supported with GL textures for now. + if (GetParam() != ResourceProvider::GLTexture) + return; + + scoped_ptr<ResourceProviderContext> child_context_owned( + ResourceProviderContext::Create(shared_data_.get())); + + FakeOutputSurfaceClient child_output_surface_client; + scoped_ptr<OutputSurface> child_output_surface(FakeOutputSurface::Create3d( + child_context_owned.PassAs<TestWebGraphicsContext3D>())); + CHECK(child_output_surface->BindToClient(&child_output_surface_client)); + + scoped_ptr<ResourceProvider> child_resource_provider( + ResourceProvider::Create(child_output_surface.get(), 0, false)); + + gfx::Size size(1, 1); + ResourceFormat format = RGBA_8888; + size_t pixel_size = TextureSize(size, format); + ASSERT_EQ(4U, pixel_size); + + ResourceProvider::ResourceId id1 = child_resource_provider->CreateResource( + size, GL_CLAMP_TO_EDGE, ResourceProvider::TextureUsageAny, format); + uint8_t data1[4] = {1, 2, 3, 4}; + gfx::Rect rect(size); + child_resource_provider->SetPixels(id1, data1, rect, rect, gfx::Vector2d()); + + ResourceProvider::ResourceId id2 = child_resource_provider->CreateResource( + size, GL_CLAMP_TO_EDGE, ResourceProvider::TextureUsageAny, format); + uint8_t data2[4] = {5, 5, 5, 5}; + child_resource_provider->SetPixels(id2, data2, rect, rect, gfx::Vector2d()); + + ReturnedResourceArray returned_to_child; + int child_id = + resource_provider_->CreateChild(GetReturnCallback(&returned_to_child)); + { + // Transfer some resources to the parent. + ResourceProvider::ResourceIdArray resource_ids_to_transfer; + resource_ids_to_transfer.push_back(id1); + resource_ids_to_transfer.push_back(id2); + TransferableResourceArray list; + child_resource_provider->PrepareSendToParent(resource_ids_to_transfer, + &list); + ASSERT_EQ(2u, list.size()); + EXPECT_NE(0u, list[0].sync_point); + EXPECT_NE(0u, list[1].sync_point); + EXPECT_TRUE(child_resource_provider->InUseByConsumer(id1)); + EXPECT_TRUE(child_resource_provider->InUseByConsumer(id2)); + resource_provider_->ReceiveFromChild(child_id, list); + resource_provider_->DeclareUsedResourcesFromChild(child_id, + resource_ids_to_transfer); + } + + EXPECT_EQ(2u, resource_provider_->num_resources()); + ResourceProvider::ResourceIdMap resource_map = + resource_provider_->GetChildToParentMap(child_id); + ResourceProvider::ResourceId mapped_id1 = resource_map[id1]; + ResourceProvider::ResourceId mapped_id2 = resource_map[id2]; + EXPECT_NE(0u, mapped_id1); + EXPECT_NE(0u, mapped_id2); + EXPECT_FALSE(resource_provider_->InUseByConsumer(id1)); + EXPECT_FALSE(resource_provider_->InUseByConsumer(id2)); + + { + // The parent transfers the resources to the grandparent. + ResourceProvider::ResourceIdArray resource_ids_to_transfer; + resource_ids_to_transfer.push_back(mapped_id1); + resource_ids_to_transfer.push_back(mapped_id2); + TransferableResourceArray list; + resource_provider_->PrepareSendToParent(resource_ids_to_transfer, &list); + + ASSERT_EQ(2u, list.size()); + EXPECT_NE(0u, list[0].sync_point); + EXPECT_NE(0u, list[1].sync_point); + EXPECT_TRUE(resource_provider_->InUseByConsumer(id1)); + EXPECT_TRUE(resource_provider_->InUseByConsumer(id2)); + + // Release the resource in the parent. Set no resources as being in use. The + // resources are exported so that can't be transferred back yet. + ResourceProvider::ResourceIdArray no_resources; + resource_provider_->DeclareUsedResourcesFromChild(child_id, no_resources); + + // Destroy the child, the resources should be returned immediately from the + // parent and marked as lost. + EXPECT_EQ(0u, returned_to_child.size()); + EXPECT_EQ(2u, resource_provider_->num_resources()); + + resource_provider_->DestroyChild(child_id); + + EXPECT_EQ(0u, resource_provider_->num_resources()); + ASSERT_EQ(2u, returned_to_child.size()); + // TODO(danakj): Verify the resources are marked as lost. + EXPECT_NE(0u, returned_to_child[0].sync_point); + EXPECT_NE(0u, returned_to_child[1].sync_point); + returned_to_child.clear(); + + // Return the resources from the grandparent to the parent. They should be + // dropped on the floor since they were already returned to the child. + EXPECT_EQ(2u, list.size()); + EXPECT_EQ(mapped_id1, list[0].id); + EXPECT_EQ(mapped_id2, list[1].id); + ReturnedResourceArray returned; + TransferableResource::ReturnResources(list, &returned); + resource_provider_->ReceiveReturnsFromParent(returned); + + EXPECT_EQ(0u, returned_to_child.size()); + } } TEST_P(ResourceProviderTest, DeleteTransferredResources) { @@ -709,7 +943,9 @@ TEST_P(ResourceProviderTest, DeleteTransferredResources) { gfx::Rect rect(size); child_resource_provider->SetPixels(id, data, rect, rect, gfx::Vector2d()); - int child_id = resource_provider_->CreateChild(); + ReturnedResourceArray returned_to_child; + int child_id = + resource_provider_->CreateChild(GetReturnCallback(&returned_to_child)); { // Transfer some resource to the parent. ResourceProvider::ResourceIdArray resource_ids_to_transfer; @@ -721,25 +957,24 @@ TEST_P(ResourceProviderTest, DeleteTransferredResources) { EXPECT_NE(0u, list[0].sync_point); EXPECT_TRUE(child_resource_provider->InUseByConsumer(id)); resource_provider_->ReceiveFromChild(child_id, list); + resource_provider_->DeclareUsedResourcesFromChild(child_id, + resource_ids_to_transfer); } // Delete textures in the child, while they are transfered. child_resource_provider->DeleteResource(id); EXPECT_EQ(1u, child_resource_provider->num_resources()); { - // Transfer resources back from the parent to the child. - ResourceProvider::ResourceIdMap resource_map = - resource_provider_->GetChildToParentMap(child_id); - ResourceProvider::ResourceId mapped_id = resource_map[id]; - EXPECT_NE(0u, mapped_id); - ResourceProvider::ResourceIdArray resource_ids_to_transfer; - resource_ids_to_transfer.push_back(mapped_id); - ReturnedResourceArray list; - resource_provider_->PrepareSendReturnsToChild( - child_id, resource_ids_to_transfer, &list); - ASSERT_EQ(1u, list.size()); - EXPECT_NE(0u, list[0].sync_point); - child_resource_provider->ReceiveReturnsFromParent(list); + EXPECT_EQ(0u, returned_to_child.size()); + + // Transfer resources back from the parent to the child. Set no resources as + // being in use. + ResourceProvider::ResourceIdArray no_resources; + resource_provider_->DeclareUsedResourcesFromChild(child_id, no_resources); + + ASSERT_EQ(1u, returned_to_child.size()); + EXPECT_NE(0u, returned_to_child[0].sync_point); + child_resource_provider->ReceiveReturnsFromParent(returned_to_child); } EXPECT_EQ(0u, child_resource_provider->num_resources()); } @@ -821,7 +1056,9 @@ class ResourceProviderTestTextureFilters : public ResourceProviderTest { SetResourceFilter(child_resource_provider.get(), id, child_filter); Mock::VerifyAndClearExpectations(child_context); - int child_id = parent_resource_provider->CreateChild(); + ReturnedResourceArray returned_to_child; + int child_id = parent_resource_provider->CreateChild( + GetReturnCallback(&returned_to_child)); { // Transfer some resource to the parent. ResourceProvider::ResourceIdArray resource_ids_to_transfer; @@ -844,6 +1081,10 @@ class ResourceProviderTestTextureFilters : public ResourceProviderTest { consumeTextureCHROMIUM(GL_TEXTURE_2D, _)); parent_resource_provider->ReceiveFromChild(child_id, list); Mock::VerifyAndClearExpectations(parent_context); + + parent_resource_provider->DeclareUsedResourcesFromChild( + child_id, resource_ids_to_transfer); + Mock::VerifyAndClearExpectations(parent_context); } ResourceProvider::ResourceIdMap resource_map = parent_resource_provider->GetChildToParentMap(child_id); @@ -872,19 +1113,19 @@ class ResourceProviderTestTextureFilters : public ResourceProviderTest { texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, child_filter)); { - // Transfer resources back from the parent to the child. - ResourceProvider::ResourceIdArray resource_ids_to_transfer; - resource_ids_to_transfer.push_back(mapped_id); - ReturnedResourceArray list; + EXPECT_EQ(0u, returned_to_child.size()); + // Transfer resources back from the parent to the child. Set no resources + // as being in use. + ResourceProvider::ResourceIdArray no_resources; EXPECT_CALL(*parent_context, insertSyncPoint()); + parent_resource_provider->DeclareUsedResourcesFromChild(child_id, + no_resources); + Mock::VerifyAndClearExpectations(parent_context); - parent_resource_provider->PrepareSendReturnsToChild( - child_id, resource_ids_to_transfer, &list); - ASSERT_EQ(1u, list.size()); - child_resource_provider->ReceiveReturnsFromParent(list); + ASSERT_EQ(1u, returned_to_child.size()); + child_resource_provider->ReceiveReturnsFromParent(returned_to_child); } - Mock::VerifyAndClearExpectations(parent_context); // The child remembers the texture filter is set to |child_filter|. EXPECT_CALL(*child_context, bindTexture(GL_TEXTURE_2D, texture_id)); diff --git a/cc/resources/return_callback.h b/cc/resources/return_callback.h new file mode 100644 index 0000000..d12254b --- /dev/null +++ b/cc/resources/return_callback.h @@ -0,0 +1,17 @@ +// Copyright 2013 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 CC_RESOURCES_RETURN_CALLBACK_H_ +#define CC_RESOURCES_RETURN_CALLBACK_H_ + +#include "base/callback.h" +#include "cc/resources/returned_resource.h" + +namespace cc { + +typedef base::Callback<void(const ReturnedResourceArray&)> ReturnCallback; + +} // namespace cc + +#endif // CC_RESOURCES_RETURN_CALLBACK_H_ diff --git a/cc/test/fake_delegated_renderer_layer_impl.cc b/cc/test/fake_delegated_renderer_layer_impl.cc index 782bc1c..1420ae2 100644 --- a/cc/test/fake_delegated_renderer_layer_impl.cc +++ b/cc/test/fake_delegated_renderer_layer_impl.cc @@ -32,6 +32,18 @@ static ResourceProvider::ResourceId AddResourceToFrame( return resource_id; } +ResourceProvider::ResourceIdSet FakeDelegatedRendererLayerImpl::Resources() + const { + ResourceProvider::ResourceIdSet set; + ResourceProvider::ResourceIdArray array; + array = ResourcesForTesting(); + for (size_t i = 0; i < array.size(); ++i) + set.insert(array[i]); + return set; +} + +void NoopReturnCallback(const ReturnedResourceArray& returned) {} + void FakeDelegatedRendererLayerImpl::SetFrameDataForRenderPasses( ScopedPtrVector<RenderPass>* pass_list) { scoped_ptr<DelegatedFrameData> delegated_frame(new DelegatedFrameData); @@ -46,9 +58,8 @@ void FakeDelegatedRendererLayerImpl::SetFrameDataForRenderPasses( pass->quad_list[j]->IterateResources(add_resource_to_frame_callback); } - ReturnedResourceArray resources_for_ack; + CreateChildIdIfNeeded(base::Bind(&NoopReturnCallback)); SetFrameData(delegated_frame.Pass(), gfx::RectF()); - CollectUnusedResources(&resources_for_ack); } } // namespace cc diff --git a/cc/test/fake_delegated_renderer_layer_impl.h b/cc/test/fake_delegated_renderer_layer_impl.h index a9ca0f9..a958baf 100644 --- a/cc/test/fake_delegated_renderer_layer_impl.h +++ b/cc/test/fake_delegated_renderer_layer_impl.h @@ -24,9 +24,7 @@ class FakeDelegatedRendererLayerImpl : public DelegatedRendererLayerImpl { const ScopedPtrVector<RenderPass>& RenderPassesInDrawOrder() const { return RenderPassesInDrawOrderForTesting(); } - const ResourceProvider::ResourceIdSet& Resources() const { - return ResourcesForTesting(); - } + ResourceProvider::ResourceIdSet Resources() const; void SetFrameDataForRenderPasses(ScopedPtrVector<RenderPass>* pass_list); diff --git a/cc/test/layer_tree_test.h b/cc/test/layer_tree_test.h index 6d44019..f3e0fec 100644 --- a/cc/test/layer_tree_test.h +++ b/cc/test/layer_tree_test.h @@ -152,6 +152,9 @@ class LayerTreeTest : public testing::Test, public TestHooks { return proxy()->ImplThreadTaskRunner() ? proxy()->ImplThreadTaskRunner() : main_task_runner_.get(); } + base::SingleThreadTaskRunner* MainThreadTaskRunner() { + return main_task_runner_.get(); + } Proxy* proxy() const { return layer_tree_host_ ? layer_tree_host_->proxy() : NULL; } diff --git a/cc/trees/layer_tree_host_unittest_delegated.cc b/cc/trees/layer_tree_host_unittest_delegated.cc index af79c84..012f6f8 100644 --- a/cc/trees/layer_tree_host_unittest_delegated.cc +++ b/cc/trees/layer_tree_host_unittest_delegated.cc @@ -7,6 +7,7 @@ #include <algorithm> #include "base/bind.h" +#include "base/location.h" #include "base/synchronization/waitable_event.h" #include "base/threading/thread.h" #include "base/time/time.h" @@ -765,8 +766,8 @@ class LayerTreeHostDelegatedTestMergeResources EXPECT_EQ(1u, map.count(555)); EXPECT_EQ(2u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); EndTest(); } @@ -850,11 +851,6 @@ class LayerTreeHostDelegatedTestReturnUnusedResources delegated_->SetFrameData(frame.Pass()); break; case 2: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 3: // All of the resources are in use. delegated_->TakeUnusedResourcesForChildCompositor(&resources); EXPECT_EQ(0u, resources.size()); @@ -862,16 +858,12 @@ class LayerTreeHostDelegatedTestReturnUnusedResources // Keep using 999 but stop using 555. frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1)); AddTextureQuad(frame.get(), 999); + AddTransferableResource(frame.get(), 999); AddTextureQuad(frame.get(), 444); AddTransferableResource(frame.get(), 444); delegated_->SetFrameData(frame.Pass()); break; - case 4: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 5: + case 3: // 555 is no longer in use. delegated_->TakeUnusedResourcesForChildCompositor(&resources); { @@ -883,16 +875,17 @@ class LayerTreeHostDelegatedTestReturnUnusedResources frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1)); delegated_->SetFrameData(frame.Pass()); break; - case 6: + case 4: // Postpone collecting resources for a frame. They should still be there // the next frame. layer_tree_host()->SetNeedsCommit(); return; - case 7: - // 444 and 999 are no longer in use. + case 5: + // 444 and 999 are no longer in use. We sent two refs to 999, so we + // should get two back. delegated_->TakeUnusedResourcesForChildCompositor(&resources); { - unsigned expected[] = {444, 999}; + unsigned expected[] = {444, 999, 999}; EXPECT_RESOURCES(expected, resources); } EndTest(); @@ -941,11 +934,6 @@ class LayerTreeHostDelegatedTestReusedResources delegated_->SetFrameData(frame.Pass()); break; case 2: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 3: // All of the resources are in use. delegated_->TakeUnusedResourcesForChildCompositor(&resources); EXPECT_EQ(0u, resources.size()); @@ -953,6 +941,7 @@ class LayerTreeHostDelegatedTestReusedResources // Keep using 999 but stop using 555 and 444. frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1)); AddTextureQuad(frame.get(), 999); + AddTransferableResource(frame.get(), 999); delegated_->SetFrameData(frame.Pass()); // Resource are not immediately released. @@ -962,19 +951,17 @@ class LayerTreeHostDelegatedTestReusedResources // Now using 555 and 444 again, but not 999. frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1)); AddTextureQuad(frame.get(), 555); + AddTransferableResource(frame.get(), 555); AddTextureQuad(frame.get(), 444); + AddTransferableResource(frame.get(), 444); delegated_->SetFrameData(frame.Pass()); break; - case 4: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 5: - // The 999 resource is the only unused one. + case 3: + // The 999 resource is the only unused one. Two references were sent, so + // two should be returned. delegated_->TakeUnusedResourcesForChildCompositor(&resources); { - unsigned expected[] = {999}; + unsigned expected[] = {999, 999}; EXPECT_RESOURCES(expected, resources); } EndTest(); @@ -1017,11 +1004,6 @@ class LayerTreeHostDelegatedTestFrameBeforeAck delegated_->SetFrameData(frame.Pass()); break; case 2: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 3: // All of the resources are in use. delegated_->TakeUnusedResourcesForChildCompositor(&resources); EXPECT_EQ(0u, resources.size()); @@ -1029,6 +1011,7 @@ class LayerTreeHostDelegatedTestFrameBeforeAck // Keep using 999 but stop using 555 and 444. frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1)); AddTextureQuad(frame.get(), 999); + AddTransferableResource(frame.get(), 999); delegated_->SetFrameData(frame.Pass()); // Resource are not immediately released. @@ -1037,20 +1020,15 @@ class LayerTreeHostDelegatedTestFrameBeforeAck // The parent compositor (this one) does a commit. break; - case 4: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 5: + case 3: delegated_->TakeUnusedResourcesForChildCompositor(&resources); { unsigned expected[] = {444, 555}; EXPECT_RESOURCES(expected, resources); } - // The child compositor sends a frame before receiving an for the - // second frame. It uses 999, 444, and 555 again. + // The child compositor sends a frame referring to resources not in the + // frame. frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1)); AddTextureQuad(frame.get(), 999); AddTextureQuad(frame.get(), 555); @@ -1061,7 +1039,7 @@ class LayerTreeHostDelegatedTestFrameBeforeAck } virtual void DidActivateTreeOnThread(LayerTreeHostImpl* host_impl) OVERRIDE { - if (host_impl->active_tree()->source_frame_number() != 5) + if (host_impl->active_tree()->source_frame_number() != 3) return; LayerImpl* root_impl = host_impl->active_tree()->root_layer(); @@ -1079,7 +1057,7 @@ class LayerTreeHostDelegatedTestFrameBeforeAck EXPECT_EQ(1u, map.count(999)); EXPECT_EQ(1u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); const RenderPass* pass = delegated_impl->RenderPassesInDrawOrder()[0]; EXPECT_EQ(1u, pass->quad_list.size()); @@ -1125,11 +1103,6 @@ class LayerTreeHostDelegatedTestFrameBeforeTakeResources delegated_->SetFrameData(frame.Pass()); break; case 2: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 3: // All of the resources are in use. delegated_->TakeUnusedResourcesForChildCompositor(&resources); EXPECT_EQ(0u, resources.size()); @@ -1146,12 +1119,7 @@ class LayerTreeHostDelegatedTestFrameBeforeTakeResources // The parent compositor (this one) does a commit. break; - case 4: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 5: + case 3: // The child compositor sends a frame before taking resources back // from the previous commit. This frame makes use of the resources 555 // and 444, which were just released during commit. @@ -1172,12 +1140,7 @@ class LayerTreeHostDelegatedTestFrameBeforeTakeResources EXPECT_RESOURCES(expected, resources); } break; - case 6: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 7: + case 4: delegated_->TakeUnusedResourcesForChildCompositor(&resources); EXPECT_EQ(0u, resources.size()); EndTest(); @@ -1186,7 +1149,7 @@ class LayerTreeHostDelegatedTestFrameBeforeTakeResources } virtual void DidActivateTreeOnThread(LayerTreeHostImpl* host_impl) OVERRIDE { - if (host_impl->active_tree()->source_frame_number() != 5) + if (host_impl->active_tree()->source_frame_number() != 3) return; LayerImpl* root_impl = host_impl->active_tree()->root_layer(); @@ -1205,9 +1168,9 @@ class LayerTreeHostDelegatedTestFrameBeforeTakeResources EXPECT_EQ(1u, map.count(444)); EXPECT_EQ(3u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(444)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); + EXPECT_EQ(1u, delegated_impl->Resources().count(444)); const RenderPass* pass = delegated_impl->RenderPassesInDrawOrder()[0]; EXPECT_EQ(3u, pass->quad_list.size()); @@ -1256,11 +1219,6 @@ class LayerTreeHostDelegatedTestBadFrame delegated_->SetFrameData(frame.Pass()); break; case 2: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 3: // All of the resources are in use. delegated_->TakeUnusedResourcesForChildCompositor(&resources); EXPECT_EQ(0u, resources.size()); @@ -1279,12 +1237,7 @@ class LayerTreeHostDelegatedTestBadFrame // The parent compositor (this one) does a commit. break; - case 4: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 5: + case 3: // The bad frame's resource is given back to the child compositor. delegated_->TakeUnusedResourcesForChildCompositor(&resources); { @@ -1297,12 +1250,7 @@ class LayerTreeHostDelegatedTestBadFrame AddTextureQuad(frame.get(), 999); delegated_->SetFrameData(frame.Pass()); break; - case 6: - // Retrieve unused resources to the main thread. - // TODO(danakj): Shouldn't need to commit to get resources. - layer_tree_host()->SetNeedsCommit(); - return; - case 7: + case 4: // The unused 555 from the last good frame is now released. delegated_->TakeUnusedResourcesForChildCompositor(&resources); { @@ -1339,8 +1287,8 @@ class LayerTreeHostDelegatedTestBadFrame EXPECT_EQ(1u, map.count(555)); EXPECT_EQ(2u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); const RenderPass* pass = delegated_impl->RenderPassesInDrawOrder()[0]; EXPECT_EQ(2u, pass->quad_list.size()); @@ -1352,15 +1300,15 @@ class LayerTreeHostDelegatedTestBadFrame EXPECT_EQ(map.find(555)->second, quad2->resource_id); break; } - case 3: { + case 2: { // We only keep resources from the last valid frame. EXPECT_EQ(2u, map.size()); EXPECT_EQ(1u, map.count(999)); EXPECT_EQ(1u, map.count(555)); EXPECT_EQ(2u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); // The bad frame is dropped though, we still have the frame with 999 and // 555 in it. @@ -1374,20 +1322,13 @@ class LayerTreeHostDelegatedTestBadFrame EXPECT_EQ(map.find(555)->second, quad2->resource_id); break; } - case 5: - // Resources given to our parent compositor will be returned now, but - // the DelegatedRendererLayerImpl doesn't know about it until the next - // commit. - // TODO(danakj): Shouldn't need a commit to return resources to the - // DelegatedRendererLayerImpl or to the main thread. - break; - case 6: { + case 3: { // We have the new good frame with just 999 in it. EXPECT_EQ(1u, map.size()); EXPECT_EQ(1u, map.count(999)); EXPECT_EQ(1u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); const RenderPass* pass = delegated_impl->RenderPassesInDrawOrder()[0]; EXPECT_EQ(1u, pass->quad_list.size()); @@ -1455,7 +1396,7 @@ class LayerTreeHostDelegatedTestUnnamedResource EXPECT_EQ(1u, map.count(555)); EXPECT_EQ(1u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); } virtual void AfterTest() OVERRIDE {} @@ -1470,7 +1411,7 @@ class LayerTreeHostDelegatedTestDontLeakResource PostSetNeedsCommitToMainThread(); } - virtual void DidCommit() OVERRIDE { + virtual void DidCommitAndDrawFrame() OVERRIDE { scoped_ptr<DelegatedFrameData> frame; ReturnedResourceArray resources; @@ -1504,11 +1445,6 @@ class LayerTreeHostDelegatedTestDontLeakResource delegated_->SetFrameData(frame.Pass()); break; case 3: - // The impl side will get back the resource at some point. - // TODO(piman): The test should work without this. - layer_tree_host()->SetNeedsCommit(); - break; - case 4: // The now unused resource 555 should be returned. resources.clear(); delegated_->TakeUnusedResourcesForChildCompositor(&resources); @@ -1538,7 +1474,7 @@ class LayerTreeHostDelegatedTestDontLeakResource EXPECT_EQ(1u, map.count(555)); EXPECT_EQ(1u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); } virtual void SwapBuffersOnThread(LayerTreeHostImpl* host_impl, @@ -1577,6 +1513,7 @@ class LayerTreeHostDelegatedTestResourceSentToParent // it present. frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1)); AddTextureQuad(frame.get(), 555); + AddTransferableResource(frame.get(), 555); delegated_->SetFrameData(frame.Pass()); break; case 3: @@ -1585,20 +1522,46 @@ class LayerTreeHostDelegatedTestResourceSentToParent EXPECT_EQ(0u, resources.size()); // The impl side will get back the resource at some point. - // TODO(danakj): The test should work without this. - layer_tree_host()->SetNeedsCommit(); + ImplThreadTaskRunner()->PostTask(FROM_HERE, + receive_resource_on_thread_); break; - case 4: - // 999 was returned from the grandparent and could be released. - delegated_->TakeUnusedResourcesForChildCompositor(&resources); - { - unsigned expected[] = {999}; - EXPECT_RESOURCES(expected, resources); - } + } + } - EndTest(); - break; + void ReceiveResourceOnThread(LayerTreeHostImpl* host_impl) { + LayerImpl* root_impl = host_impl->active_tree()->root_layer(); + FakeDelegatedRendererLayerImpl* delegated_impl = + static_cast<FakeDelegatedRendererLayerImpl*>(root_impl->children()[0]); + + const ResourceProvider::ResourceIdMap& map = + host_impl->resource_provider()->GetChildToParentMap( + delegated_impl->ChildId()); + + // Receive 999 back from the grandparent. + CompositorFrameAck ack; + output_surface()->ReturnResource(map.find(999)->second, &ack); + host_impl->ReclaimResources(&ack); + host_impl->OnSwapBuffersComplete(); + + // And then it should be released by the DelegatedRendererLayer. + MainThreadTaskRunner()->PostTask( + FROM_HERE, + base::Bind(&LayerTreeHostDelegatedTestResourceSentToParent:: + DidReceiveResourceOnMainThread, + base::Unretained(this))); + } + + void DidReceiveResourceOnMainThread() { + ReturnedResourceArray resources; + + // 999 was returned from the grandparent and could be released. + delegated_->TakeUnusedResourcesForChildCompositor(&resources); + { + unsigned expected[] = {999}; + EXPECT_RESOURCES(expected, resources); } + + EndTest(); } virtual void DidActivateTreeOnThread(LayerTreeHostImpl* host_impl) OVERRIDE { @@ -1620,8 +1583,8 @@ class LayerTreeHostDelegatedTestResourceSentToParent EXPECT_EQ(1u, map.count(555)); EXPECT_EQ(2u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); // The 999 resource will be sent to a grandparent compositor. break; @@ -1633,13 +1596,13 @@ class LayerTreeHostDelegatedTestResourceSentToParent // 999 is in the parent, so not held by delegated renderer layer. EXPECT_EQ(1u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); - // Receive 999 back from the grandparent. - CompositorFrameAck ack; - output_surface()->ReturnResource(map.find(999)->second, &ack); - host_impl->ReclaimResources(&ack); - host_impl->OnSwapBuffersComplete(); + receive_resource_on_thread_ = + base::Bind(&LayerTreeHostDelegatedTestResourceSentToParent:: + ReceiveResourceOnThread, + base::Unretained(this), + host_impl); break; } case 3: @@ -1655,7 +1618,7 @@ class LayerTreeHostDelegatedTestResourceSentToParent virtual void AfterTest() OVERRIDE {} - TransferableResource resource_in_grandparent; + base::Closure receive_resource_on_thread_; }; SINGLE_AND_MULTI_THREAD_DELEGATING_RENDERER_TEST_F( @@ -1753,16 +1716,16 @@ class LayerTreeHostDelegatedTestCommitWithoutTake EXPECT_EQ(1u, map.count(444)); EXPECT_EQ(3u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(444)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); + EXPECT_EQ(1u, delegated_impl->Resources().count(444)); break; case 2: EXPECT_EQ(1u, map.size()); EXPECT_EQ(1u, map.count(555)); EXPECT_EQ(1u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); break; case 3: EXPECT_EQ(2u, map.size()); @@ -1770,8 +1733,8 @@ class LayerTreeHostDelegatedTestCommitWithoutTake EXPECT_EQ(1u, map.count(555)); EXPECT_EQ(2u, delegated_impl->Resources().size()); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(999)->second)); - EXPECT_EQ(1u, delegated_impl->Resources().count(map.find(555)->second)); + EXPECT_EQ(1u, delegated_impl->Resources().count(999)); + EXPECT_EQ(1u, delegated_impl->Resources().count(555)); } } diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc index 689cc452..3d5e7a5 100644 --- a/cc/trees/single_thread_proxy.cc +++ b/cc/trees/single_thread_proxy.cc @@ -125,7 +125,7 @@ void SingleThreadProxy::CreateAndInitializeOutputSurface() { } { - DebugScopedSetMainThreadBlocked mainThreadBlocked(this); + DebugScopedSetMainThreadBlocked main_thread_blocked(this); DebugScopedSetImplThread impl(this); layer_tree_host_->DeleteContentsTexturesOnImplThread( layer_tree_host_impl_->resource_provider()); @@ -182,7 +182,7 @@ void SingleThreadProxy::DoCommit(scoped_ptr<ResourceUpdateQueue> queue) { DCHECK(Proxy::IsMainThread()); // Commit immediately. { - DebugScopedSetMainThreadBlocked mainThreadBlocked(this); + DebugScopedSetMainThreadBlocked main_thread_blocked(this); DebugScopedSetImplThread impl(this); // This CapturePostTasks should be destroyed before CommitComplete() is @@ -263,7 +263,7 @@ void SingleThreadProxy::Stop() { TRACE_EVENT0("cc", "SingleThreadProxy::stop"); DCHECK(Proxy::IsMainThread()); { - DebugScopedSetMainThreadBlocked mainThreadBlocked(this); + DebugScopedSetMainThreadBlocked main_thread_blocked(this); DebugScopedSetImplThread impl(this); layer_tree_host_->DeleteContentsTexturesOnImplThread( @@ -365,7 +365,21 @@ void SingleThreadProxy::CompositeImmediately(base::TimeTicks frame_begin_time) { device_viewport_damage_rect, false, // for_readback &frame)) { - layer_tree_host_impl_->SwapBuffers(frame); + { + DebugScopedSetMainThreadBlocked main_thread_blocked(this); + DebugScopedSetImplThread impl(this); + + // This CapturePostTasks should be destroyed before + // DidCommitAndDrawFrame() is called since that goes out to the embedder, + // and we want the embedder to receive its callbacks before that. + // NOTE: This maintains consistent ordering with the ThreadProxy since + // the DidCommitAndDrawFrame() must be post-tasked from the impl thread + // there as the main thread is not blocked, so any posted tasks inside + // the swap buffers will execute first. + BlockingTaskRunner::CapturePostTasks blocked; + + layer_tree_host_impl_->SwapBuffers(frame); + } DidSwapFrame(); } } |