Created attachment 150834 [details] Patch

https://chromiumcodereview.appspot.com/10689108/ is based on this.

Comment on attachment 150834 [details] Patch There are a bunch of other places in TextureLayerChromium where the TextureLayerChromiumClient's context is used. Should those be robust to a null context as well, and maybe a comment added to the TextureLayerChromiumClient declaration that context is optional?

(In reply to comment #3) > (From update of attachment 150834 [details]) > There are a bunch of other places in TextureLayerChromium where the TextureLayerChromiumClient's context is used. Should those be robust to a null context as well, and maybe a comment added to the TextureLayerChromiumClient declaration that context is optional? AFAIK, the other references are for rate limiting and we have ASSERTs in CCLayerTreeHost for that. I'm happy to add ASSERTs in TextureLayerChromium w.r.t. to rate limiting if it improves readability. But perhaps a comment suffices?

Created attachment 150838 [details] Patch

Comment on attachment 150838 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150838&action=review > Source/WebCore/platform/graphics/chromium/TextureLayerChromium.cpp:137 > + if (m_client->context()) > + m_contextLost = m_client->context()->getGraphicsResetStatusARB() != GraphicsContext3D::NO_ERROR; Would m_contextLost = m_client->context() && m_client->context()->... be any more/less correct? I just wonder about the fact you can come by here and not change m_contextLost at all. What if you had a context and it was lost and now you don't have a context, what should this variable's state be? Maybe this is completely not possible and not worth considering?

(In reply to comment #6) > (From update of attachment 150838 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150838&action=review > > > Source/WebCore/platform/graphics/chromium/TextureLayerChromium.cpp:137 > > + if (m_client->context()) > > + m_contextLost = m_client->context()->getGraphicsResetStatusARB() != GraphicsContext3D::NO_ERROR; > > Would m_contextLost = m_client->context() && m_client->context()->... be any more/less correct? I just wonder about the fact you can come by here and not change m_contextLost at all. What if you had a context and it was lost and now you don't have a context, what should this variable's state be? Maybe this is completely not possible and not worth considering? AFAIK, m_contextLost is a one way switch. Once it's set, TLC::drawsContent() will return false and TLC::update() will not be called again.

I've plumbed through a context (the one where the texture IDs are created).

Comment on attachment 150838 [details] Patch Cleared review? from attachment 150838 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).