RESOLVED FIXED Bug 77269
[chromium] Defer makeContextCurrent in compositor until first frame 
https://bugs.webkit.org/show_bug.cgi?id=77269
Summary [chromium] Defer makeContextCurrent in compositor until first frame
James Robinson
Reported 2012-01-27 19:13:39 PST
[chromium] Defer makeContextCurrent in compositor until first frame
Attachments
Patch (36.82 KB, patch)
2012-01-27 19:21 PST, James Robinson 		no flags
DetailsFormatted DiffDiff
Patch (40.44 KB, patch)
2012-01-29 17:25 PST, James Robinson 		no flags
DetailsFormatted DiffDiff
Patch for landing (37.64 KB, patch)
2012-01-31 12:59 PST, James Robinson 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
James Robinson
Comment 1 2012-01-27 19:21:34 PST
Created attachment 124424 [details] Patch
James Robinson
Comment 2 2012-01-29 17:25:36 PST
Created attachment 124477 [details] Patch
James Robinson
Comment 3 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.
Nat Duca
Comment 4 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?
Kenneth Russell
Comment 5 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?
James Robinson
Comment 6 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.
Kenneth Russell
Comment 7 2012-01-31 11:45:44 PST
Comment on attachment 124477 [details] Patch LGTM otherwise based on Nat's review.
James Robinson
Comment 8 2012-01-31 12:59:08 PST
Created attachment 124803 [details] Patch for landing
WebKit Review Bot
Comment 9 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
WebKit Review Bot
Comment 10 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
James Robinson
Comment 11 2012-02-03 15:19:09 PST
Committed r106700: <http://trac.webkit.org/changeset/106700>
Note You need to log in before you can comment on or make changes to this bug.