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

Yeah it's definitely not sync with the parser, on that we agree. Acid3 has
a 10ms pause between each test.

On Mon, Jan 2, 2017 at 3:11 PM Simon Pieters <simonp@opera.com> wrote:

> Ah, OK. Yes, I agree it should be available "quickly". But it should also
> *not* be available sync in the same script that sets .src, since it needs a
> task.
>
> Just trying to make sure browsers aren't going in different directions
> here. :-)
>
> On Mon, Jan 2, 2017 at 9:00 PM, Ian Hickson <ian@hixie.ch> wrote:
>
> 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
>
> 😸
>
>
> --

-- 
Ian Hickson

😸