Re: Bug in Acid3 test #72 (race condition when images decode asynchronously)

I spoke a bit loosely. Here's a more accurate description:

Everything up to step 13 of "update the image data" happens synchronously
from the parser, then the fetching happens in parallel (main fetch step
10), which posts a task straight away (because the data: URL's data is
available without any delay, see basic fetch, noting that nothing there
needs time to resolve), and that triggers step 14 of "update the image
data" which requires the image to be decoded and the size determined
synchronously in a single task.

The point is that even if it happens in parallel, there's nothing to
fetching the data: URL that will take any time, since the data is right
there. If it takes ten milliseconds to base64-decode a short data: URL then
the browser is buggy regardless of what the spec says, and the acid3 test
is really not relevant to the question at that point. The whole point of
data: URLs is to inline the data so that it can be rendered quickly.

On Mon, Jan 2, 2017 at 5:22 AM Simon Pieters <simonp@opera.com> wrote:

> Where is it defined that data: is fetched sync? I thought it was defined
> to be fetched async.
>
> Step 6 awaits a stable state before entering the synchronous section, so
> that is a microtask before the fetch can start. Step 13 starts the fetch
> but the Fetch spec doesn't special case data: URLs to be sync, as far as I
> can tell?
>
> I believe Chromium is trying to make <img src="data:..."> load async. See
> https://bugs.chromium.org/p/chromium/issues/detail?id=514206
>
> On Sat, Dec 31, 2016 at 4:46 AM, Ian Hickson <ian@hixie.ch> wrote:
>
> Glad to hear it. :-)
>
> Cheers,
> --
> Ian Hickson
>
> On Fri, Dec 30, 2016 at 4:12 PM Daniel Holbert <dholbert@mozilla.com>
> wrote:
>
> Thanks for clarifying that -- I wasn't familiar with that "update the
> image data" spec section.
>
> Based on that section, I think you're correct, and Acid3 test #72 is
> indeed OK after all.  I've got a local patch to fix Firefox to perform
> data-URI dimension-decodes synchronously, so that we'll go back to
> satisfying this chunk of spec & passing that subtest.
>
> ~Daniel
>
> On 12/23/2016 02:13 AM, Ian Hickson wrote:
> > I believe this is non-conforming. data: URLs are defined to be fetched
> > synchronously and image decoding to obtain dimensions is defined to
> > happen instantaneously (for non-data: URLs, this is black-box
> > indistinguishable from it being rolled into the network delay, of
> course).
> >
> > Specifically, see step 14 of "update the image data", noting that
> > everything up to step 13 happens synchronously from the parser, then the
> > fetching happens as per the fetch spec in one task (because it's a data:
> > URL, it's not asynchronous), and that triggers step 14 which requires
> > the image to be decoded and the size determined synchronously in a
> > single task.
> >
> > --
> > Ian Hickson
> >
> > On Fri, Dec 23, 2016 at 2:03 AM Daniel Holbert <dholbert@mozilla.com
> > <mailto:dholbert@mozilla.com>> wrote:
> >
> >     On 12/23/2016 12:06 AM, Ian Hickson wrote:
> >     > The image in question is a data: URL. There's a ten millisecond gap
> >     > between the last test and this test. That guarantees that there's
> at
> >     > least one opportunity to handle the microtask in which the data:
> URL
> >     > gets loaded. As far as I can tell this means that by the time the
> >     height
> >     > property accessor is read, it's guaranteed that the image is
> loaded.
> >
> >     Nah, it's actually not guaranteed to be loaded -- hence the bug that
> was
> >     filed for Firefox failing this subtest. :)
> >
> >     In current Firefox versions, image-decoding actually happens on a
> >     *helper-thread* -- not in a main-thread microtask. So, depending on
> >     thread-switching / signalling overhead, the image very well might not
> >     have been decoded (& had its results reported back) when the 10
> >     millisecond timeout expires. Hence, it may very well be in the
> >     spec-defined "unavailable" state at that point, when the test reads
> the
> >     <img> size.
> >
> >     You might think that this is silly and we should redesign this to
> decode
> >     image sizes faster, and you'd perhaps be right.
> >
> >     But whether we redesign it or not, our current behavior *is*
> compliant
> >     with the HTML spec.  And I don't see why the Acid3 test should
> >     arbitrarily mandate a decode-in-10ms behavior.
> >
> >     I linked to a patch in my initial post, which simply makes the test
> >     check whether the image is ready before reading the size (& retry if
> >     not).  Given the above explanation (and the spec discussion in my
> >     initial post), does my suggested change seem reasonable?
> >
> >     Thanks,
> >     ~Daniel
> >
> > --
> >
> > --
> > Ian Hickson
> >
> > 😸
> >
>
> --
>
> --
> Ian Hickson
>
> 😸
>
>
> --

-- 
Ian Hickson

😸