[webkit-reviews] review granted: [Bug 209863] ASSERTION FAILED: m_wrapper on webgl/max-active-contexts-webglcontextlost-prevent-default.html : [Attachment 395192] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 209863: ASSERTION FAILED: m_wrapper on
webgl/max-active-contexts-webglcontextlost-prevent-default.html
https://bugs.webkit.org/show_bug.cgi?id=209863

Attachment 395192: Patch

https://bugs.webkit.org/attachment.cgi?id=395192&action=review




--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 395192
  --> https://bugs.webkit.org/attachment.cgi?id=395192
Patch

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

> Source/WebCore/html/HTMLCanvasElement.cpp:963
> +    if (is<WebGLRenderingContextBase>(m_context.get())) {
> +	   // WebGL rendering context may fire contextlost / contextchange /
contextrestored events at any point.
> +	   return m_hasRelevantWebGLEventListener &&
!downcast<WebGLRenderingContextBase>(*m_context).isContextUnrecoverablyLost();
> +    }

Why not include is<WebGLRenderingContextBase>(m_context.get()) in the cached
m_hasRelevantWebGLEventListener boolean to have one less check to do here?

> Source/WebCore/html/HTMLCanvasElement.cpp:980
> +#if ENABLE(WEBGL)
> +    m_hasRelevantWebGLEventListener =
hasEventListeners(eventNames().webglcontextchangedEvent)
> +	   || hasEventListeners(eventNames().webglcontextlostEvent)
> +	   || hasEventListeners(eventNames().webglcontextrestoredEvent);
> +#endif

We’re caching this because this is too expensive to do inside
virtualHasPendingActivity?