1432039 - AzureCanvasBackend and AzureContentBackend in about:support are not displayed correctly

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Henrik Winnemöller]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180121100439

Steps to reproduce:

User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
Build-ID: 20180121100439

1. Create a fresh profile, open about:config
2. Set "gfx.canvas.azure.backends" to "skia,cairo"
3. Set "gfx.content.azure.backends" to "skia,cairo"
4. Restart Firefox


Actual results:

about:support shows
AzureCanvasBackend: Direct2D 1.1
AzureContentBackend: Direct2D 1.1


Expected results:

about:support shows
AzureCanvasBackend: Skia
AzureContentBackend: Skia

Comment 1

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Have you tried to disable the hardware acceleration? Set the value of layers.acceleration.disabled to true, then restart the browser. In this case, I see that AzureCanvasBackend (UI Process) and AzureContentBackend (UI Process) are both set on skia. AzureCanvasBackend and AzureContentBackend entries disappear (checked on 59.0b7 (20180205211730) and 
58.0.2 (20180206200532)). In fact, you don't even need to set other pref, except layers.acceleration.disabled.

Comment 2

[Henrik Winnemöller]

(In reply to Iulia Cristescu, QA [:JuliaC] from comment #1)
> In fact, you don't even need to set other pref, except layers.acceleration.disabled.

I can reproduce that behaviour. Still, I am not sure that the about:support page always shows the actual state of which API is used for AzureCanvasBackend and AzureContentBackend. When investigating https://bugzilla.mozilla.org/show_bug.cgi?id=1422288, one of the solutions was

> [r]emoving direct2d1.1 from gfx.canvas.azure.backends (https://bugzilla.mozilla.org/show_bug.cgi?id=1422288#c3)

That fixed the bug for me, but about:support still statet that "Direct2D 1.1" was still enabled although it clearly was not in this case. I also removed the "Direct2D 1.1" from gfx.content.azure.backends to further investigate that behaviour and it also did not show any difference on the about:support page.

Comment 3

[Sotaro Ikeda [:sotaro]]

I confirmed the problem. I take a look.

Comment 4

[Sotaro Ikeda [:sotaro]]

The prefs worked as expected, but about:support showed wrong value.

The problem existed at gfxPlatform::GetAzureBackendInfo(). When gpu process exists, it decides "AzureCanvasBackend" and "AzureContentBackend" just from gfxConfig::IsEnabled(gfx::Feature::DIRECT2D). It could becomes true even if the prefs does not have direct2d1.1.
  https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatform.cpp#2727

Then the problem did not happen when multi process(e10s) was disabled.

Comment 5

[Sotaro Ikeda [:sotaro]]

It seems better to improve the assumption of content process's back-end prefs.

Comment 6

[Sotaro Ikeda [:sotaro]]

Attachment: patch - improve assumption of content process's back-end prefs

Comment 7

[Sotaro Ikeda [:sotaro]]

Confirmed attachment 8951529 [details] [diff] [review] addressed the problem for me.

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8951529 [details] [diff] [review]
patch -  improve assumption of content process's back-end prefs

Review of attachment 8951529 [details] [diff] [review]:
-----------------------------------------------------------------

While the fully correct thing would be to get this information back from the content processes, this simplification, even with the duplicated logic, is going to be correct a great majority of the time.  The only thing I would add is a comment in gfxWindowsPlatform... code that its logic is being used elsewhere as well.  That way if somebody changes the logic in one place, they will know to redo it in the other as well.

Comment 9

[Sotaro Ikeda [:sotaro]]

(In reply to Milan Sreckovic [:milan] from comment #8)
> Comment on attachment 8951529 [details] [diff] [review]
> patch -  improve assumption of content process's back-end prefs
> 
> Review of attachment 8951529 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> While the fully correct thing would be to get this information back from the
> content processes, this simplification, even with the duplicated logic, is
> going to be correct a great majority of the time.  The only thing I would
> add is a comment in gfxWindowsPlatform... code that its logic is being used
> elsewhere as well.  That way if somebody changes the logic in one place,
> they will know to redo it in the other as well.

I am going to keep the assumption. But I am going to remove logic duplication, since the logic change could happen like Bug 1421818.

Comment 10

[Sotaro Ikeda [:sotaro]]

Attachment: patch - improve assumption of content process's back-end prefs

Comment 11

[Sotaro Ikeda [:sotaro]]

Attachment: patch - improve assumption of content process's back-end prefs

Updated a comment.

Comment 12

[Sotaro Ikeda [:sotaro]]

attachment 8957479 [details] [diff] [review] added BackendPrefsData and gfxPlatform::GetBackendPrefs() to unify and simplify the logic.

Comment 13

[Sotaro Ikeda [:sotaro]]

Attachment: patch - improve assumption of content process's back-end prefs

Fixed build failure on mac.

Comment 14

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b36d5e43f1be236490d94c671f2c4b629b5823b0

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c71ee5e970f6
Improve assumption of content process's back-end prefs r=jrmuizel

Comment 16

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/c71ee5e970f6