105558 2012-12-20 11:47:33 -0800 Get rid of acceleratedCompositingForScrollableFramesEnabled altogether 2012-12-20 13:49:35 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 71714 105573 1 tonikitoo tonikitoo jamesr kenneth vangelis oldest_to_newest 795611 0 tonikitoo 2012-12-20 11:47:33 -0800 Talked to James Robinson about this a couple of some months ago, and just got around this code again and remember. Pasted below the relevant bits of conversion: (...) 705190 Sep 21 16:27:44 <jamesr_> acceleratedCompositingForScrollableFramesEnabled could probably just go away completely 705193 Sep 21 16:28:22 <tonikitoo> jamesr_, can it? in favor of anything else? 705194 Sep 21 16:28:31 <tonikitoo> let me know, and I can help with that 705195 Sep 21 16:28:56 <jamesr_> tonikitoo, so when we initially added force compositing mode we automatically promoted iframes too (it helped with gmail at the time) 705196 Sep 21 16:29:01 <jamesr_> but then gmail rewrote their dom, heh 705197 Sep 21 16:29:22 <jamesr_> so we added this to get forceCompositingMode back to just entering compositing mode but not promoting anything 705198 Sep 21 16:29:44 <jamesr_> while still having the other behavior available. i don't think we want that other behavior at all nowadays, so we could just get rid of it 705201 Sep 21 16:30:29 <jamesr_> tonikitoo, so i think we could just drop the setting 705202 Sep 21 16:30:37 <jamesr_> at least, chromium isn't using it right now afaik 795655 1 tonikitoo 2012-12-20 12:14:56 -0800 Some options: 1) Just reverting bug 71714 would change the behavior for all ports, since it would make "force compositing mode" associated only to the frame scrollability. 2) keep the current behavior: the setting acceleratedCompositingForScrollableFramesEnabled is FALSE for all ports, then we could just make "force compositing mode" to be FALSE, if it is an inner frame, like below: void RenderLayerCompositor::cacheAcceleratedCompositingFlags() { ... if (forceCompositingMode && m_renderView->document()->ownerElement()) - forceCompositingMode = settings->acceleratedCompositingForScrollableFramesEnabled() && requiresCompositingForScrollableFrame(); + forceCompositingMode = false; ... But then, some chromium specific tests would start failing, including: platform/chromium/compositing/force-compositing-mode/overflow-iframe-layer.html @James: any thoughts? 795660 2 tonikitoo 2012-12-20 12:24:09 -0800 (In reply to comment #1) > Some options: > > 1) Just reverting bug 71714 would change the behavior for all ports, since it would make "force compositing mode" associated only to the frame scrollability. > > 2) keep the current behavior: the setting acceleratedCompositingForScrollableFramesEnabled is FALSE for all ports, then we could just make "force compositing mode" to be FALSE, if it is an inner frame, like below: > > void RenderLayerCompositor::cacheAcceleratedCompositingFlags() > { > ... > if (forceCompositingMode && m_renderView->document()->ownerElement()) > - forceCompositingMode = settings->acceleratedCompositingForScrollableFramesEnabled() && requiresCompositingForScrollableFrame(); > + forceCompositingMode = false; > ... > > But then, some chromium specific tests would start failing, including: > > platform/chromium/compositing/force-compositing-mode/overflow-iframe-layer.html > > @James: any thoughts? We could actually remove these tests, since they would be no longer applicable. 795692 3 jamesr 2012-12-20 12:52:45 -0800 We're using it now in chromium. 795706 4 tonikitoo 2012-12-20 13:01:50 -0800 (In reply to comment #3) > We're using it now in chromium. Ok, then a couple of questions: - Are you using it in conjunction to ForceCompositingMode only, like previously? - I think it makes sense to turn it into a compositing trigger instead(?) 795764 5 tonikitoo 2012-12-20 13:49:35 -0800 (In reply to comment #4) > (In reply to comment #3) > > We're using it now in chromium. > > Ok, then a couple of questions: > > - Are you using it in conjunction to ForceCompositingMode only, like previously? > - I think it makes sense to turn it into a compositing trigger instead(?) Based on James' input, I am closing this bug for now in favor of bug 105573, but feel free to answer the questions above here or there.