Bug 50364 – Throw webglcontextlost and webglcontextrestored events when a WebGL context is lost and restored.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Alexey Marinichev]

This adds a call to getGraphicsResetStatusARB to see if the context was lost.  If it was it will attempt to create a new one.  A javascript event is thrown in both cases.

[Alexey Marinichev]

Created attachment 75342 [details]
Patch

[Kenneth Russell]

Comment on attachment 75342 [details]
Patch

As we've discussed offline you should start a timer when the context is lost -- but only if the user has registered a webglcontextrestored callback on the canvas. The timer would poll periodically to see whether it can successfully recreate the context, and once it does, send the webglcontextrestored event. You can find examples of setting up periodic callbacks in the implementation of DOMWindow::setInterval (WebCore/page/DOMWindow.cpp). The current implementation relies on the app making WebGL calls against the lost context in order to discover that the context has been restored, which isn't supposed to be the programming model according to the WebGL spec.

[Alexey Marinichev]

Created attachment 75555 [details]
Patch

[Kenneth Russell]

Comment on attachment 75555 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=75555&action=review

Looks good overall. A few minor issues.

> WebCore/html/canvas/WebGLRenderingContext.cpp:97
> +        startOneShot(1);

Instead of 1, use a const double with a descriptive name like secondsBetweenRestoreAttempts.

> WebCore/html/canvas/WebGLRenderingContext.cpp:106
> +            startOneShot(1);

Const double instead of 1.

> WebCore/html/canvas/WebGLRenderingContext.cpp:2125
> +        m_restoreTimer.startOneShot(1);

Const double instead of 1.

> WebCore/html/canvas/WebGLRenderingContext.h:67
> +class WebGLRenderingContextRestoreTimer : public TimerBase {
> +public:
> +    WebGLRenderingContextRestoreTimer(WebGLRenderingContext* context) : m_context(context) { }
> +private:
> +    virtual void fired();
> +    WebGLRenderingContext* m_context;
> +};

In WebKit code each class generally goes into its own header file. In this case I think you ought to be able to forward declare this class as long as you change the data member to a RefPtr below.

> WebCore/html/canvas/WebGLRenderingContext.h:357
> +    WebGLRenderingContextRestoreTimer m_restoreTimer;

Please try making this a RefPtr<WebGLRenderingContextRestoreTimer> and see whether that allows the declaration and definition of WebGLRenderingContextRestoreTimer to move to the .cpp file.

[Alexey Marinichev]

Created attachment 75741 [details]
Patch

[Alexey Marinichev]

Introduced secondsBetweenRestoreAttempts as requested.

Also, rather than adding another header file, I moved WebGLRenderingContextRestoreTimer inside WebGLRenderingContext.  friend statement is no longer needed.

[Kenneth Russell]

Comment on attachment 75741 [details]
Patch

Looks good.

[WebKit Commit Bot]

Comment on attachment 75741 [details]
Patch

Rejecting patch 75741 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build', '--no-clean', '--no-update', '--build-style=both']" exit_code: 1
ERROR: Working directory has local commits, pass --force-clean to continue.

Full output: http://queues.webkit.org/results/6786072

[Kenneth Russell]

Comment on attachment 75741 [details]
Patch

Let's try again.

[WebKit Commit Bot]

Comment on attachment 75741 [details]
Patch

Clearing flags on attachment: 75741

Committed r73424: <http://trac.webkit.org/changeset/73424>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 75741 [details]:

fast/media/mq-aspect-ratio.html
fast/dom/onerror-img.html

Please file bugs against the tests.  These tests were authored by [e-mail].  The commit-queue is continuing to process your patch.