Thanks for catching this. We still have time to get this landed and merged back into M71 to unblock launching Android OOP-D there so I'll mark it RBS. 

In terms of work necessary here:
1. GpuMemoryBufferFactoryAndroidHardwareBuffer: Not thread safe.
2. GpuMemoryBufferFactoryIOSurface: I think this is already thread safe from https://crrev.com/769703004? 
3. GpuMemoryBufferFactoryNativePixmap: This has a lock to guard |native_pixmaps_| but I don't think it's actually thread safe. CreateBuferMemoryBuffer() calls SurfaceFactoryOzone::CreateNativePixmap() which isn't necessarily thread safe.
4. GpuMemoryBufferFactoryDXGI: I'm not sure if the DX calls are thread safe but otherwise it's stateless. This also isn't used with OOP-D display compositor.

+geofflang, do you know if it's safe to call Angle's internals and D3D from any thread other than the main GPU thread (where we initialize ANGLE & bindings etc)? We already do it without OOP-D (calling it on the IO thread) though I think the use case is limited to VR so it may be uncommon enough that potential races are rare. +billorr who introduced GpuMemoryBufferFactoryDXGI

For GpuMemoryBufferFactoryDXGI, I think we are assuming we have an angle context bound to the thread we are on for QueryD3D11DeviceObjectFromANGLE to work.  This is the only ANGLE call made when creating an image.

I believe CreateTexture2D, and ID3D11Device methods in general, are thread safe since we don't specify D3D11_CREATE_DEVICE_SINGLETHREADED when creating the d3d11Device.  That said, we'd have to be careful if angle could destroy the device out from under us.

GLImageDXGIHandle calls eglDestroySurface, QueryD3D11DeviceObjectFromANGLE, and several other ANGLE functions to bind the image.  This needs to happen on the same thread as ANGLE is otherwise used.

"we are assuming we have an angle context bound to the thread we are on for QueryD3D11DeviceObjectFromANGLE to work" I'm pretty sure that isn't true on the IO thread where CreateBuferMemoryBuffer is called (today), or on the compositor thread (with OOP-D). I /think/ we're not using any context state, it's only EGL state via the Display, and only looking at constant data, so we're /probably/ ok? Would love confirmation though. Otherwise it might be interesting to extract the d3d device at GpuMemoryBufferFactoryDXGI creation time.
Re: GLImageDXGIHandle, that should only be called on the main thread, so that should be ok.

Right now ANGLE's EGL functions are not thread safe but you can safely call them from any thread if you lock and only call from one thread at a time.  I plan on internally locking in each EGL call like mesa does.  That said, EGL queries should be safe, they don't rely on any thread state but I would avoid creating and destroying surfaces.

The ID3D11Device returned by QueryD3D11DeviceObjectFromANGLE will be safe to use for resource creation or queries on any thread, D3D says it's thread safe.

I'll pick this task up. The crucial GpuMemoryBufferFactory instances that need to be thread safe in M71 are Mac and Android.

GpuMemoryBufferFactoryAndroidHardwareBuffer
1. |buffer_map_| needs to be protected by a lock.
2. GpuMemoryBufferImplAndroidHardwareBuffer::Create() needs to be verified as thread safe.
2a. AndroidHardwareBufferCompat::GetInstance() is thread safe.
2b. AndroidHardwareBufferCompat::Allocate() just calls into Android API AHardwareBuffer_allocate().
3. CreateImageForGpuMemoryBuffer() calls eglCreateImageKHR() so it needs to be called on the GPU thread. This isn't part of the GpuMemoryBufferFactory interface though.

ericrk/khushalsagar: Do you know if AHardwareBuffer_allocate() is thread safe for 2b?

GpuMemoryBufferFactoryIOSurface
1. https://crrev.com/769703004 added a lock to protect |io_surfaces_| because it's used on multiple threads already. This was because the GpuMemoryBufferFactory and ImageFactory functions can be called on different threads, so it doesn't necessarily guarantee GpuMemoryBufferFactory functions are thread safe.
2. gfx::CreateIOSurface() calls into a couple of macOS APIs: IOSurfaceCreate(), IOSurfaceLock() and IOSurfaceUnlock(). I believe IOSurface is supposed to thread safe.
3. It also calls macOS API IOSurfaceLookupFromMachPort() directly. I believe IOSurface is supposed to thread safe.
4. Similar concerns with CreateImageForGpuMemoryBuffer() but again not important.

This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Re #8, yes, the framework API for AHardwareBuffer_allocate is thread-safe.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7dd348c0aeeee59b40ff6a21e04b8bfa659c2831

commit 7dd348c0aeeee59b40ff6a21e04b8bfa659c2831
Author: kylechar <kylechar@chromium.org>
Date: Fri Nov 09 22:09:02 2018

Improve GpuMemoryBufferFactoryAndroidHardwareBuffer thread safety.

The overridden functions for GpuMemoryBufferFactory can be called on
multiple threads with OOP-D. Add a lock to protect |buffer_map_| which
is used in both CreateGpuMemoryBuffer() and DestroyGpuMemoryBuffer().

Bug:  902163 
Change-Id: Ib80169103677f5bdaccc2d4803efa029f4910b03
Reviewed-on: https://chromium-review.googlesource.com/c/1329547
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606989}
[modify] https://crrev.com/7dd348c0aeeee59b40ff6a21e04b8bfa659c2831/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.cc
[modify] https://crrev.com/7dd348c0aeeee59b40ff6a21e04b8bfa659c2831/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.h

This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d6affc78f6841064921ca6d01930afae49908f1

commit 6d6affc78f6841064921ca6d01930afae49908f1
Author: kylechar <kylechar@chromium.org>
Date: Tue Nov 13 22:03:23 2018

Improve InProcessGpuMemoryBufferManager thread safety.

Don't pass GpuChannelManager to InProcessGpuMemoryBufferManager. Most of
GpuChannelManager should only be accessed from the GPU thread so there
is the potential for unsafe operations. Both SyncPointManager and
GpuMemoryBufferFactory are thread safe so they should be fine.

Also ensure that GpuMemoryBuffer destruction callbacks dereference their
WeakPtr on the correct thread. GpuMemoryBuffers can be created and used
on any thread, even though InProcessGpuMemoryBufferManager isn't used
on multiple threads currently.

Bug:  902163 
Change-Id: I8118def4fabd0307f0a58d403b9f9c08bc72fc6b
Reviewed-on: https://chromium-review.googlesource.com/c/1330030
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607761}
[modify] https://crrev.com/6d6affc78f6841064921ca6d01930afae49908f1/cc/test/pixel_test.cc
[modify] https://crrev.com/6d6affc78f6841064921ca6d01930afae49908f1/components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/6d6affc78f6841064921ca6d01930afae49908f1/components/viz/service/display_embedder/in_process_gpu_memory_buffer_manager.h
[modify] https://crrev.com/6d6affc78f6841064921ca6d01930afae49908f1/components/viz/service/main/viz_compositor_thread_runner.cc
[modify] https://crrev.com/6d6affc78f6841064921ca6d01930afae49908f1/components/viz/service/main/viz_compositor_thread_runner.h
[modify] https://crrev.com/6d6affc78f6841064921ca6d01930afae49908f1/gpu/ipc/service/gpu_memory_buffer_factory.h

At this point I think all GpuMemoryBufferFactory implementations are thread safe. The only one I changed was GpuMemoryBufferImplAndroidHardwareBuffer. This would only be used on the VizCompositorThread with Android SurfaceControl enabled. That isn't launched yet so the fix doesn't need a merge back to M71. GpuMemoryBufferFactoryNativePixmap was already thread safe after all.

InProcessGpuMemoryBufferManager is only used on the VizCompositorThread, so the fixes for thread safety there also don't need a merge back to M71. I don't think GpuChannelManager::InternalDestroyGpuMemoryBuffer() needs a PostTask anymore but otherwise this is done.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de5cab17425f8285be094c04f5cf25fa01c4f970

commit de5cab17425f8285be094c04f5cf25fa01c4f970
Author: kylechar <kylechar@chromium.org>
Date: Mon Dec 17 19:59:48 2018

Don't PostTask DestroyGpuMemoryBuffer().

GpuMemoryBufferFactory::DestroyGpuMemoryBuffer() is thread safe so it's
no longer necessary to PostTask to the IO thread before calling it.

Bug:  902163 
Change-Id: Ic50b1642f4929ae7ac0076f336b6c1efd931d643
Reviewed-on: https://chromium-review.googlesource.com/c/1379399
Reviewed-by: Jonathan Backer <backer@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617194}
[modify] https://crrev.com/de5cab17425f8285be094c04f5cf25fa01c4f970/gpu/ipc/service/gpu_channel_manager.cc
[modify] https://crrev.com/de5cab17425f8285be094c04f5cf25fa01c4f970/gpu/ipc/service/gpu_channel_manager.h