This covers replacing PRLogModuleInfo w/ LazyLogModule in the 'gfx/' directory. Current usage: > ./gfx/2d/Factory.cpp:61: sLog = PR_NewLogModule("gfx2d"); > ./gfx/layers/Layers.cpp:2386: sLog = PR_NewLogModule("Layers"); > ./gfx/thebes/gfxFT2FontList.cpp:57: sLog = PR_NewLogModule("fontInfoLog"); > ./gfx/thebes/gfxPlatform.cpp:1817: sFontlistLog = PR_NewLogModule("fontlist"); > ./gfx/thebes/gfxPlatform.cpp:1818: sFontInitLog = PR_NewLogModule("fontinit"); > ./gfx/thebes/gfxPlatform.cpp:1819: sTextrunLog = PR_NewLogModule("textrun"); > ./gfx/thebes/gfxPlatform.cpp:1820: sTextrunuiLog = PR_NewLogModule("textrunui"); > ./gfx/thebes/gfxPlatform.cpp:1821: sCmapDataLog = PR_NewLogModule("cmapdata"); > ./gfx/thebes/gfxPlatform.cpp:1822: sTextPerfLog = PR_NewLogModule("textperf"); > ./gfx/thebes/gfxUserFontSet.cpp:32: sLog = PR_NewLogModule("userfonts");

Comment on attachment 8680459 [details] [diff] [review] (part 4) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/ Review of attachment 8680459 [details] [diff] [review]: ----------------------------------------------------------------- Bas: gfx/2d/ has a couple of the few remaining occurrences of PR_LOGGING and MOZ_LOGGING in the tree. Is this necessary/deliberate? Moz2D appears to have its own logging that builds on top of NSPR logging (e.g. see BasicLogger::ShouldOutputMessage()). Also, the definition of GetGFX2DLog() is protected by PR_LOGGING but the declaration and uses are protected by MOZ_LOGGING. That's clearly wrong, so I'll change the PR_LOGGING on the definition to MOZ_LOGGING unless you think it's ok to remove those checks altogether.

Comment on attachment 8680445 [details] [diff] [review] (part 2) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/ Review of attachment 8680445 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxFT2FontList.cpp @@ +48,5 @@ > #include <sys/stat.h> > > using namespace mozilla; > > +static LazyLogModule sFontInfoLog; Need to specify the log name: > static LazyLogModule sFontInfoLog("fontInfoLog"); I'm surprised this compiled, we should file a bug for that.

Comment on attachment 8680450 [details] [diff] [review] (part 3) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/ Review of attachment 8680450 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/Layers.cpp @@ -1821,5 @@ > void > Layer::Log(const char* aPrefix) > { > - if (!IsLogEnabled()) > - return; All of these tests are still important, they check that the "Layer" log module has been set to output the Debug level and avoid potentially expensive string building if not. @@ -2389,5 @@ > -/*static*/ bool > -LayerManager::IsLogEnabled() > -{ > - MOZ_ASSERT(!!sLog, > - "layer manager must be created before logging is allowed"); I think we just need to remove the assert, but keep the function. ::: gfx/layers/Layers.h @@ -619,5 @@ > void PostPresent(); > > void BeginTabSwitch(); > > - static bool IsLogEnabled(); As noted above, we should keep this around. @@ -1608,5 @@ > * Return display list log. > */ > void GetDisplayListLog(nsCString& log); > > - static bool IsLogEnabled() { return LayerManager::IsLogEnabled(); } ...and keep this if it's actually used.

Comment on attachment 8680459 [details] [diff] [review] (part 4) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/ Review of attachment 8680459 [details] [diff] [review]: ----------------------------------------------------------------- The code changes look good, I agree the PR_LOGGING check should be changed. If I recall correctly there is (or was) a desire to ship moz2d w/o various libxul dependencies, hence the MOZ_LOGGING bit (which is set via --disable-logging). I'll let Bas confirm that.

Comment on attachment 8680461 [details] [diff] [review] (part 5) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/ Review of attachment 8680461 [details] [diff] [review]: ----------------------------------------------------------------- Code looks good, just a couple minor comments. ::: gfx/thebes/gfxPlatform.cpp @@ +1809,5 @@ > + static LazyLogModule sFontInitLog("fontinit"); > + static LazyLogModule sTextrunLog("textrun"); > + static LazyLogModule sTextrunuiLog("textrunui"); > + static LazyLogModule sCmapDataLog("cmapdata"); > + static LazyLogModule sTextPerfLog("textperf"); AFAICT only "textrun", "textrunui", and "textperf" are used, we might want to just remove the others. That could be a follow up if you don't want to do it here. @@ +1817,5 @@ > + case eGfxLog_fontinit: return sFontInitLog; > + case eGfxLog_textrun: return sTextrunLog; > + case eGfxLog_textrunui: return sTextrunuiLog; > + case eGfxLog_cmapdata: return sCmapDataLog; > + case eGfxLog_textperf: return sTextPerfLog; nit: Is this being updated to proper mozilla style or is that just personal preference? If the latter, maybe leave as the original author intended so that the style conforms w/ the rest of the file.

Oh and a small follow-up: you might want to update your patch titles to specify the subdirs.

Updated version that changes the stray PR_LOGGING to MOZ_LOGGING and updates a relevant comment.

> ::: gfx/thebes/gfxPlatform.cpp > @@ +1809,5 @@ > > + static LazyLogModule sFontInitLog("fontinit"); > > + static LazyLogModule sTextrunLog("textrun"); > > + static LazyLogModule sTextrunuiLog("textrunui"); > > + static LazyLogModule sCmapDataLog("cmapdata"); > > + static LazyLogModule sTextPerfLog("textperf"); > > AFAICT only "textrun", "textrunui", and "textperf" are used No, there are GetLog() calls with the corresponding eGfxLog_* constants for all of them.

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c10a54f5688

Comment on attachment 8680926 [details] [diff] [review] (part 4) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/ Review of attachment 8680926 [details] [diff] [review]: ----------------------------------------------------------------- We actually use this to log way more than just 2d. But this'll do for now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e22814c4a12037573ff53014026c0f847b1df18 Bug 1219476 (part 1) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/. r=erahm. https://hg.mozilla.org/integration/mozilla-inbound/rev/a1c6f3ef493ca3ba9ba3b582bd41167bfab3385d Bug 1219476 (part 2) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/. r=erahm. https://hg.mozilla.org/integration/mozilla-inbound/rev/901035724d856b70b52210e3d9aca7347fdf0f38 Bug 1219476 (part 3) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/. r=erahm. https://hg.mozilla.org/integration/mozilla-inbound/rev/72ce6aeb54e409f105227d41aeea87baf9f925e7 Bug 1219476 (part 4) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/. r=erahm. https://hg.mozilla.org/integration/mozilla-inbound/rev/9e8dcc47b7b55e2bfee8e064ab9d7ea14af259e9 Bug 1219476 (part 5) - Replace PRLogModuleInfo usage with LazyLogModule in gfx/. r=erahm.

https://hg.mozilla.org/mozilla-central/rev/2e22814c4a12 https://hg.mozilla.org/mozilla-central/rev/a1c6f3ef493c https://hg.mozilla.org/mozilla-central/rev/901035724d85 https://hg.mozilla.org/mozilla-central/rev/72ce6aeb54e4 https://hg.mozilla.org/mozilla-central/rev/9e8dcc47b7b5

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/2e22814c4a12 https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/a1c6f3ef493c https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/901035724d85 https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/72ce6aeb54e4 https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/9e8dcc47b7b5

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion