Fix crash in Canvas2DLayerBridge after a GPU resource is lost

Fix crash in Canvas2DLayerBridge after a GPU resource is lost

BUG=411864
TEST=blink unit test Canvas2DLayerBridgeTest.testPrepareMailboxAndLoseResource

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

[Justin Novosad, 2014-09-09 20:16:53]

junov@chromium.org changed reviewers:
+ senorblanco@chromium.org

[Justin Novosad, 2014-09-09 20:16:54]

PTAL

[Stephen White, 2014-09-09 20:33:06]

https://codereview.chromium.org/552423002/diff/1/Source/platform/graphics/Can...
File Source/platform/graphics/Canvas2DLayerBridge.cpp (right):

https://codereview.chromium.org/552423002/diff/1/Source/platform/graphics/Can...
Source/platform/graphics/Canvas2DLayerBridge.cpp:513: if
(mailboxInfo->m_image->getTexture()) {
Nit: refactor out mailboxInfo->m_image->getTexture() into a temporary, as it's
called 3 times now.

https://codereview.chromium.org/552423002/diff/1/Source/platform/graphics/Can...
Source/platform/graphics/Canvas2DLayerBridge.cpp:521: #if ENABLE(ASSERT)
Code paths that are conditional on asserts give me pause. If this has no effect
in non-assert mode, let's just assign it unconditionally (ie., no #ifdef).

[Stephen White, 2014-09-09 20:44:40]

LGTM (bots willing)

[Stephen White, 2014-09-09 20:44:42]

The CQ bit was checked by senorblanco@chromium.org

[commit-bot: I haz the power, 2014-09-09 20:45:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/junov@chromium.org/552423002/20001

[commit-bot: I haz the power, 2014-09-09 22:16:34]

Committed patchset #2 (id:20001) as 181679

[Hongbo Min, 2014-09-10 04:51:07]

hongbo.min@intel.com changed reviewers:
+ hongbo.min@intel.com

[Hongbo Min, 2014-09-10 04:51:08]

https://codereview.chromium.org/552423002/diff/20001/Source/platform/graphics...
File Source/platform/graphics/Canvas2DLayerBridge.cpp (right):

https://codereview.chromium.org/552423002/diff/20001/Source/platform/graphics...
Source/platform/graphics/Canvas2DLayerBridge.cpp:525: // mailboxes will not be
recycled after this point. Calling remove()
It seems that the issue of memory-use-after-free is caused by not reset
m_releasedMailboxInfoIndex in case of gpu context is lost, right? If so,
alternatively we can also reset m_releasedMailboxInfoIndex to avoid
freeReleasedMailbox called by accident. Of course, in your code, it ensures
freeReleasedMailbox not called again.

[Justin Novosad, 2014-09-10 14:54:10]

On 2014/09/10 04:51:08, Hongbo Min wrote:
>
https://codereview.chromium.org/552423002/diff/20001/Source/platform/graphics...
> File Source/platform/graphics/Canvas2DLayerBridge.cpp (right):
> 
>
https://codereview.chromium.org/552423002/diff/20001/Source/platform/graphics...
> Source/platform/graphics/Canvas2DLayerBridge.cpp:525: // mailboxes will not be
> recycled after this point. Calling remove()
> It seems that the issue of memory-use-after-free is caused by not reset
> m_releasedMailboxInfoIndex in case of gpu context is lost, right? If so,
> alternatively we can also reset m_releasedMailboxInfoIndex to avoid
> freeReleasedMailbox called by accident. Of course, in your code, it ensures
> freeReleasedMailbox not called again.

It was caused by the call to remove(i), which triggers self-destruction of the
canvas 2D layer bridge. The problem is that this destroys the m_mailboxes member
while we are manipulating it in remove().