Closed Bug 1350677 Opened 8 years ago Closed 8 years ago

Label nsExiprationTracker subclass gfxFontCache

Categories

(Core :: Graphics, enhancement, P1)

Product:
Core
Component:
Graphics
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: bevis, Assigned: vliu)

References

Details

(Whiteboard: [QDL][TDC-MVP][GFX][gfx-noted])

Attachments

(1 file, 1 obsolete file)

0001-Bug-1350677-Add-Labeling-for-gfxFontCache.-r-jfktham.patch
(deleted), patch
Details | Diff | Splinter Review
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
Whiteboard: [QDL][TDC-MVP][GFX]
Whiteboard: [QDL][TDC-MVP][GFX] → [QDL][TDC-MVP][GFX][gfx-noted]
Assignee: nobody → 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.
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
Priority: -- → P1
Hi jfkthame, Based on Comment 1 and Comment 2, could you please have a review for me? Really thanks
Attachment #8856861 - Flags: review?(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.
Attachment #8856861 - Flags: review?(jfkthame) → review+
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
Attachment #8856861 - Attachment is obsolete: true
Pushed by vliu@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7a0ee65d3bf8 Add Labeling for gfxFontCache. r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/7a0ee65d3bf8
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: