I think r119313 introduced a crasher; see backtrace below. Fix seems simple enough: return 0 if !m_private->layerTreeHost()->context(). I'll upload a patch. (gdb) bt #0 0x00007f2cb89daa32 in WTF::RefPtr<WebCore::GraphicsContext3D>::get (this=0x20) at ../../third_party/WebKit/Source/WTF/wtf/RefPtr.h:58 #1 0x00007f2cb89da8ea in WebCore::CCGraphicsContext::context3D (this=0x0) at ../../third_party/WebKit/Source/WebCore/platform/graphics/chromium/cc/CCGraphicsContext.h:47 #2 0x00007f2cb89da874 in WebKit::WebLayerTreeView::context (this=0x7f2cb496faf8) at ../../third_party/WebKit/Source/WebKit/chromium/src/WebLayerTreeView.cpp:170 #3 0x00007f2cb8a17096 in WebKit::WebViewImpl::graphicsContext3D (this=0x7f2cb496f880) at ../../third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:3659 #4 0x00007f2cc083065a in RenderViewImpl::createMediaPlayer (this=0x7f2cae18d000, frame=0x7f2cad4d6e00, client=0x7f2cac8a43d0) at ../../content/renderer/render_view_impl.cc:2369 #5 0x00007f2cb89db5bb in WebKit::createWebMediaPlayer (client=0x7f2cac8a43d0, frame=0x7f2caccb6000) at ../../third_party/WebKit/Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:56 #6 0x00007f2cb89dc77f in WebKit::WebMediaPlayerClientImpl::loadInternal (this=0x7f2cac8a43c0) at ../../third_party/WebKit/Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:314 #7 0x00007f2cb89dc6ad in WebKit::WebMediaPlayerClientImpl::load (this=0x7f2cac8a43c0, url=...) at ../../third_party/WebKit/Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:303 #8 0x00007f2cb975d18f in WebCore::MediaPlayer::loadWithNextMediaEngine(WebCore::MediaPlayerFactory*) () from /home/fischman/chrome_root/src/out_lumpy/Debug/lib/libwebkit.so #9 0x00007f2cb975cea0 in WebCore::MediaPlayer::load(WebCore::KURL const&, WebCore::ContentType const&, WTF::String const&) () from /home/fischman/chrome_root/src/out_lumpy/Debug/lib/libwebkit.so #10 0x00007f2cba03db3e in WebCore::HTMLMediaElement::loadResource(WebCore::KURL const&, WebCore::ContentType&, WTF::String const&) () from /home/fischman/chrome_root/src/out_lumpy/Debug/lib/libwebkit.so #11 0x00007f2cba03d45d in WebCore::HTMLMediaElement::selectMediaResource() () from /home/fischman/chrome_root/src/out_lumpy/Debug/lib/libwebkit.so #12 0x00007f2cba03d135 in WebCore::HTMLMediaElement::loadInternal() () from /home/fischman/chrome_root/src/out_lumpy/Debug/lib/libwebkit.so #13 0x00007f2cba03cc15 in WebCore::HTMLMediaElement::load(int&) () from /home/fischman/chrome_root/src/out_lumpy/Debug/lib/libwebkit.so #14 0x00007f2cba308522 in WebCore::HTMLMediaElementV8Internal::loadCallback(v8::Arguments const&) () from /home/fischman/chrome_root/src/out_lumpy/Debug/lib/libwebkit.so #15 0x00007f2cbef531b3 in v8::internal::HandleApiCallHelper<false> (args=..., isolate=0x7f2cb48b9000) at ../../v8/src/builtins.cc:1145 #16 0x00007f2cbef4d54c in v8::internal::Builtin_Impl_HandleApiCall (args=..., isolate=0x7f2cb48b9000) at ../../v8/src/builtins.cc:1162 #17 0x00007f2cbef4d51d in v8::internal::Builtin_HandleApiCall (args=..., isolate=0x7f2cb48b9000) at ../../v8/src/builtins.cc:1161
Created attachment 146934 [details] Patch
Comment on attachment 146934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146934&action=review Test? In what situation is ->context() NULL? > Source/WebKit/chromium/src/WebLayerTreeView.cpp:171 > + WebCore::CCGraphicsContext* c = m_private->layerTreeHost()->context(); > + if (!c) we don't use one-letter variable names in WebKit (probably because we don't have an 80 column limit), please give this a better name. Perhaps "context" ?
Created attachment 146939 [details] Patch
(In reply to comment #2) > (From update of attachment 146934 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146934&action=review > > Test? In what situation is ->context() NULL? I don't know. I was hoping the backtrace in the bug report would be enough for you to divine what's gone wrong. > > Source/WebKit/chromium/src/WebLayerTreeView.cpp:171 > > + WebCore::CCGraphicsContext* c = m_private->layerTreeHost()->context(); > > + if (!c) > > we don't use one-letter variable names in WebKit (probably because we don't have an 80 column limit), please give this a better name. Perhaps "context" ? I dispute that "context" is any clearer in this context, but done ;)
Comment on attachment 146939 [details] Patch Looking more closely it seems quite reasonable that we would get here and extractWeb...() handles a null parameter fine, but we need a test to avoid regressing this (if we had one then we wouldn't have broken this in r119313)
(In reply to comment #5) > (From update of attachment 146939 [details]) > Looking more closely it seems quite reasonable that we would get here and extractWeb...() handles a null parameter fine, but we need a test to avoid regressing this (if we had one then we wouldn't have broken this in r119313) Since you understand this stuff 10000x better than me can you suggest a useful test scenario that'd hit this case? (obvs I could construct a mock something to tickle the codepath, but that doesn't seem very useful)
James/Alexandre: ping? Can you make a concrete suggestion for testing (comment #6), or r+ this without test? (I think we all agree this is a uniformly-positive change to the codebase, what with the not crashing and all).
With the fix for http://code.google.com/p/chromium/issues/detail?id=129103 is this still relevant?
(In reply to comment #8) > With the fix for http://code.google.com/p/chromium/issues/detail?id=129103 is this still relevant? Reverting the proposed patch locally, I no longer see the crashes (which were intermittent before). Are you comfortable asserting m_private->layerTreeHost()->context() is never NULL?
When that patch lands I'm more comfortable deleting the getter that lets you hit this codepath :)
(In reply to comment #10) > When that patch lands I'm more comfortable deleting the getter that lets you hit this codepath :) SGTM!
I've got patches on the way to remove WebView::graphicsContext3D() which I think will make this crash impossible.