971817 - gfxPlatform::GetLog should decide if it's static or not

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

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

gfxPlatform::GetLog is static, so it can be called without using gfxPlatform::GetPlatform(), which means it can be called without the singleton getting created and gfxPlatform::Init() being called.

gfxPlatform::GetLog, however, uses file statics that are only initialized in gfxPlatform::Init.  So, if anybody calls gfxPlatform::GetLog() before gfxPlatform::GetPlatform() is called, they will get a wrong result.

Right now, nsPresContext constructor calls gfxPlatform::GetLog() and that happens before gfxPlatform::GetPlatform() call, so the value is wrong.

To be safe, we should just make gfxPlatform::GetLog() a non-static, so that it needs to be accessed as gfxPlatform::GetPlatform()->GetLog() instead.

Alternatively, the initialization of those statics should be done in GetLog() rather than Init().

Comment 1

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

John, any preference?  Seven files call GetLog(), so more files will be touched if we make this non-static, but I'm not sure just the size of the patch should be guiding us here.

Comment 2

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

Changed the summary to remove the solution suggestion.

Comment 3

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

Attachment: Make gfxPlatform::GetLog responsible for having the correct logs, rather than count on gfxPlatform::Init() getting called first.

Here's an option where we just initialize those logs when we need them.

Comment 4

[John Daggett (:jtd)]

Comment on attachment 8374970 [details] [diff] [review]
Make gfxPlatform::GetLog responsible for having the correct logs, rather than count on gfxPlatform::Init() getting called first.

Yup, looks good.

Comment 5

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

Attachment: Make gfxPlatform::GetLog responsible for having the correct logs, rather than count on gfxPlatform::Init() getting called first. Carry r=jdagget.

Just rebasing.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a99d7db15f4a

Comment 7

[Phil Ringnalda (:philor)]

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