793923 - [Azure] Content using wrong pref

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Azure content is using gfx.canvas.azure.backends instead of gfx.content.backends.

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Fixed gfxPlatform to use gfx.content.azure.backends properly

Comment 2

[Nick Cameron [:nrc]]

I don't see how this patch addresses this bug: I expected to see the name of a pref changed, but the patch doesn't seem to do that. It does turn a global into a local and recompute the backend list for every call of GetBackendPref rather than just once, and I don't see why you would want to do that? (In particular, does GetBackendPref get called a lot or just once?)

Comment 3

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

The pref is only read once for the first pref and it doesn't get read again. Removing the use of the global variable means that it will read the pref every time but it also means it will read the correct pref. GetBackendPref would usually be called either 3 or 6 times depending on whether the init function is called once or twice for the given platform.

Comment 4

[Nick Cameron [:nrc]]

I think this is OK. Given that we only check for two different kinds of backends, we might want to just keep two globals around and save on parsing and pref lookup (we are sensitive about startup time). OTOH, it is a drop in the ocean, really, so maybe it doesn't matter. I guess if we expect the number of calls to GetBackendPref increase we should use two globals, and if we expect the number of kinds of backend to increase we should use this more generic code.

Comment 5

[Nick Cameron [:nrc]]

Comment on attachment 665768 [details] [diff] [review]
Fixed gfxPlatform to use gfx.content.azure.backends properly

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

lgtm, but asking roc for another review just to make sure if we want to take the minor hit to startup time (see comment above).

Comment 6

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

See https://tbpl.mozilla.org/?tree=Try&rev=c688628b3c28

Comment 7

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://tbpl.mozilla.org/?tree=Try&rev=bfaa0f40db06

Comment 8

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Ignore that last comment.

Comment 9

[Jacek Caban]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b610742fced

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8b610742fced