1218881 - Enforce queries' new availability semantics

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Queries are not supposed become available until the browser has spun the event loop. Chrome implemented this here:
https://codereview.chromium.org/1408803003

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Jeff, do you have any implementation suggestions for this?

Comment 2

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> Jeff, do you have any implementation suggestions for this?

We should be very wary about this. I suspect we may see porting issues with this restriction. It should be given a pref to bypass it, and we should be ready to remove it.

Implementation of this is very low priority.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Attachment: Enforce queries' new availability semantics

This makes WebGLQuery into an nsIRunnable. We dispatch it to the event loop so
that we can make sure queries don't become available before they are supposed to. It also adds a pref to control this behavior.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Attachment: Enforce queries' new availability semantics v2

Comment 5

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8699688 [details] [diff] [review]
Enforce queries' new availability semantics v2

Review of attachment 8699688 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/WebGLQuery.h
@@ +24,5 @@
>      explicit WebGLQuery(WebGLContext* webgl);
>  
> +    class AvailableRunnable final : public nsRunnable
> +    {
> +        public:

public: (and private:) should be on the same indent as {

@@ +26,5 @@
> +    class AvailableRunnable final : public nsRunnable
> +    {
> +        public:
> +        AvailableRunnable(WebGLQuery* query) : mQuery(query) { }
> +        NS_IMETHOD Run() override {

Please leave a newline between one-line function defs and multi-line definitions.

@@ +31,5 @@
> +            mQuery->mCanBeAvailable = true;
> +            return NS_OK;
> +        }
> +        private:
> +        RefPtr<WebGLQuery> mQuery;

const RefPtr, and can be public. (doesn't matter though)

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cdc2bfed4faa

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for static build bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=18789248&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd319db81bb8

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9c29c0efe691

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9c29c0efe691