Add CHECKs to calls to IOSurfaceLookupFromMachPort

When allocating an anonymous IOSurface-backed GLImage, we have the
sequence of calls
- IOSurfaceCreate
- IOSurfaceCreateMachPort
- IOSurfaceRelease (implicitly in ScopedCFTypeRef<IOSurfaceRef>)
- IOSurfaceLookupFromMachPort
The final call persistently fails on some machines.

This adds a CHECK when that call fails, so we can assess how often
it occurs in the wild.

This also adds a call to IOSurfaceCreateMachPort and
IOSurfaceLookupFromMachPort before the implicit IOSurfaceRelease, to
see if this may be the result of a reference counting bug.

Add lots more LOG(ERROR) spew to help further diagnose this.

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie630067759d0bdf096be907986e53afdc632b15d
Reviewed-on: https://chromium-review.googlesource.com/895142
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Victor Miura <vmiura@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533240}

gpu/ipc/client/gpu_memory_buffer_impl_io_surface.cc

@@ -55,10 +55,17 @@ GpuMemoryBufferImplIOSurface::CreateFromHandle(
     gfx::BufferFormat format,
     gfx::BufferUsage usage,
     const DestructionCallback& callback) {
+  if (!handle.mach_port) {
+    LOG(ERROR) << "Invalid IOSurface mach port returned to client.";
+    return nullptr;
+  }
+
   base::ScopedCFTypeRef<IOSurfaceRef> io_surface(
       IOSurfaceLookupFromMachPort(handle.mach_port.get()));
-  if (!io_surface)
+  if (!io_surface) {
+    LOG(ERROR) << "Failed to open IOSurface via mach port returned to client.";
     return nullptr;
+  }
 
   return base::WrapUnique(
       new GpuMemoryBufferImplIOSurface(handle.id, size, format, callback,

gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc

@@ -34,7 +34,7 @@ GpuMemoryBufferFactoryIOSurface::CreateGpuMemoryBuffer(
     int client_id,
     SurfaceHandle surface_handle) {
   // Don't clear anonymous io surfaces.
-  bool should_clear = (client_id != 0);
+  bool should_clear = (client_id != kAnonymousClientId);
   base::ScopedCFTypeRef<IOSurfaceRef> io_surface(
       gfx::CreateIOSurface(size, format, should_clear));
   if (!io_surface) {
@@ -54,6 +54,17 @@ GpuMemoryBufferFactoryIOSurface::CreateGpuMemoryBuffer(
   handle.type = gfx::IO_SURFACE_BUFFER;
   handle.id = id;
   handle.mach_port.reset(IOSurfaceCreateMachPort(io_surface));
+
+  // TODO(ccameron): This should never happen, but a similar call to
+  // IOSurfaceLookupFromMachPort is failing below. This should determine if
+  // the lifetime of the underlying IOSurface determines the failure.
+  // https://crbug.com/795649
+  CHECK(handle.mach_port);
+  base::ScopedCFTypeRef<IOSurfaceRef> io_surface_recreated(
+      IOSurfaceLookupFromMachPort(handle.mach_port.get()));
+  CHECK_NE(nullptr, io_surface_recreated.get())
+      << "Failed to reconstitute still-existing IOSurface from mach port.";
+
   return handle;
 }
 
@@ -112,10 +123,17 @@ GpuMemoryBufferFactoryIOSurface::CreateAnonymousImage(const gfx::Size& size,
   gfx::GpuMemoryBufferHandle handle = CreateGpuMemoryBuffer(
       gfx::GpuMemoryBufferId(next_anonymous_image_id_++), size, format, usage,
       kAnonymousClientId, gpu::kNullSurfaceHandle);
+  if (handle.is_null())
+    return scoped_refptr<gl::GLImage>();
 
-  base::ScopedCFTypeRef<IOSurfaceRef> io_surface;
-  io_surface.reset(IOSurfaceLookupFromMachPort(handle.mach_port.get()));
-  DCHECK_NE(nullptr, io_surface.get());
+  base::ScopedCFTypeRef<IOSurfaceRef> io_surface(
+      IOSurfaceLookupFromMachPort(handle.mach_port.get()));
+  // TODO(ccameron): This should never happen, but has been seen in the wild. If
+  // this happens frequently, it can be replaced by directly using the allocated
+  // IOSurface, rather than going through the handle creation.
+  // https://crbug.com/795649
+  CHECK_NE(nullptr, io_surface.get())
+      << "Failed to reconstitute just-created IOSurface from mach port.";
 
   scoped_refptr<gl::GLImageIOSurface> image(
       gl::GLImageIOSurface::Create(size, internalformat));

ui/accelerated_widget_mac/accelerated_widget_mac.mm

@@ -148,7 +148,7 @@ void AcceleratedWidgetMac::UpdateCALayerTree(
     GotCALayerFrame(base::scoped_nsobject<CALayer>(remote_layer_.get(),
                                                    base::scoped_policy::RETAIN),
                     ca_layer_params.pixel_size, ca_layer_params.scale_factor);
-  } else {
+  } else if (ca_layer_params.io_surface_mach_port) {
     base::ScopedCFTypeRef<IOSurfaceRef> io_surface(
         IOSurfaceLookupFromMachPort(ca_layer_params.io_surface_mach_port));
     if (!io_surface) {
@@ -156,6 +156,11 @@ void AcceleratedWidgetMac::UpdateCALayerTree(
     }
     GotIOSurfaceFrame(io_surface, ca_layer_params.pixel_size,
                       ca_layer_params.scale_factor);
+  } else {
+    LOG(ERROR) << "Frame had neither valid CAContext nor valid IOSurface.";
+    base::ScopedCFTypeRef<IOSurfaceRef> io_surface;
+    GotIOSurfaceFrame(io_surface, ca_layer_params.pixel_size,
+                      ca_layer_params.scale_factor);
   }
   if (view_)
     view_->AcceleratedWidgetSwapCompleted();

ui/gl/gl_image_io_surface.mm

@@ -220,6 +220,10 @@ bool GLImageIOSurface::Initialize(IOSurfaceRef io_surface,
                                   gfx::BufferFormat format) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!io_surface_);
+  if (!io_surface) {
+    LOG(ERROR) << "Invalid IOSurface";
+    return false;
+  }
 
   if (!ValidInternalFormat(internalformat_)) {
     LOG(ERROR) << "Invalid internalformat: "