Bug 77269 - [chromium] Defer makeContextCurrent in compositor until first frame
Summary: [chromium] Defer makeContextCurrent in compositor until first frame
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-27 19:13 PST by James Robinson
Modified: 2012-02-03 15:19 PST (History)
8 users (show)

See Also:


Attachments
Patch (36.82 KB, patch)
2012-01-27 19:21 PST, James Robinson
no flags Details | Formatted Diff | Diff
Patch (40.44 KB, patch)
2012-01-29 17:25 PST, James Robinson
no flags Details | Formatted Diff | Diff
Patch for landing (37.64 KB, patch)
2012-01-31 12:59 PST, James Robinson
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2012-01-27 19:13:39 PST
[chromium] Defer makeContextCurrent in compositor until first frame
Comment 1 James Robinson 2012-01-27 19:21:34 PST
Created attachment 124424 [details]
Patch
Comment 2 James Robinson 2012-01-29 17:25:36 PST
Created attachment 124477 [details]
Patch
Comment 3 James Robinson 2012-01-29 17:42:18 PST
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.
Comment 4 Nat Duca 2012-01-30 17:43:09 PST
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?
Comment 5 Kenneth Russell 2012-01-30 18:27:45 PST
Comment on attachment 124477 [details]
Patch

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

> Source/WebCore/ChangeLog:78
> +2012-01-27  James Robinson  <jamesr@chromium.org>

Nit: this entry is left over. Possible you've got two bugs' patches in this one?
Comment 6 James Robinson 2012-01-30 18:49:44 PST
Ah yeah, I had http://trac.webkit.org/changeset/106306 merged into this branch. I can take care of cleaning ChangeLogs up when landing.
Comment 7 Kenneth Russell 2012-01-31 11:45:44 PST
Comment on attachment 124477 [details]
Patch

LGTM otherwise based on Nat's review.
Comment 8 James Robinson 2012-01-31 12:59:08 PST
Created attachment 124803 [details]
Patch for landing
Comment 9 WebKit Review Bot 2012-01-31 16:43:00 PST
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
Comment 10 WebKit Review Bot 2012-01-31 23:50:05 PST
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
Comment 11 James Robinson 2012-02-03 15:19:09 PST
Committed r106700: <http://trac.webkit.org/changeset/106700>