Bug 77269 – [chromium] Defer makeContextCurrent in compositor until first frame

• Component: New Bugs
• Status: RESOLVED FIXED

[James Robinson]

[chromium] Defer makeContextCurrent in compositor until first frame

[James Robinson]

Created attachment 124424 [details]
Patch

[James Robinson]

Created attachment 124477 [details]
Patch

[James Robinson]

This is half of the fix for http://code.google.com/p/chromium/issues/detail?id=106815, which requires that we be able to initialize the compositor before we're ready to call makeContextCurrent on the compositor context.  This defers LayerRendererChromium::initialize() until we're ready to start making the first frame.

This depends on the scheduler not attempting to produce frames until the surface is ready, so it only fixes things for the single threaded scheduler.  The threaded scheduler needs a way to be notified of when it's safe to call makeContextCurrent().  Without that fix, it's possible (but I think very unlikely in practice) that window.open()'d windows with --force-compositing-mode and the threaded compositor may fail to enter compositing mode correctly.

The single threaded scheduler is patched here: https://chromiumcodereview.appspot.com/9225050/ to avoid that issue.

[Nat Duca]

Comment on attachment 124477 [details]
Patch

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

Neat stuff! Unofficial LGTM.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp:311
> +    if (!m_layerRendererInitialized)

/me wonders how m_layerRendererInitialized and m_contentsTextureManager behave on context lost? Is m_layerRendererInitialized true and m_contentsTextureManager non-null? Wearingmy overddesign hat for a moment, it might be nice to put these inside some inner struct that is m_layerRendererState that was obviously null when layer renderer was lost.

But anyway, just thinking out loud.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h:157
>  

Can we make layerRendererCapabilities() pop if !m_layerRendererInitialized?

[Kenneth Russell]

Comment on attachment 124477 [details]
Patch

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

> Source/WebCore/ChangeLog:78
> [phone]  James Robinson  <[e-mail]>

Nit: this entry is left over. Possible you've got two bugs' patches in this one?

[James Robinson]

Ah yeah, I had http://trac.webkit.org/changeset/106306 merged into this branch. I can take care of cleaning ChangeLogs up when landing.

[Kenneth Russell]

Comment on attachment 124477 [details]
Patch

LGTM otherwise based on Nat's review.

[James Robinson]

Created attachment 124803 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 124803 [details]
Patch for landing

Rejecting attachment 124803 [details] from commit-queue.

New failing tests:
compositing/scroll-painted-composited-content.html
platform/chromium/compositing/layout-width-change.html
compositing/geometry/fixed-in-composited.html
compositing/masks/multiple-masks.html
compositing/iframes/overlapped-nested-iframes.html
compositing/geometry/vertical-scroll-composited.html
compositing/masks/masked-ancestor.html
compositing/geometry/ancestor-overflow-change.html
compositing/plugins/invalidate_rect.html
compositing/overflow/fixed-position-ancestor-clip.html
platform/chromium/compositing/img-layer-grow.html
compositing/direct-image-compositing.html
transforms/3d/point-mapping/3d-point-mapping-deep.html
compositing/geometry/tall-page-composited.html
compositing/geometry/horizontal-scroll-composited.html
compositing/masks/simple-composited-mask.html
compositing/geometry/fixed-position.html
compositing/iframes/become-composited-nested-iframes.html
transforms/3d/point-mapping/3d-point-mapping-origins.html
http/tests/inspector/inspect-element.html
compositing/scaling/tiled-layer-recursion.html
Full output: http://queues.webkit.org/results/11387219

[WebKit Review Bot]

Comment on attachment 124803 [details]
Patch for landing

Attachment 124803 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11387348

New failing tests:
compositing/scroll-painted-composited-content.html
platform/chromium/compositing/layout-width-change.html
compositing/geometry/fixed-in-composited.html
compositing/masks/multiple-masks.html
compositing/iframes/overlapped-nested-iframes.html
compositing/geometry/vertical-scroll-composited.html
compositing/masks/masked-ancestor.html
compositing/geometry/ancestor-overflow-change.html
compositing/plugins/invalidate_rect.html
compositing/overflow/fixed-position-ancestor-clip.html
platform/chromium/compositing/img-layer-grow.html
compositing/direct-image-compositing.html
transforms/3d/point-mapping/3d-point-mapping-deep.html
compositing/geometry/tall-page-composited.html
compositing/geometry/horizontal-scroll-composited.html
compositing/masks/simple-composited-mask.html
compositing/geometry/fixed-position.html
compositing/iframes/become-composited-nested-iframes.html
transforms/3d/point-mapping/3d-point-mapping-origins.html
http/tests/inspector/inspect-element.html
compositing/scaling/tiled-layer-recursion.html

[James Robinson]

Committed r106700: <http://trac.webkit.org/changeset/106700>