Bug 105558 – Get rid of acceleratedCompositingForScrollableFramesEnabled altogether

• Component: Layout and Rendering
• Status: RESOLVED WONTFIX

[Antonio Gomes]

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

[Antonio Gomes]

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?

[Antonio Gomes]

(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.

[James Robinson]

We're using it now in chromium.

[Antonio Gomes]

(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(?)

[Antonio Gomes]

(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.