cc: Allocate resources on worker context.

cc/resources/resource_provider.h

Index: cc/resources/resource_provider.h
diff --git a/cc/resources/resource_provider.h b/cc/resources/resource_provider.h
index 2b664ffd702bbf2d6c936d3216876ea74289563a..910493b0a3af513f373b89214a3ec7d2a2ec64d8 100644
--- a/cc/resources/resource_provider.h
+++ b/cc/resources/resource_provider.h
@@ -207,7 +207,8 @@ class CC_EXPORT ResourceProvider
   // 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,
@@ -241,18 +242,17 @@ class CC_EXPORT ResourceProvider
                      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);
   };
@@ -286,42 +286,33 @@ class CC_EXPORT ResourceProvider
    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_;
-    base::ThreadChecker thread_checker_;
+    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_;
+
     DISALLOW_COPY_AND_ASSIGN(ScopedWriteLockGL);
   };
 
@@ -329,15 +320,15 @@ class CC_EXPORT ResourceProvider
    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);
   };
@@ -346,7 +337,7 @@ class CC_EXPORT ResourceProvider
    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);
@@ -416,9 +407,10 @@ class CC_EXPORT ResourceProvider
 
    private:
     ResourceProvider* resource_provider_;
-    ResourceId resource_id_;
-    SkBitmap sk_bitmap_;
+    Resource* resource_;
     gfx::ColorSpace color_space_;
+    SkBitmap sk_bitmap_;
+
     base::ThreadChecker thread_checker_;
 
     DISALLOW_COPY_AND_ASSIGN(ScopedWriteLockSoftware);
@@ -438,12 +430,9 @@ class CC_EXPORT ResourceProvider
 
    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);
@@ -726,8 +715,6 @@ class CC_EXPORT ResourceProvider
   void PopulateSkBitmapWithResource(SkBitmap* sk_bitmap,
                                     const Resource* resource);
 
-  void CreateMailboxAndBindResource(gpu::gles2::GLES2Interface* gl,
-                                    Resource* resource);
 
   void TransferResource(Resource* source,
                         ResourceId id,
@@ -741,11 +728,23 @@ class CC_EXPORT ResourceProvider
                                              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.