1350677 - Label nsExiprationTracker subclass gfxFontCache

Status: RESOLVED FIXED

(Core :: Graphics, enhancement, P1)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

Follow up of bug 1341537 comment 1.

nsExpirationTracker::Notify() implicitly called by an internal timer:
which implicitly apply a timer internally in nsExpirationTracker::CheckStartTimer:
http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/xpcom/ds/nsExpirationTracker.h#122

Comment 1

[Vincent Liu[:vliu]]

There were two reasons we can label gfxFontCache as System group.

1. gfxFontCache was constructed from gfxPlatform::Init(), so it was Singleton per Content process.
2. The NotifyExpired callback function didn't touch any web content or any JS operations.

Comment 2

[Vincent Liu[:vliu]]

For the WordCacheExpirationTimerCallback callback function in [1], it also didn't touch web content. Based on this, we can also label it as system group.

[1]: http://searchfox.org/mozilla-central/source/gfx/thebes/gfxFont.cpp#183

Comment 3

[Vincent Liu[:vliu]]

Attachment: 0001-Bug-1350677-Add-Labeling-for-gfxFontCache.-r-jfktham.patch

Hi jfkthame,
Based on Comment 1 and Comment 2, could you please have a review for me? Really thanks

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 8856861 [details] [diff] [review]
0001-Bug-1350677-Add-Labeling-for-gfxFontCache.-r-jfktham.patch

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

Yes, as I understand it SystemGroup should be fine here.

Comment 5

[Vincent Liu[:vliu]]

Attachment: 0001-Bug-1350677-Add-Labeling-for-gfxFontCache.-r-jfktham.patch

There were some bad implicit conversion constructor for 'gfxFontCache' in some platforms. It seems that I missed explicit declaration for 'gfxFontCache'. Since the change is small, I would attached the modified patch and carried r+ from Comment 4. Here is the try result in [1].

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bc98bebf24354c2795e27fd4582a099d6dd4be2

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a0ee65d3bf8
Add Labeling for gfxFontCache. r=jfkthame

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7a0ee65d3bf8