Bug 140081 - Font::primaryFontData() should return a reference
Summary: Font::primaryFontData() should return a reference
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-01-05 08:28 PST by Antti Koivisto
Modified: 2022-07-27 14:28 PDT (History)
7 users (show)

See Also:


Attachments
patch (64.63 KB, patch)
2015-01-05 09:23 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch2 (64.67 KB, patch)
2015-01-05 09:27 PST, Antti Koivisto
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2015-01-05 08:28:00 PST
It is not null
Comment 1 Antti Koivisto 2015-01-05 09:23:07 PST
Created attachment 243977 [details]
patch
Comment 2 Antti Koivisto 2015-01-05 09:27:11 PST
Created attachment 243978 [details]
patch2
Comment 3 Darin Adler 2015-01-05 09:37:35 PST
Comment on attachment 243978 [details]
patch2

View in context: https://bugs.webkit.org/attachment.cgi?id=243978&action=review

A tiny bit sad that something used this often got a little longer, but I suppose it’s strange that a font has a function that returns “a font”.

> Source/WebCore/platform/graphics/FontGlyphs.h:77
> +    const FontData& primaryFontData(const FontDescription& description) { return *realizeFontDataAt(description, 0); }

Do we even need a function for this?
Comment 4 Antti Koivisto 2015-01-05 09:46:02 PST
(In reply to comment #3)
> Comment on attachment 243978 [details]
> patch2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=243978&action=review
> 
> A tiny bit sad that something used this often got a little longer, but I
> suppose it’s strange that a font has a function that returns “a font”.

Yeah. We might even want to rename Font to something like StyleFont (since it is essentially CSS font definition) and FontData to Font.

> Do we even need a function for this?

Not really. I'll get rid of it.
Comment 5 Myles C. Maxfield 2015-01-05 13:28:16 PST
See https://bugs.webkit.org/show_bug.cgi?id=135290
Comment 6 Myles C. Maxfield 2015-01-05 13:31:44 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 243978 [details]
> > patch2
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=243978&action=review
> > 
> > A tiny bit sad that something used this often got a little longer, but I
> > suppose it’s strange that a font has a function that returns “a font”.
> 
> Yeah. We might even want to rename Font to something like StyleFont (since
> it is essentially CSS font definition) and FontData to Font.
> 
> > Do we even need a function for this?
> 
> Not really. I'll get rid of it.

I actually think that this is a good function to have. It lightens the cognitive load of code that calls this function, and I don't think there is any information lost between just using the name.
Comment 7 Antti Koivisto 2015-01-06 03:37:11 PST
https://trac.webkit.org/r177955
Comment 8 Antti Koivisto 2015-01-06 03:39:03 PST
(In reply to comment #6)
> I actually think that this is a good function to have. It lightens the
> cognitive load of code that calls this function, and I don't think there is
> any information lost between just using the name.

There were two functions with similar names. This one was used only in a few places internally in FontGlyphs and was really just creating confusion. Now "primary font data" is always a SimpleFontData.
Comment 9 Ahmad Saleem 2022-07-27 12:49:52 PDT
As per Comment 07, I did landed and there was no backout, I searched with bug number on Webkit Github.

Can this be marked as "RESOLVED FIXED"? Thanks!
Comment 10 Ryosuke Niwa 2022-07-27 14:27:25 PDT
Yup, fixed.
Comment 11 Radar WebKit Bug Importer 2022-07-27 14:28:16 PDT
<rdar://problem/97684797>