Only SkFontMgr_New_DirectWrite should call GetSystemFontCollection.

Only SkFontMgr_New_DirectWrite should call GetSystemFontCollection.

Aside from the factory helper, there should be no calls to
IDWriteFactory::GetSystemFontCollection. All other code should work in
terms of IDWriteFontCollection for flexibility and correctness.

R=caryclark@google.com

Committed: https://code.google.com/p/skia/source/detail?r=12331

[bungeman-skia, 2013-11-20 18:45:26]

This is a continuation of https://codereview.chromium.org/76763002/ to update
the DirectWrite backend to the new interfaces, as well as moving toward
isolating functionality for cross-process use.

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.cpp
File src/ports/SkFontHost_win_dw.cpp (left):

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:634: #elif SK_DWRITE_DEFAULT_FONT_STRATEGY ==
SK_DWRITE_DEFAULT_FONT_MESSAGE
Previously, everything but this block was effectively ifdef-ed out. This change
moves this into getDefaultFont.

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:781: void SkDWriteFontFromTypeface(const
SkTypeface* face, IDWriteFont** font) {
This was a caller to get_default_font. However, it is not used and has no
external declaration. It was put here when mirroring the gdi backend was a
priority, but this capability should no longer be needed with the newer
interfaces.

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:1434: static HRESULT get_by_family_name(const
char familyName[], IDWriteFontFamily** fontFamily) {
This is moved into getByFamilyName and changed to use fFontCollection instead of
the system font collection.

[caryclark, 2013-11-20 19:09:14]

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.cpp
File src/ports/SkFontHost_win_dw.cpp (right):

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:133: HRESULT getByFamilyName(const char
familyName[], IDWriteFontFamily** fontFamily);
why return HRESULT if caller doesn't check it?

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:134: HRESULT getDefaultFont(IDWriteFont** font);
looks like the existing pattern is private helper functions, then private data
members

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:1775: }
getDefaultFont could be a local static function, right? Or did I miss where it
depends on SkFontMgr_DirectWrite?

[bungeman-skia, 2013-11-20 21:38:38]

Did some more work to address comments pointing out the fundamental weaknesses
in the existing code.

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.cpp
File src/ports/SkFontHost_win_dw.cpp (right):

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:133: HRESULT getByFamilyName(const char
familyName[], IDWriteFontFamily** fontFamily);
On 2013/11/20 19:09:14, caryclark wrote:
> why return HRESULT if caller doesn't check it?

Re-worked, it's now checked (at least in one place).

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:134: HRESULT getDefaultFont(IDWriteFont** font);
On 2013/11/20 19:09:14, caryclark wrote:
> looks like the existing pattern is private helper functions, then private data
> members

Done.

https://codereview.chromium.org/77023006/diff/1/src/ports/SkFontHost_win_dw.c...
src/ports/SkFontHost_win_dw.cpp:1775: }
On 2013/11/20 19:09:14, caryclark wrote:
> getDefaultFont could be a local static function, right? Or did I miss where it
> depends on SkFontMgr_DirectWrite?

That was going to be my next change, to get rid of the gdi interop for the
default font, since under the hood it interacts with the system font collection.
Since it simplifies the code a bit and addresses this comment, I've done that.

[caryclark, 2013-11-20 21:45:52]

lgtm

[bungeman-skia, 2013-11-20 22:42:16]

Committed patchset #2 manually as r12331 (presubmit successful).