Crash in RemoteLayerBackingStore::encode when m_frontBuffer is nullptr.

[Jeremy Jones]

Crash in RemoteLayerBackingStore::encode when m_frontBuffer is nullptr.

[Jeremy Jones]

Created attachment 222399 [details]
Patch

[Tim Horton]

Comment on attachment 222399 [details]
Patch

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

At the least, this seems likely to be an incomplete fix, because whatever is causing this seems like it would also cause a failure in the m_acceleratesDrawing branch.

Do you know why it is that we are encoding a backing store that has never been painted? Is this a custom layer or something?

> Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm:97
> +    {

These go on the previous line.

[Tim Horton]

Comment on attachment 222399 [details]
Patch

did not mean to r+

[Simon Fraser (smfr)]

Comment on attachment 222399 [details]
Patch

We need to know why this happens.

[Jeremy Jones]

This happens because, in the following code, previouslyDrewContents is true.

RemoteLayerBackingStore::display()
...
    // If we previously were drawsContent=YES, and now are not, we need
    // to note that our backing store has been cleared.
    if (!m_layer->owner() || !m_layer->owner()->platformCALayerDrawsContent()) {
        bool previouslyDrewContents = hasFrontBuffer();

        m_frontBuffer = nullptr;
#if USE(IOSURFACE)
        m_frontSurface = nullptr;
#endif

        return previouslyDrewContents;
    }

[Jeremy Jones]

This page has a <video> tag.

m_layer->owner()->platformCALayerDrawsContent() is false because...

void RenderLayerBacking::updateDrawsContent(bool isSimpleContainer)
  bool RenderLayerBacking::containsPaintedContent(bool isSimpleContainer) const
...
#if ENABLE(VIDEO)
    if (renderer().isVideo() && toRenderVideo(renderer()).shouldDisplayVideo())
        return m_owningLayer.hasBoxDecorationsOrBackground();
#endif

[Jeremy Jones]

To repro this problem:

    DebugUseWebKit2 = 1;
    WebKitAVFoundationEnabled = 1;
    WebKitVideoPluginProxyEnabled = 0;

Visit a page with a <video> tag.

[Tim Horton]

Comment on attachment 222399 [details]
Patch

OK, so it is a custom layer. We should totally avoid encoding the RemoteLayerBackingStore in this case; can you move this bool out to RemoteLayerTreeTransaction's encoder, and avoid encoding the backing store completely?

[Jeremy Jones]

Created attachment 222523 [details]
Patch

[Tim Horton]

Comment on attachment 222523 [details]
Patch

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

> Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm:312
> +        bool hasFrontBuffer{false};

please "bool hasFontBuffer = false;"

[Tim Horton]

Much better! Thanks for fixing.

[Jeremy Jones]

Created attachment 222607 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 222607 [details]
Patch

Clearing flags on attachment: 222607

Committed r163103: <http://trac.webkit.org/changeset/163103>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.