cc: Allocate resources on worker context.

cc/resources/resource_provider.h

 // Copyright 2012 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_RESOURCE_PROVIDER_H_
 #define CC_RESOURCES_RESOURCE_PROVIDER_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 189 matching lines @@
   // 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_token is set on any TransferableResource, this will
   // wait on it.
   void ReceiveFromChild(
-      int child, const TransferableResourceArray& transferable_resources);
+      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 ResourceIdSet& resources_from_child);
 
   // Receives resources from the parent, moving them from mailboxes. Resource
   // IDs passed are in the child namespace.
   // NOTE: if the sync_token is set on any TransferableResource, this will
@@ skipping 13 matching lines @@
   // The following lock classes are part of the ResourceProvider API and are
   // needed to read and write the resource contents. The user must ensure
   // that they only use GL locks on GL resources, etc, and this is enforced
   // by assertions.
   class CC_EXPORT ScopedReadLockGL {
    public:
     ScopedReadLockGL(ResourceProvider* resource_provider,
                      ResourceId resource_id);
     ~ScopedReadLockGL();
 
-    unsigned texture_id() const { return texture_id_; }
-    GLenum target() const { return target_; }
-    const gfx::Size& size() const { return size_; }
-    const gfx::ColorSpace& color_space() const { return color_space_; }
+    unsigned texture_id() const { return resource_->gl_id; }
+    GLenum target() const { return resource_->target; }
+    const gfx::Size& size() const { return resource_->size; }
+    const gfx::ColorSpace& color_space() const {
+      return resource_->color_space;
+    }
 
    private:
     ResourceProvider* resource_provider_;
     ResourceId resource_id_;
-    unsigned texture_id_;
-    GLenum target_;
-    gfx::Size size_;
-    gfx::ColorSpace color_space_;
+    const Resource* resource_;
 
     DISALLOW_COPY_AND_ASSIGN(ScopedReadLockGL);
   };
 
   class CC_EXPORT ScopedSamplerGL {
    public:
     ScopedSamplerGL(ResourceProvider* resource_provider,
                     ResourceId resource_id,
                     GLenum filter);
     ScopedSamplerGL(ResourceProvider* resource_provider,
@@ skipping 13 matching lines @@
     GLenum unit_;
     GLenum target_;
 
     DISALLOW_COPY_AND_ASSIGN(ScopedSamplerGL);
   };
 
   class CC_EXPORT ScopedWriteLockGL {
    public:
     ScopedWriteLockGL(ResourceProvider* resource_provider,
                       ResourceId resource_id,
-                      bool create_mailbox);
+                      bool async_worker_context_enabled);
     ~ScopedWriteLockGL();
 
-    unsigned texture_id() const { return texture_id_; }
-    GLenum target() const { return target_; }
-    ResourceFormat format() const { return format_; }
-    const gfx::Size& size() const { return size_; }
+    unsigned texture_id() const { return resource_->gl_id; }
+    GLenum target() const { return resource_->target; }
+    ResourceFormat format() const { return resource_->format; }
+    const gfx::Size& size() const { return resource_->size; }
+
     // Will return the invalid color space unless
     // |enable_color_correct_rasterization| is true.
     const gfx::ColorSpace& color_space_for_raster() const {
       return color_space_;
     }
 
-    const TextureMailbox& mailbox() const { return mailbox_; }
+    const TextureMailbox& mailbox() const { return resource_->mailbox(); }
 
-    void set_sync_token(const gpu::SyncToken& sync_token) {
-      sync_token_ = sync_token;
-      has_sync_token_ = true;
-    }
+    void LazyAllocate(gpu::gles2::GLES2Interface* gl, unsigned gl_id);
 
-    void set_synchronized(bool synchronized) { synchronized_ = synchronized; }
+    void UpdateSyncToken(const gpu::SyncToken& sync_token);
 
    private:
     ResourceProvider* resource_provider_;
-    ResourceId resource_id_;
-    unsigned texture_id_;
-    GLenum target_;
-    ResourceFormat format_;
-    gfx::Size size_;
-    TextureMailbox mailbox_;
-    gpu::SyncToken sync_token_;
-    bool has_sync_token_;
-    bool synchronized_;
+    ResourceProvider::Resource* resource_;

[piman, 2017/05/17 20:58:58]

I don't think it's safe to keep a pointer to the Resource, if we're going to
access it from another thread. It lives in an unordered_map, and operations on
that map (even if they touch other elements) could invalidate the iterators due
to rehashing.
That would be even more likely to occur if we changed the map to a flat_map
(which could make sense given the number of elements and typical operations on
them).

[sunnyps, 2017/05/17 22:01:35]

unordered_map doesn't invalidate references except when removing that particular
item. It invalidates iterators on insert when growing the map but not
references. The behavior is similar to vector<forward_list<T>>.

In fact, we used pointers until a change I made last year:
https://codereview.chromium.org/1951193002/
I agree that using pointers limits the choice of containers we can use. Do you
think it's acceptable to copy the entire Resource struct into the lock? Copying
only parts of the struct seemed to complicate the code.

[vmiura, 2017/05/17 22:13:10]

I think pointers can be invalidated when the items are rehashed.  See Iterator
invalidation table.

http://en.cppreference.com/w/cpp/container/unordered_map

[sunnyps, 2017/06/08 00:30:07]

Changed this to not use Resource*. Although it's not a problem now because of
unordered_map semantics, requiring pointers to not be invalidated severely
restricts the choice of containers.

+    gfx::ColorSpace color_space_;
+
     base::ThreadChecker thread_checker_;
-    gfx::ColorSpace color_space_;
 
     DISALLOW_COPY_AND_ASSIGN(ScopedWriteLockGL);
   };
 
   class CC_EXPORT ScopedTextureProvider {
    public:
     ScopedTextureProvider(gpu::gles2::GLES2Interface* gl,
                           ScopedWriteLockGL* resource_lock,
-                          bool use_mailbox);
+                          bool async_worker_context_enabled);
     ~ScopedTextureProvider();
 
     unsigned texture_id() const { return texture_id_; }
 
    private:
     gpu::gles2::GLES2Interface* gl_;
-    bool use_mailbox_;
-    unsigned texture_id_;
+    bool async_worker_context_enabled_;
+    unsigned texture_id_ = 0;
 
     DISALLOW_COPY_AND_ASSIGN(ScopedTextureProvider);
   };
 
   class CC_EXPORT ScopedSkSurfaceProvider {
    public:
     ScopedSkSurfaceProvider(ContextProvider* context_provider,
                             ScopedWriteLockGL* resource_lock,
-                            bool use_mailbox,
+                            bool async_worker_context_enabled,
                             bool use_distance_field_text,
                             bool can_use_lcd_text,
                             int msaa_sample_count);
     ~ScopedSkSurfaceProvider();
 
     SkSurface* sk_surface() { return sk_surface_.get(); }
 
    private:
     ScopedTextureProvider texture_provider_;
     sk_sp<SkSurface> sk_surface_;
@@ skipping 49 matching lines @@
     SkBitmap& sk_bitmap() { return sk_bitmap_; }
     bool valid() const { return !!sk_bitmap_.getPixels(); }
     // Will return the invalid color space unless
     // |enable_color_correct_rasterization| is true.
     const gfx::ColorSpace& color_space_for_raster() const {
       return color_space_;
     }
 
    private:
     ResourceProvider* resource_provider_;
-    ResourceId resource_id_;
+    Resource* resource_;
+    gfx::ColorSpace color_space_;
     SkBitmap sk_bitmap_;
-    gfx::ColorSpace color_space_;
+
     base::ThreadChecker thread_checker_;
 
     DISALLOW_COPY_AND_ASSIGN(ScopedWriteLockSoftware);
   };
 
   class CC_EXPORT ScopedWriteLockGpuMemoryBuffer {
    public:
     ScopedWriteLockGpuMemoryBuffer(ResourceProvider* resource_provider,
                                    ResourceId resource_id);
     ~ScopedWriteLockGpuMemoryBuffer();
     gfx::GpuMemoryBuffer* GetGpuMemoryBuffer();
     // Will return the invalid color space unless
     // |enable_color_correct_rasterization| is true.
     const gfx::ColorSpace& color_space_for_raster() const {
       return color_space_;
     }
 
    private:
     ResourceProvider* resource_provider_;
-    ResourceId resource_id_;
-    ResourceFormat format_;
-    gfx::BufferUsage usage_;
-    gfx::Size size_;
-    std::unique_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer_;
+    Resource* resource_;
     gfx::ColorSpace color_space_;
+
     base::ThreadChecker thread_checker_;
 
     DISALLOW_COPY_AND_ASSIGN(ScopedWriteLockGpuMemoryBuffer);
   };
 
   // All resources that are returned to children while an instance of this
   // class exists will be stored and returned when the instance is destroyed.
   class CC_EXPORT ScopedBatchReturnResources {
    public:
     explicit ScopedBatchReturnResources(ResourceProvider* resource_provider);
@@ skipping 262 matching lines @@
   Resource* InsertResource(ResourceId id, Resource resource);
   Resource* GetResource(ResourceId id);
   const Resource* LockForRead(ResourceId id);
   void UnlockForRead(ResourceId id);
   Resource* LockForWrite(ResourceId id);
   void UnlockForWrite(Resource* resource);
 
   void PopulateSkBitmapWithResource(SkBitmap* sk_bitmap,
                                     const Resource* resource);
 
-  void CreateMailboxAndBindResource(gpu::gles2::GLES2Interface* gl,
-                                    Resource* resource);
 
   void TransferResource(Resource* source,
                         ResourceId id,
                         TransferableResource* resource);
   enum DeleteStyle {
     NORMAL,
     FOR_SHUTDOWN,
   };
   void DeleteResourceInternal(ResourceMap::iterator it, DeleteStyle style);
   void DeleteAndReturnUnusedResourcesToChild(ChildMap::iterator child_it,
                                              DeleteStyle style,
                                              const ResourceIdArray& unused);
   void DestroyChildInternal(ChildMap::iterator it, DeleteStyle style);
+
   void LazyCreate(Resource* resource);
-  void LazyAllocate(Resource* resource);
-  void LazyCreateImage(Resource* resource);
 
-  void BindImageForSampling(Resource* resource);
+  void LazyCreateMailbox(Resource* resource);
+
+  void LazyAllocate(ResourceProvider::Resource* resource,

[piman, 2017/05/17 20:58:58]

nit: no need for ResourceProvider::

+                    gpu::gles2::GLES2Interface* gl,
+                    unsigned texture_id);
+
+  void LazyAllocateGpuMemoryBuffer(ResourceProvider::Resource* resource);

[piman, 2017/05/17 20:58:58]

nit: ditto

+
+  void LazyCreateImage(Resource* resource, gpu::gles2::GLES2Interface* gl);
+
+  void BindImageForSampling(Resource* resource,
+                            gpu::gles2::GLES2Interface* gl,
+                            unsigned texture_id);
+
   // Binds the given GL resource to a texture target for sampling using the
   // specified filter for both minification and magnification. Returns the
   // texture target used. The resource must be locked for reading.
   GLenum BindForSampling(ResourceId resource_id, GLenum unit, GLenum filter);
 
   // Returns null if we do not have a ContextProvider.
   gpu::gles2::GLES2Interface* ContextGL() const;
   bool IsGLContextLost() const;
 
   // Will return the invalid color space unless
@@ skipping 53 matching lines @@
   // Set of resource Ids that would like to be notified about promotion hints.
   ResourceIdSet wants_promotion_hints_set_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(ResourceProvider);
 };
 
 }  // namespace cc
 
 #endif  // CC_RESOURCES_RESOURCE_PROVIDER_H_