Enforce context refrence guards in WebGL extensions.

Enforce context refrence guards in WebGL extensions.

It appears that in some cases if an extension object outlives the context it was
created with that context could be GCed out from under it mid-function. This new
pattern enforces a scoped ref count of the associated canvas any time the
context is used by an extension.

BUG=429478
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185069

[bajones, 2014-11-10 18:16:57]

bajones@chromium.org changed reviewers:
+ kbr@chromium.org

[bajones, 2014-11-10 18:16:57]

PTAL

[Ken Russell (switch to Gerrit), 2014-11-10 18:36:43]

The call sites all look good. One concern about the implementation.

Have you been able to verify locally that this fixes the crash (by running the
failing test multiple times, for example)?

https://codereview.chromium.org/687533005/diff/1/Source/core/html/canvas/WebG...
File Source/core/html/canvas/WebGLExtension.cpp (right):

https://codereview.chromium.org/687533005/diff/1/Source/core/html/canvas/WebG...
Source/core/html/canvas/WebGLExtension.cpp:37: m_context->ref();
See above comment about just using RefPtr for m_context.

https://codereview.chromium.org/687533005/diff/1/Source/core/html/canvas/WebG...
File Source/core/html/canvas/WebGLExtension.h (right):

https://codereview.chromium.org/687533005/diff/1/Source/core/html/canvas/WebG...
Source/core/html/canvas/WebGLExtension.h:50:
RawPtrWillBeWeakMember<WebGLRenderingContextBase> m_context;
I'm not sure this is the correct transition type. We want this temporary object
to strongly keep alive the target context; otherwise, we'll be back in the same
boat of the context possibly disappearing out from under the extension. Can we
just use RefPtrWillBeMember here? Even though WebGLRenderingContext isn't
RefCounted, it still has the necessary ref and deref methods to have RefPtr
operate upon it.

[bajones, 2014-11-10 19:57:37]

On 2014/11/10 at 18:36:43, kbr wrote:
> Have you been able to verify locally that this fixes the crash (by running the
failing test multiple times, for example)?

Flaky failures are always hard to verify fixes for, but as far as I can tell it
addresses the problem. (Spin-looping the fuzzer produced test doesn't cause the
browser to explode.) 

> I'm not sure this is the correct transition type. We want this temporary
object to strongly keep alive the target context; otherwise, we'll be back in
the same boat of the context possibly disappearing out from under the extension.
Can we just use RefPtrWillBeMember here? Even though WebGLRenderingContext isn't
RefCounted, it still has the necessary ref and deref methods to have RefPtr
operate upon it.

I forgot that we had talked about this on Friday, thanks for the reminder.
RefPointerWillBeMember does seem to do the job.

[Ken Russell (switch to Gerrit), 2014-11-10 21:07:12]

Thanks for updating this. LGTM

[bajones, 2014-11-10 21:12:26]

The CQ bit was checked by bajones@chromium.org

[commit-bot: I haz the power, 2014-11-10 21:13:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/687533005/20001

[commit-bot: I haz the power, 2014-11-10 22:15:14]

Committed patchset #2 (id:20001) as 185069