Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
197213014:
Add an explicit flush after copyTextureCHROMIUM() to make the WebGL layer contents updated. (Closed)
|
Created:
6 years, 9 months ago by Jun Jiang Modified:
6 years, 9 months ago Reviewers:
Ken Russell (switch to Gerrit), bajones, jamesr, junj, piman, Zhenyao Mo, no sievers, vmiura CC:
blink-reviews, jamesr, krit, dsinclair, jbroman, danakj, Rik, Stephen Chennney, pdr., rwlbuis, jbauman, piman Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Visibility:
Public. |
DescriptionAdd an explicit flush after copyTextureCHROMIUM() to make the WebGL layer contents updated.
It can avoid reading from previous unchanged WebGL layer content during printing.
BUG=351956
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169637
Patch Set 1 #
Total comments: 5
Messages
Total messages: 24 (0 generated)
+sievers and vmiura for review and comment https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... File Source/platform/graphics/gpu/DrawingBuffer.cpp (right): https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... Source/platform/graphics/gpu/DrawingBuffer.cpp:392: m_context->flush(); Uh oh. If this is really the needed fix then there are a bunch of other places throughout the code that also need to be fixed.
https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... File Source/platform/graphics/gpu/DrawingBuffer.cpp (right): https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... Source/platform/graphics/gpu/DrawingBuffer.cpp:392: m_context->flush(); On 2014/03/17 19:17:32, Ken Russell wrote: > Uh oh. If this is really the needed fix then there are a bunch of other places > throughout the code that also need to be fixed. We do need to glFlush() whenever we need the changes to be visible in another context. We seem to do this here in DrawingBuffer::prepareMailbox() where we pass the texture to the compositor in a mailbox (Canvas2DLayerBridge::prepareMailbox() also does this, see webContext->flush()). I don't know what the return path in here implies (also vs. the path below) and who will be consuming this texture.
https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... File Source/platform/graphics/gpu/DrawingBuffer.cpp (right): https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... Source/platform/graphics/gpu/DrawingBuffer.cpp:392: m_context->flush(); On 2014/03/17 19:48:09, sievers wrote: > On 2014/03/17 19:17:32, Ken Russell wrote: > > Uh oh. If this is really the needed fix then there are a bunch of other places > > throughout the code that also need to be fixed. > > We do need to glFlush() whenever we need the changes to be visible in another > context. We seem to do this here in DrawingBuffer::prepareMailbox() where we > pass the texture to the compositor in a mailbox > (Canvas2DLayerBridge::prepareMailbox() also does this, see webContext->flush()). > > I don't know what the return path in here implies (also vs. the path below) and > who will be consuming this texture. My point is that there are other callers of copyTextureCHROMIUM which do not issue a flush immediately afterward. At least there's one in RendererGpuVideoAcceleratorFactories::ReadPixels (src/content/renderer/media/renderer_gpu_video_accelerator_factories.cc). I strongly feel that we should add a Telemetry test for this right away so that it doesn't regress again.
https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... File Source/platform/graphics/gpu/DrawingBuffer.cpp (right): https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... Source/platform/graphics/gpu/DrawingBuffer.cpp:392: m_context->flush(); On 2014/03/17 19:48:09, sievers wrote: > On 2014/03/17 19:17:32, Ken Russell wrote: > > Uh oh. If this is really the needed fix then there are a bunch of other places > > throughout the code that also need to be fixed. > > We do need to glFlush() whenever we need the changes to be visible in another > context. We seem to do this here in DrawingBuffer::prepareMailbox() where we > pass the texture to the compositor in a mailbox > (Canvas2DLayerBridge::prepareMailbox() also does this, see webContext->flush()). > > I don't know what the return path in here implies (also vs. the path below) and > who will be consuming this texture. Checked if other sites have calls to flush(). ImageBuffer::copyToPlatformTexture YES DrawingBuffer::prepareMailbox YES DrawingBuffer::copyToPlatformTexture YES WebMediaPlayerAndroid::copyVideoTextureToPlatformTexture NO RendererGpuVideoAcceleratorFactories::ReadPixels NO WebMediaPlayerImpl::copyVideoTextureToPlatformTexture YES DrawingBuffer::paintCompositedResultsToCanvas NO <- this CL Flushing automatically in copyTextureCHROMIUM may not be sufficient as copyTextureCHROMIUM may not be the last command. example: context->copyTextureCHROMIUM(GL_TEXTURE_2D, getBackingTexture(), texture, level, internalFormat, destType); context->pixelStorei(GC3D_UNPACK_FLIP_Y_CHROMIUM, false); context->pixelStorei(GC3D_UNPACK_UNPREMULTIPLY_ALPHA_CHROMIUM, false); context->flush();
On 2014/03/17 21:35:07, vmiura wrote: > https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... > File Source/platform/graphics/gpu/DrawingBuffer.cpp (right): > > https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... > Source/platform/graphics/gpu/DrawingBuffer.cpp:392: m_context->flush(); > On 2014/03/17 19:48:09, sievers wrote: > > On 2014/03/17 19:17:32, Ken Russell wrote: > > > Uh oh. If this is really the needed fix then there are a bunch of other > places > > > throughout the code that also need to be fixed. > > > > We do need to glFlush() whenever we need the changes to be visible in another > > context. We seem to do this here in DrawingBuffer::prepareMailbox() where we > > pass the texture to the compositor in a mailbox > > (Canvas2DLayerBridge::prepareMailbox() also does this, see > webContext->flush()). > > > > I don't know what the return path in here implies (also vs. the path below) > and > > who will be consuming this texture. > > Checked if other sites have calls to flush(). > > ImageBuffer::copyToPlatformTexture YES > DrawingBuffer::prepareMailbox YES > DrawingBuffer::copyToPlatformTexture YES > WebMediaPlayerAndroid::copyVideoTextureToPlatformTexture NO > RendererGpuVideoAcceleratorFactories::ReadPixels NO > WebMediaPlayerImpl::copyVideoTextureToPlatformTexture YES > DrawingBuffer::paintCompositedResultsToCanvas NO <- this CL > > Flushing automatically in copyTextureCHROMIUM may not be sufficient as > copyTextureCHROMIUM may not be the last command. > > example: > context->copyTextureCHROMIUM(GL_TEXTURE_2D, getBackingTexture(), texture, > level, internalFormat, destType); > context->pixelStorei(GC3D_UNPACK_FLIP_Y_CHROMIUM, false); > context->pixelStorei(GC3D_UNPACK_UNPREMULTIPLY_ALPHA_CHROMIUM, false); > context->flush(); Agreed that we should not imply the flush. It also would not be obvious why glCopyTextureCHROMIUM() behaves different from glCopyTexImage2D() or GL draw calls in that regard, or any other GL commands like you said.
Hi, Kenneth, vmiura and sievers. Thanks for your comments. I have tried to substitute flush() with shallowFlush() in several places independently, but got wrong results instead. Either the PDF printing in MAC OSX gives wrong result or WebGL conformance fails. It seems to me that an explicit flush is needed to make the change visible across contexts. As vmiura mentioned, there are several other places missing a flush and I have uploaded a patch at https://codereview.chromium.org/200763011 for review as well.
Why is the flush needed? What command on a different context is this flush ensuring ordering with?
On 2014/03/18 11:11:35, junj wrote: > Hi, Kenneth, vmiura and sievers. Thanks for your comments. > > I have tried to substitute flush() with shallowFlush() in several places > independently, but got wrong results instead. Either the PDF printing in MAC OSX > gives wrong result or WebGL conformance fails. It seems to me that an explicit > flush is needed to make the change visible across contexts. As vmiura mentioned, > there are several other places missing a flush and I have uploaded a patch at > https://codereview.chromium.org/200763011 for review as well. Shallow flush ensures ordering of commands on the same channel (i.e. in the same process). glFlush() ensures changes to a texture are visible in other contexts in the same share group (and implies the above). Sync Points ensure ordering of commands across channels (for example render and browser compositor) but do not imply glFlush(). So the question comes down to what context the texture being copied into is being used in.
On 2014/03/18 21:09:03, sievers wrote: > On 2014/03/18 11:11:35, junj wrote: > > Hi, Kenneth, vmiura and sievers. Thanks for your comments. > > > > I have tried to substitute flush() with shallowFlush() in several places > > independently, but got wrong results instead. Either the PDF printing in MAC > OSX > > gives wrong result or WebGL conformance fails. It seems to me that an explicit > > flush is needed to make the change visible across contexts. As vmiura > mentioned, > > there are several other places missing a flush and I have uploaded a patch at > > https://codereview.chromium.org/200763011 for review as well. > > Shallow flush ensures ordering of commands on the same channel (i.e. in the same > process). > glFlush() ensures changes to a texture are visible in other contexts in the same > share group (and implies the above). > Sync Points ensure ordering of commands across channels (for example render and > browser compositor) but do not imply glFlush(). > > So the question comes down to what context the texture being copied into is > being used in. So does this mean MakeCurrent does not ensure changes are visible in other contexts?
https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... File Source/platform/graphics/gpu/DrawingBuffer.cpp (right): https://codereview.chromium.org/197213014/diff/1/Source/platform/graphics/gpu... Source/platform/graphics/gpu/DrawingBuffer.cpp:392: m_context->flush(); On 2014/03/17 21:35:08, vmiura wrote: > On 2014/03/17 19:48:09, sievers wrote: > > On 2014/03/17 19:17:32, Ken Russell wrote: > > > Uh oh. If this is really the needed fix then there are a bunch of other > places > > > throughout the code that also need to be fixed. > > > > We do need to glFlush() whenever we need the changes to be visible in another > > context. We seem to do this here in DrawingBuffer::prepareMailbox() where we > > pass the texture to the compositor in a mailbox > > (Canvas2DLayerBridge::prepareMailbox() also does this, see > webContext->flush()). > > > > I don't know what the return path in here implies (also vs. the path below) > and > > who will be consuming this texture. > > Checked if other sites have calls to flush(). > > ImageBuffer::copyToPlatformTexture YES > DrawingBuffer::prepareMailbox YES > DrawingBuffer::copyToPlatformTexture YES > WebMediaPlayerAndroid::copyVideoTextureToPlatformTexture NO > RendererGpuVideoAcceleratorFactories::ReadPixels NO > WebMediaPlayerImpl::copyVideoTextureToPlatformTexture YES > DrawingBuffer::paintCompositedResultsToCanvas NO <- this CL > > Flushing automatically in copyTextureCHROMIUM may not be sufficient as > copyTextureCHROMIUM may not be the last command. > > example: > context->copyTextureCHROMIUM(GL_TEXTURE_2D, getBackingTexture(), texture, > level, internalFormat, destType); > context->pixelStorei(GC3D_UNPACK_FLIP_Y_CHROMIUM, false); > context->pixelStorei(GC3D_UNPACK_UNPREMULTIPLY_ALPHA_CHROMIUM, false); > context->flush(); Right, what matters is not the copyTextureCHROMIUM per se, it's about when we need ordering between contexts. The convention is that flush() is needed to ensure ordering. The reason is that mac needs a (deep) flush.
On 2014/03/18 21:16:37, vmiura wrote: > On 2014/03/18 21:09:03, sievers wrote: > > On 2014/03/18 11:11:35, junj wrote: > > > Hi, Kenneth, vmiura and sievers. Thanks for your comments. > > > > > > I have tried to substitute flush() with shallowFlush() in several places > > > independently, but got wrong results instead. Either the PDF printing in MAC > > OSX > > > gives wrong result or WebGL conformance fails. It seems to me that an > explicit > > > flush is needed to make the change visible across contexts. As vmiura > > mentioned, > > > there are several other places missing a flush and I have uploaded a patch > at > > > https://codereview.chromium.org/200763011 for review as well. > > > > Shallow flush ensures ordering of commands on the same channel (i.e. in the > same > > process). > > glFlush() ensures changes to a texture are visible in other contexts in the > same > > share group (and implies the above). > > Sync Points ensure ordering of commands across channels (for example render > and > > browser compositor) but do not imply glFlush(). > > > > So the question comes down to what context the texture being copied into is > > being used in. > > So does this mean MakeCurrent does not ensure changes are visible in other > contexts? Consulted with piman: wglMakeCurrent() and glXMakeCurrent() seem to imply a flush of the previous context. Angle (which uses a single underlying DX context) and Mac probably don't. (Let's ignore EGL for the time being... it makes even less guarantees when it comes to flush ordering. But we currently use virtual contexts to make it compatible where needed.) Proposed way to go forward that is compatible with what we mostly have been doing on the client side already: 1) Have the client insert a glFlush() every time it changes a texture and it needs to be visible in another context. Therefore LGTM this patch. 2) Consider making behavior wrt context switches compatible on the service side by guaranteeing that a context switch implies a flush. Then all glFlushes() in the client code can be downgraded to a shallow flush or sync point.
On 2014/03/18 19:17:12, jamesr wrote: > Why is the flush needed? What command on a different context is this flush > ensuring ordering with? Hi, jamesr. Thanks for your comments. Following is my understanding on this: At this first stage, the WebGL content is going to be copied from DrawingBuffer into WebGL surface layer(backed by a texture based SkBitmap) *in the WebGL context*. Then in the second stage, canvas()->makePresentationCopy() is called to make a PresentationCopy Image which would be used in HTMLCanvasElement::paint() later. In HTMLCanvasElement::makePresentationCopy(), it will call ImageBuffer->copyImage(CopyBackingStore, Unscaled) and a deep copy happens *in another context* for the texture based SkBitmap for WebGL surface. Specially, in this copy, a new texture based SkBitmap will be created in a different context and copy is done *in this different context*. In the third stage, context->drawImage(m_presentedImage.get(), ...) is called to draw the presented Image. If flush() is not called or use a shallowFlush() here, the copy done in the second stage may copy previous content to the present Image for drawing. That is, we need maintain an order in the gpu server side to make the texture updated before it is used in another context. It is the reason to introduce a Flush() here.
On 2014/03/18 22:44:16, sievers wrote: > On 2014/03/18 21:16:37, vmiura wrote: > > On 2014/03/18 21:09:03, sievers wrote: > > > On 2014/03/18 11:11:35, junj wrote: > > > > Hi, Kenneth, vmiura and sievers. Thanks for your comments. > > > > > > > > I have tried to substitute flush() with shallowFlush() in several places > > > > independently, but got wrong results instead. Either the PDF printing in > MAC > > > OSX > > > > gives wrong result or WebGL conformance fails. It seems to me that an > > explicit > > > > flush is needed to make the change visible across contexts. As vmiura > > > mentioned, > > > > there are several other places missing a flush and I have uploaded a patch > > at > > > > https://codereview.chromium.org/200763011 for review as well. > > > > > > Shallow flush ensures ordering of commands on the same channel (i.e. in the > > same > > > process). > > > glFlush() ensures changes to a texture are visible in other contexts in the > > same > > > share group (and implies the above). > > > Sync Points ensure ordering of commands across channels (for example render > > and > > > browser compositor) but do not imply glFlush(). > > > > > > So the question comes down to what context the texture being copied into is > > > being used in. > > > > So does this mean MakeCurrent does not ensure changes are visible in other > > contexts? > > Consulted with piman: > > wglMakeCurrent() and glXMakeCurrent() seem to imply a flush of the previous > context. > Angle (which uses a single underlying DX context) and Mac probably don't. > > (Let's ignore EGL for the time being... it makes even less guarantees when it > comes to flush ordering. But we currently use virtual contexts to make it > compatible where needed.) > > Proposed way to go forward that is compatible with what we mostly have been > doing on the client side already: > > 1) Have the client insert a glFlush() every time it changes a texture and it > needs to be visible in another context. > > Therefore LGTM this patch. > > 2) Consider making behavior wrt context switches compatible on the service side > by guaranteeing that a context switch implies a flush. > Then all glFlushes() in the client code can be downgraded to a shallow flush or > sync point. I agree with sievers on this. Kenneth, vmiura and piman, any more comments on this?
On 2014/03/19 07:29:59, junj wrote: > On 2014/03/18 22:44:16, sievers wrote: > > On 2014/03/18 21:16:37, vmiura wrote: > > > On 2014/03/18 21:09:03, sievers wrote: > > > > On 2014/03/18 11:11:35, junj wrote: > > > > > Hi, Kenneth, vmiura and sievers. Thanks for your comments. > > > > > > > > > > I have tried to substitute flush() with shallowFlush() in several places > > > > > independently, but got wrong results instead. Either the PDF printing in > > MAC > > > > OSX > > > > > gives wrong result or WebGL conformance fails. It seems to me that an > > > explicit > > > > > flush is needed to make the change visible across contexts. As vmiura > > > > mentioned, > > > > > there are several other places missing a flush and I have uploaded a > patch > > > at > > > > > https://codereview.chromium.org/200763011 for review as well. > > > > > > > > Shallow flush ensures ordering of commands on the same channel (i.e. in > the > > > same > > > > process). > > > > glFlush() ensures changes to a texture are visible in other contexts in > the > > > same > > > > share group (and implies the above). > > > > Sync Points ensure ordering of commands across channels (for example > render > > > and > > > > browser compositor) but do not imply glFlush(). > > > > > > > > So the question comes down to what context the texture being copied into > is > > > > being used in. > > > > > > So does this mean MakeCurrent does not ensure changes are visible in other > > > contexts? > > > > Consulted with piman: > > > > wglMakeCurrent() and glXMakeCurrent() seem to imply a flush of the previous > > context. > > Angle (which uses a single underlying DX context) and Mac probably don't. > > > > (Let's ignore EGL for the time being... it makes even less guarantees when it > > comes to flush ordering. But we currently use virtual contexts to make it > > compatible where needed.) > > > > Proposed way to go forward that is compatible with what we mostly have been > > doing on the client side already: > > > > 1) Have the client insert a glFlush() every time it changes a texture and it > > needs to be visible in another context. > > > > Therefore LGTM this patch. > > > > 2) Consider making behavior wrt context switches compatible on the service > side > > by guaranteeing that a context switch implies a flush. > > Then all glFlushes() in the client code can be downgraded to a shallow flush > or > > sync point. > > I agree with sievers on this. Kenneth, vmiura and piman, any more comments on > this? Agreed, and LGTM.
On 2014/03/19 17:04:14, vmiura wrote: > On 2014/03/19 07:29:59, junj wrote: > > On 2014/03/18 22:44:16, sievers wrote: > > > On 2014/03/18 21:16:37, vmiura wrote: > > > > On 2014/03/18 21:09:03, sievers wrote: > > > > > On 2014/03/18 11:11:35, junj wrote: > > > > > > Hi, Kenneth, vmiura and sievers. Thanks for your comments. > > > > > > > > > > > > I have tried to substitute flush() with shallowFlush() in several > places > > > > > > independently, but got wrong results instead. Either the PDF printing > in > > > MAC > > > > > OSX > > > > > > gives wrong result or WebGL conformance fails. It seems to me that an > > > > explicit > > > > > > flush is needed to make the change visible across contexts. As vmiura > > > > > mentioned, > > > > > > there are several other places missing a flush and I have uploaded a > > patch > > > > at > > > > > > https://codereview.chromium.org/200763011 for review as well. > > > > > > > > > > Shallow flush ensures ordering of commands on the same channel (i.e. in > > the > > > > same > > > > > process). > > > > > glFlush() ensures changes to a texture are visible in other contexts in > > the > > > > same > > > > > share group (and implies the above). > > > > > Sync Points ensure ordering of commands across channels (for example > > render > > > > and > > > > > browser compositor) but do not imply glFlush(). > > > > > > > > > > So the question comes down to what context the texture being copied into > > is > > > > > being used in. > > > > > > > > So does this mean MakeCurrent does not ensure changes are visible in other > > > > contexts? > > > > > > Consulted with piman: > > > > > > wglMakeCurrent() and glXMakeCurrent() seem to imply a flush of the previous > > > context. > > > Angle (which uses a single underlying DX context) and Mac probably don't. > > > > > > (Let's ignore EGL for the time being... it makes even less guarantees when > it > > > comes to flush ordering. But we currently use virtual contexts to make it > > > compatible where needed.) > > > > > > Proposed way to go forward that is compatible with what we mostly have been > > > doing on the client side already: > > > > > > 1) Have the client insert a glFlush() every time it changes a texture and it > > > needs to be visible in another context. > > > > > > Therefore LGTM this patch. > > > > > > 2) Consider making behavior wrt context switches compatible on the service > > side > > > by guaranteeing that a context switch implies a flush. > > > Then all glFlushes() in the client code can be downgraded to a shallow flush > > or > > > sync point. > > > > I agree with sievers on this. Kenneth, vmiura and piman, any more comments on > > this? > > Agreed, and LGTM. Kenneth, could you give a "LGTM" as owner of Source/platform/* ?
On 2014/03/20 01:57:14, junj wrote: > On 2014/03/19 17:04:14, vmiura wrote: > > On 2014/03/19 07:29:59, junj wrote: > > > On 2014/03/18 22:44:16, sievers wrote: > > > > On 2014/03/18 21:16:37, vmiura wrote: > > > > > On 2014/03/18 21:09:03, sievers wrote: > > > > > > On 2014/03/18 11:11:35, junj wrote: > > > > > > > Hi, Kenneth, vmiura and sievers. Thanks for your comments. > > > > > > > > > > > > > > I have tried to substitute flush() with shallowFlush() in several > > places > > > > > > > independently, but got wrong results instead. Either the PDF > printing > > in > > > > MAC > > > > > > OSX > > > > > > > gives wrong result or WebGL conformance fails. It seems to me that > an > > > > > explicit > > > > > > > flush is needed to make the change visible across contexts. As > vmiura > > > > > > mentioned, > > > > > > > there are several other places missing a flush and I have uploaded a > > > patch > > > > > at > > > > > > > https://codereview.chromium.org/200763011 for review as well. > > > > > > > > > > > > Shallow flush ensures ordering of commands on the same channel (i.e. > in > > > the > > > > > same > > > > > > process). > > > > > > glFlush() ensures changes to a texture are visible in other contexts > in > > > the > > > > > same > > > > > > share group (and implies the above). > > > > > > Sync Points ensure ordering of commands across channels (for example > > > render > > > > > and > > > > > > browser compositor) but do not imply glFlush(). > > > > > > > > > > > > So the question comes down to what context the texture being copied > into > > > is > > > > > > being used in. > > > > > > > > > > So does this mean MakeCurrent does not ensure changes are visible in > other > > > > > contexts? > > > > > > > > Consulted with piman: > > > > > > > > wglMakeCurrent() and glXMakeCurrent() seem to imply a flush of the > previous > > > > context. > > > > Angle (which uses a single underlying DX context) and Mac probably don't. > > > > > > > > (Let's ignore EGL for the time being... it makes even less guarantees when > > it > > > > comes to flush ordering. But we currently use virtual contexts to make it > > > > compatible where needed.) > > > > > > > > Proposed way to go forward that is compatible with what we mostly have > been > > > > doing on the client side already: > > > > > > > > 1) Have the client insert a glFlush() every time it changes a texture and > it > > > > needs to be visible in another context. > > > > > > > > Therefore LGTM this patch. > > > > > > > > 2) Consider making behavior wrt context switches compatible on the service > > > side > > > > by guaranteeing that a context switch implies a flush. > > > > Then all glFlushes() in the client code can be downgraded to a shallow > flush > > > or > > > > sync point. > > > > > > I agree with sievers on this. Kenneth, vmiura and piman, any more comments > on > > > this? > > > > Agreed, and LGTM. > > Kenneth, could you give a "LGTM" as owner of Source/platform/* ? Oh, I made a mistake on using the words "LG??" here. Kenneth, please help to take a look another look. Thanks.
On 2014/03/20 03:15:05, junj wrote: > Oh, I made a mistake on using the words "LG??" here. Kenneth, please help to > take a look another look. Thanks. Thanks Jun for the fix and everyone for the careful thought. LGTM; however, it is absolutely essential that we add automated tests for the printing path right away. I've filed follow-on bug 354341 so that bug 351956 can be closed as fixed, and possibly merges requested. Looking for volunteers to fix bug 354341.
The CQ bit was checked by jun.a.jiang@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jun.a.jiang@intel.com/197213014/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.blink on mac_blink_rel
The CQ bit was checked by jun.a.jiang@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jun.a.jiang@intel.com/197213014/1
Message was sent while issue was closed.
Change committed as 169637 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
