Bug 127756

Summary: Crash in RemoteLayerBackingStore::encode when m_frontBuffer is nullptr.
Product: WebKit Reporter: Jeremy Jones <jeremyj-wk>
Component: New BugsAssignee: Jeremy Jones <jeremyj-wk>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, simon.fraser, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Jeremy Jones 2014-01-27 19:45:54 PST
Crash in RemoteLayerBackingStore::encode when m_frontBuffer is nullptr.
Comment 1 Jeremy Jones 2014-01-27 20:09:53 PST
Created attachment 222399 [details]
Patch
Comment 2 Tim Horton 2014-01-27 20:37:20 PST
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.
Comment 3 Tim Horton 2014-01-27 20:37:32 PST
Comment on attachment 222399 [details]
Patch

did not mean to r+
Comment 4 Simon Fraser (smfr) 2014-01-27 21:17:20 PST
Comment on attachment 222399 [details]
Patch

We need to know why this happens.
Comment 5 Jeremy Jones 2014-01-27 23:12:18 PST
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;
    }
Comment 6 Jeremy Jones 2014-01-27 23:57:37 PST
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
Comment 7 Jeremy Jones 2014-01-28 00:03:09 PST
To repro this problem:

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

Visit a page with a <video> tag.
Comment 8 Tim Horton 2014-01-28 01:58:59 PST
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?
Comment 9 Jeremy Jones 2014-01-28 16:03:31 PST
Created attachment 222523 [details]
Patch
Comment 10 Tim Horton 2014-01-28 18:09:51 PST
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;"
Comment 11 Tim Horton 2014-01-28 18:10:05 PST
Much better! Thanks for fixing.
Comment 12 Jeremy Jones 2014-01-29 16:39:17 PST
Created attachment 222607 [details]
Patch
Comment 13 WebKit Commit Bot 2014-01-30 12:52:52 PST
Comment on attachment 222607 [details]
Patch

Clearing flags on attachment: 222607

Committed r163103: <http://trac.webkit.org/changeset/163103>
Comment 14 WebKit Commit Bot 2014-01-30 12:52:55 PST
All reviewed patches have been landed.  Closing bug.