| Summary: | Avoid using FontPlatformData outside the FontCache | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Julien Chaffraix <jchaffraix> | ||||||||
| Component: | WebCore Misc. | Assignee: | Julien Chaffraix <jchaffraix> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ddkilzer, eric, joepeck, mario.bensi, mitz, webkit.review.bot | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Julien Chaffraix
2009-01-13 01:51:05 PST
Created attachment 26662 [details]
Step 1: Make getCachedFontPlatformData private and export an equivalent method that return a SimpleFontData
Comment on attachment 26662 [details]
Step 1: Make getCachedFontPlatformData private and export an equivalent method that return a SimpleFontData
It seems we might as well go whole-hog and remove all uses of FontPlatformData outside of the font cache if we're going to remove any. They're aren't that many as far as I can tell.
(In reply to comment #2) > (From update of attachment 26662 [details] [review]) > It seems we might as well go whole-hog and remove all uses of FontPlatformData > outside of the font cache if we're going to remove any. They're aren't that > many as far as I can tell. > I wanted to keep the changes simple so that we could easily pinpoint a regression. The aim was to remove them all so I would happily squash the 2 or 3 patches I have if you are willing to take a look at the updated patch :-) OK, all I need is a "sounds sane" from Hyatt, and I'm ready to r+. Comment on attachment 26662 [details]
Step 1: Make getCachedFontPlatformData private and export an equivalent method that return a SimpleFontData
I think the code will be easier to review if we skip this intermediary step and just make FontPlatformData private to begin with. Right now you're adding extra code in CSSFontFaceSource which just makes this patch more confusing.
Created attachment 45134 [details]
FontPlatformData removal, take 2 - left FontCached to keep the changes manageable
style-queue ran check-webkit-style on attachment 45134 [details] without any errors.
Comment on attachment 45134 [details]
FontPlatformData removal, take 2 - left FontCached to keep the changes manageable
This would break builders. Otherwise looks sane. r-.
Created attachment 47501 [details]
FontPlatformData removal, take 2 - merged with ToT
This is the merged version of the previous page. Not setting the r? until the early warning system has checked it.
Comment on attachment 47501 [details]
FontPlatformData removal, take 2 - merged with ToT
I forgot that the EWS will not run if the patch is not r?. So setting this flag now.
Comment on attachment 47501 [details]
FontPlatformData removal, take 2 - merged with ToT
Looks fine.
Attachment 47501 [details] was posted by a committer and has review+, assigning to Julien Chaffraix for commit.
|