Do not leak FamilyData in Android SkFontMgr.

Do not leak FamilyData in Android SkFontMgr.

Currently, it appears one FamilyData is leaking per scanned file.
This also cleans up the hanging currentFamily memory management.

Committed: https://skia.googlesource.com/skia/+/7a3527d7a307c62432cc81ce497a174b15f62d68

[bungeman-skia, 2015-01-30 22:58:09]

bungeman@google.com changed reviewers:
+ djsollen@google.com, mtklein@google.com, tomhudson@google.com

[bungeman-skia, 2015-01-30 22:58:13]

The CQ bit was checked by bungeman@google.com

[commit-bot: I haz the power, 2015-01-30 22:58:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/889943005/1

[commit-bot: I haz the power, 2015-01-30 22:58:37]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-01-31 04:58 UTC

[mtklein, 2015-01-30 23:07:42]

https://codereview.chromium.org/889943005/diff/1/src/ports/SkFontConfigParser...
File src/ports/SkFontConfigParser_android.cpp (right):

https://codereview.chromium.org/889943005/diff/1/src/ports/SkFontConfigParser...
src/ports/SkFontConfigParser_android.cpp:62: SkTDArray<FontFamily*>& families;  
     // The array to append families, owned by caller
Who cleans up things we add here?

https://codereview.chromium.org/889943005/diff/1/src/ports/SkFontConfigParser...
src/ports/SkFontConfigParser_android.cpp:63: SkAutoTDelete<FontFamily>
currentFamily; // The family being created, owned by this
So, uh.... we were just never deleting this?  I can't even find a "delete" in
here.

https://codereview.chromium.org/889943005/diff/1/src/ports/SkFontConfigParser...
src/ports/SkFontConfigParser_android.cpp:192: FontFamily* family = new
FontFamily();
E.g. I'm curious who calls delete on this family.

[bungeman-skia, 2015-01-30 23:14:56]

https://codereview.chromium.org/889943005/diff/1/src/ports/SkFontConfigParser...
File src/ports/SkFontConfigParser_android.cpp (right):

https://codereview.chromium.org/889943005/diff/1/src/ports/SkFontConfigParser...
src/ports/SkFontConfigParser_android.cpp:62: SkTDArray<FontFamily*>& families;  
     // The array to append families, owned by caller
On 2015/01/30 23:07:41, mtklein wrote:
> Who cleans up things we add here?

This is completely owned by the caller (this is the families passe into
GetFontFamilies). Currently, the caller of that is also leaking these, I think,
but that's another CL I'm working on.

https://codereview.chromium.org/889943005/diff/1/src/ports/SkFontConfigParser...
src/ports/SkFontConfigParser_android.cpp:63: SkAutoTDelete<FontFamily>
currentFamily; // The family being created, owned by this
On 2015/01/30 23:07:41, mtklein wrote:
> So, uh.... we were just never deleting this?  I can't even find a "delete" in
> here.

This is the 'working' FontFamily. When we finish building it, it is appended to
families. This simplifies things, because it may be possible to have a 'bad'
family which should never end up in the list ever. However, it was possible in
error conditions that one of these could just never end up being freed (it's
still possible, just less likely now, that's another CL).

https://codereview.chromium.org/889943005/diff/1/src/ports/SkFontConfigParser...
src/ports/SkFontConfigParser_android.cpp:192: FontFamily* family = new
FontFamily();
On 2015/01/30 23:07:41, mtklein wrote:
> E.g. I'm curious who calls delete on this family.

This is added to the set of families being appended (see line 200). As a result,
it is up to the caller. This is the same situation as outlined above.

[mtklein, 2015-01-30 23:23:04]

wow.

lgtm.

Any interest in taking on SkUniquePtr / SkUniquePtr::Make?

[commit-bot: I haz the power, 2015-01-30 23:24:53]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/7a3527d7a307c62432cc81ce497a174b15f62d68