Re-attach the example html. The color is purple in first page load and then becomes black after refresh.

Deleted: OffscreenCanvas-fillRect-in-worker.html 905 bytes

OffscreenCanvas-fillRect-in-worker.html 905 bytes View Download

To repro the bug, is it essential for the OffscreenCanvas to be in a Worker?

That fact that this happens on reload hints that the issue is related to some kind of persistent state. Could an internal cache in the CSSParser, which is used to decode color values.  Invoking parts the CSSParser in a Worker, outside the scope of a document is cause for a bit of concern. I don't think this has ever been done before, so I am not surprised that it would be glitchy. Usually we only evaluate style values in the context of DOM objects that are attached to active documents.

Yes, having the imagebitmap transferred across workers is essential to reproduce this bug.

I tried the following:
<script>
  var aCanvas = new OffscreenCanvas(100, 100);                                     
  var ctx = aCanvas.getContext('2d');                                              
  ctx.fillStyle = "rgb(100, 50, 150)";                                             
  ctx.fillRect(0, 0, 100, 100);                                                    
  var image = aCanvas.transferToImageBitmap();                                     
  var outputCtx = document.getElementById('output').getContext('imagebitmap');  
  outputCtx.transferImageBitmap(image);
</script>

And no matter how many times I refresh the page, the colors are correct.

This problem has to do with the CSSValuePool, which is a static global collection of CSS values. One of its many collections is called ColorValueCache, which is a heap hashmap of CSSColorValue.

Each time when a color code is supplied, this function "CSSValuePool::createColorValue(RGBA32 rgbValue)" is invoked. It checks whether the supplied rgbValue already exists in the ColorValueCache, if it exists, it will return the existing value; if not, it will call CSSColorValue::create and put that newly created value into the ColorValueCache.

In the first page load of the test html, the entry is a new entry and therefore a correct color is created. In the second page load, it reads from the ColorValueCache, which is also a correct value if printed out as String to standard output. However, the difference is while the created value in the first page load has a class type of "ColorClass", the retrieved value in the second page load has a class type of "55" (out of the enum range of CSSValue::ColorClass). This is an important difference because as long as the class type is wrong, CSSParser will judge that color parsing has failed and return a default black color.

In addition, ASSERT(header->checkHeader()); in HeapPage.h is failed; the exception log shows some segmentation related to CSSValuePool::trace().

I suspect that the fact that OffscreenCanvas loads the value from CSSValuePool in a worker thread is the reason. I am not sure whether a global heapmap is built to work properly from both window and worker contexts and if it's not what could be a better alternative data structure for CSSValuePool.

At first glance, it looks like CSSValuePool, in particular the accesses to m_colorValueCache, is not thread safe. You should probably have thread-local instances of CSSValuePool instead of a shared global instance.  Either that or make the class thread-safe.

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

commit 4c51c29704df5572015f4d4b12589c9e6dca437c
Author: xlai <xlai@chromium.org>
Date: Fri Apr 01 00:32:48 2016

Color parsing in CSSParser: avoid using ColorValueCache on worker thread

ColorValueCache is one of the many heap hashmap that CSSValuePool (a global
static instance) maintains to store re-usable CSS Color values. It is not
thread-safe; thus it is problematic when new CSS Color values are inserted into
it and later retrieved from it, on worker threads.

Because making CSSParser thread-safe is not a trivial effort, this patch is a
temporary (and possibly permanent depending on discussions) solution for the
correct color parsing of OffscreenCanvas on a worker.

BUG= 598706 

Review URL: https://codereview.chromium.org/1844083002

Cr-Commit-Position: refs/heads/master@{#384448}

[modify] https://crrev.com/4c51c29704df5572015f4d4b12589c9e6dca437c/third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-fillRect-in-worker-expected.html
[modify] https://crrev.com/4c51c29704df5572015f4d4b12589c9e6dca437c/third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-fillRect-in-worker.html
[modify] https://crrev.com/4c51c29704df5572015f4d4b12589c9e6dca437c/third_party/WebKit/Source/core/css/CSSValuePool.cpp

We can close this.  The follow-up is in  issue 599659