Bug 147627 – [CoordinatedGraphics] Fixed position element didn't make the compositing layer in WTR.

• Status: NEW
• Component: WebKit EFL

[Hunseop Jeong]

compositing/fixed-with-fixed-layout.html failed after r186911 because

# 49 line in LayoutTests/compositing/fixed-with-fixed-layout.html 
if (window.internals) {
    internals.settings.setAcceleratedCompositingForFixedPositionEnabled(true);

'internals.settings.setAcceleratedCompositingForFixedPositionEnabled(true)' didn't work properly in WTR.

CoordinatedGrapihcs set the 'AcceleratedCompositingForFixedPosition' in WebPage::setUseFixedLayout normally.
But only 'frameView->setUseFixedLayout()' was invoked in WTR if I use the 'internals.settings.setAcceleratedCompositingForFixedPositionEnabled' in test.
So we need to set the AcceleratedCompositingForFixedPositionEnabled in Internals::setUseFixedLayout() to work correctly.

[Hunseop Jeong]

Created attachment 258171 [details]
Patch

[Csaba Osztrogonác]

I have some questions about this change:
- How does this change affect the layout test results? 
- Will we have more passing tests? If yes, we should unskip them.
- Do we reset this setting to the default between each test?

[Simon Fraser (smfr)]

Comment on attachment 258171 [details]
Patch

I don't think this makes sense. fixed layout and these two settings should be orthogonal.

[Tim Horton]

(In reply to comment #3)
> Comment on attachment 258171 [details]
> Patch
> 
> I don't think this makes sense. fixed layout and these two settings should
> be orthogonal.

Right.

This:

> CoordinatedGrapihcs set the 'AcceleratedCompositingForFixedPosition' in WebPage::setUseFixedLayout normally.

Is totally bizarre and wrong, and needs fixing/deconflating.

[Hunseop Jeong]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 258171 [details]
> > Patch
> > 
> > I don't think this makes sense. fixed layout and these two settings should
> > be orthogonal.
> 
> Right.
> 
> This:
> 
> > CoordinatedGrapihcs set the 'AcceleratedCompositingForFixedPosition' in WebPage::setUseFixedLayout normally.
> 
> Is totally bizarre and wrong, and needs fixing/deconflating.

You mean, 'setAcceleratedCompositingForFixedPositionEnabled' & 'setFixedPositionCreatesStackingContext' isn't related with fixedLayout?

Now CoordinatedGraphics turn on these options in WebPage::setUseFixedLayout.
On mac, these options was set in RemoteLayerTreeDrawingArea::updatePreferences.
Could I separate these options from the fixedLayout?

[Hunseop Jeong]

(In reply to comment #2)
> I have some questions about this change:
> - How does this change affect the layout test results? 
This patch can composite the layer for the element that has the fixed position.
CoordinatedGraphics didn't composite the fixed position element in WTR. 
Is it expected result?
> - Will we have more passing tests? If yes, we should unskip them.
It makes about 30 tests passed, but sadly 200 more tests failed.
If compositing the fixed position element is right, we need to rebaseline the expected results. 
> - Do we reset this setting to the default between each test?

[Tim Horton]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Comment on attachment 258171 [details]
> > > Patch
> > > 
> > > I don't think this makes sense. fixed layout and these two settings should
> > > be orthogonal.
> > 
> > Right.
> > 
> > This:
> > 
> > > CoordinatedGrapihcs set the 'AcceleratedCompositingForFixedPosition' in WebPage::setUseFixedLayout normally.
> > 
> > Is totally bizarre and wrong, and needs fixing/deconflating.
> 
> You mean, 'setAcceleratedCompositingForFixedPositionEnabled' &
> 'setFixedPositionCreatesStackingContext' isn't related with fixedLayout?

Yes, that's what I mean. These are totally unrelated settings which the CoordinatedGraphics code for whatever reason conflates.

> Now CoordinatedGraphics turn on these options in WebPage::setUseFixedLayout.
> On mac, these options was set in
> RemoteLayerTreeDrawingArea::updatePreferences.

Turning it on in the drawing areas is still a little weird, but a thousand times better than turning it on because another unrelated setting (fixed layout) was turned on.

> Could I separate these options from the fixedLayout?

Could, and should!

[Hunseop Jeong]

gyuyoung, Do you know the reason that setAcceleratedCompositingForFixedPositionEnabled() and setFixedPositionCreatesStackingContext() was tied with fixedLayout?

In mac, set the setting values of FixedPositionCreatesStackingContext() and AcceleratedCompositingForFixedPositionEnabled() in updatePreferences() of the DrawingArea.

I think CoordinatedGraphics also need to set the FixedPositionCreatesStackingContext and AcceleratedCompositingForFixedPositionEnabled with true for the fixed position element to composite the layer.

If I move the setAcceleratedCompositingForFixedPositionEnabled() and setFixedPositionCreatesStackingContext() from WebPage::setUseFixedLayout() to CoordinatedDrawingArea::updatePreferences(), is it possible to make some problems?