This error will happen as long as gpu compositing is enabled. The error only occurs when (1) the browser is shut down (2) the next commit() is sent out.

Both
OffscreenCanvasFrameDispatcherImpl::setTransferableResourceToSharedGPUContext()
and 
OffscreenCanvasFrameDispatcherImpl::setTransferableResourceToStaticBitmapImage()
have the same problems.

Digging into this creates more questions than before:

The GL_INVALID_ENUM was generated when there is a texture that has no images (i.e. texture->HasImages() returns false) being processed at 
TextureManager::SetParameteri where the 
pname = "GL_TEXTURE_MIN_FILTER" and the param = 0. 

As we've already set "gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);" in 
OffscreenCanvasFrameDispatcherImpl::setTransferableResourceToSharedGPUContext, the texture that we care about is definitely not the one
causing the problem. Rather, there is an extra texture that's being processed at the time when browser is shutdown or another commit()
comes in, and that extra texture does not have GL_TEXTURE_MIN_FILTER defined.

As I'm not familiar with the GL commands, there are many codes in OffscreenCanvasFrameDispatcherImpl::setTransferableResourceToSharedGPUContext
that I don't get it, such as why caching the texture id (an integer?) will keep the texture, and why we do gl->ShallowFlushChromium, etc.

I am speculating that it is the release of texture that's somehow causing the problem; but I couldn't identify which.
I also tried to make some speculative fix (https://codereview.chromium.org/2758863003) which does not work.

junov@: Does the texture generated and prepared in setTransferableResourceToSharedGPUContext() and setTransferableResourceToStaticBitmapImage()
really get cached/recycled/deleted well? Why?

Also +zmo, who is more familiar with the GL commands, to see if any insights could be provided.

You will need find the GPU command buffer client side TexParameteri() that caused the INVALID_ENUM to understand why this happens.  service side won't give you a clear clue in many cases.

Usually the way I do this is I print out client side TexParameteri with texture client id, and service side TexParameteri with texture client id (not service id).  If each run I get the same id, then I could insert an assertion on the client side, and get a stack and understand what's going on.

Do you have reliable ways to reproduce this?  If yes, I can take a look.

I was trying to look at the client side by --enable-gpu-client-logging. There was indeed a "glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, 0)" getting called right before the error message being printed:

[0x24d65d967420] glBindTexture(GL_TEXTURE_2D, 153)
[0x24d65d967420] glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, 0)
[0x24d65d967420] glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, 0)
[0x24d65d967420] glDeleteTextures(1, 0x24d65e0b162c)
 0: 153
glOrderingBarrierCHROMIUM
[.DisplayCompositor-0x24d65d967420]GL ERROR :GL_INVALID_ENUM : glTexParameteri: param was GL_FALSE

On the server side, I print out some details of the texture when result==GL_INVALID_ENUM and I find that that texture looked very similar to the one prepared in OffscreenCanvasFrameDispatcherImpl::setTransferableResourceToSharedGPUContext. It has a size of 400X400X4 (size of the placeholder canvas), texture service id 18, 
texture target GL_TEXTURE_2D, and its min_filter() being GL_LINEAR. 
However, the line at texture_manager.cc::2119 attempts to set this texture's min_filter to be 0, therefore causing an GL_INVALID_ENUM.


zmo@: I guess the "texture client id" that you mentioned is 153 in the above output? How do you print out a texture client id on service side?

To reproduce this behavior, just run Chrome with the flags "--enable-experimental-canvas-features --disable-accelerated-2d-canvas --enable-gpu-compositing" with the following html file:
<canvas id="canvas2D" width="400" height="400" background-color="red"></canvas> 
<script>                                                     
var canvas2D = document.getElementById("canvas2D");                             
var offscreenCanvas = canvas2D.transferControlToOffscreen();                    
var offscreen2d = offscreenCanvas.getContext("2d");                                                                   
offscreen2d.fillStyle = "green";                                                
offscreen2d.fillRect(0, 0, 400, 400);                                           
offscreen2d.commit(); 
</script>

The error will produce at the moment when you close/refresh the browser, or if a second commit() was fired.

To me, it really looks like we generated some extra gl commands (e.g. glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, 0)) to the texture prepared in OffscreenCanvasFrameDispatcherImpl::setTransferableResourceToSharedGPUContext when that texture was finished being used. But I couldn't identify where those client-side commands are.

On the service side, each TextureRef has client_id() and service_id(). So if you can get consistent client_id() that the error is generated on from the service side from different runs, you can add line on the client side's GLES2Implementation::TexParameteri, something like if (texture_id == XXX) DCHECK(false).

Then you should get a stack trace of who is triggering the wrong command and why.

zmo@: I followed your suggested approach and get this caller that throws the error:

gpu::GpuCommandBufferStub::OnAsyncFlush()

which happens to be the method called by IPC GpuCommandBufferMsg_AsyncFlush inside GpuChannelHost::InternalFlush(). From that point onwards, it is hard to find the original client-side caller as there are a lot of commands that eventually reach InternalFlush().

Is there a way to identify who is calling that InternalFlush()? I look through GpuChannelHost but couldn't identify some identity id like client_id().

This is still service side.  We need to trace back to the client side who called TexParameteri to understand what's going on.

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

commit d49d131187ed4e0c3dd5124cdd6ba41cbc06a2a9
Author: xlai <xlai@chromium.org>
Date: Wed Mar 29 19:27:18 2017

Fix error msg of OffscreenCanvas.commit() on gpu compositing

The previous CL, https://codereview.chromium.org/2607373002/, that fixed
 a resizing problem in accelerated 2d canvas, has introduced this error
 message "GL_INVALID_ENUM : glTexParameteri: param was GL_FALSE" because
the resource.filter in OffscreenCanvasFrameDispatcherImpl is not set.
This causes ResourceProvider::DeleteAndReturnUnusedResourcesToChild to run
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, 0) as it discovers
a difference between original_filter and filter in resource when closing
down browser.

We then find out that whilst quad.nearest_neighbor indicates the desired
filtering effect on the rendered quad, the resource.filter actually
indicates the filtering algorithm on the resource inherently. To remove
this error msg whilst maintaining the correctness of filtering algorithm
applied on accelerated 2d canvas, we need to specify the resource filter to
GL_NEAREST. Then the overriding mechanism about resource->original_filter
and resource->filter in resource_provider.cc will be able to function
correctly.

In addition, I added GL_TEXTURE_MAG_FILTER as there is no reason why
we only apply linear filtering on MIN_FILTER and not MAG_FILTER.

BUG= 692599 

Review-Url: https://codereview.chromium.org/2758863003
Cr-Commit-Position: refs/heads/master@{#460488}

[modify] https://crrev.com/d49d131187ed4e0c3dd5124cdd6ba41cbc06a2a9/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp