[Webkit-unassigned] [Bug 50364] Throw webglcontextlost and webglcontextrestored events when a WebGL context is lost and restored.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50364


Kenneth Russell <kbr at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #75555|review?                     |review-
               Flag|                            |




--- Comment #4 from Kenneth Russell <kbr at google.com>  2010-12-06 13:02:24 PST ---
(From update of attachment 75555)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.