Mac: More pluming for OOP font loading

Mac: More pluming for OOP font loading

* Add font_descriptor and corresponding pluming to send an NSFont over IPC. 

* Rejigger font_loader to accept an NSFont as input and output an ATSFontContainerRef.  The reasoning behind this is that WebKit ultimately controls the font lifetime and we can only deactivate the font container once the font is no longer in use.

BUG=29729
Test=Unit tests should pass.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50076

[Avi (use Gerrit), 2010-06-14 20:43:46]

A few nits.

http://codereview.chromium.org/2804001/diff/4001/5005
File chrome/common/font_descriptor_mac_unittest.mm (right):

http://codereview.chromium.org/2804001/diff/4001/5005#newcode23
chrome/common/font_descriptor_mac_unittest.mm:23: ATSUFontID id2 =
CTFontGetPlatformFont(reinterpret_cast<CTFontRef>(font2), 0);
CTFontGetPlatformFont returns ATSFontRef not ATSUFontID. Since ATSUI is
discouraged (if not formally deprecated) and you don't use the values further in
this test, we might as well use the correct type.

http://codereview.chromium.org/2804001/diff/4001/5007
File chrome/common/font_loader_mac.mm (right):

http://codereview.chromium.org/2804001/diff/4001/5007#newcode106
chrome/common/font_loader_mac.mm:106: // A value of 3 means the font is private
and can't be seen by anyone else.
I'd prefer turning this into a constant. Where did you learn that 3 is magical?

http://codereview.chromium.org/2804001/diff/4001/5010
File chrome/common/sandbox_mac_fontloading_unittest.mm (right):

http://codereview.chromium.org/2804001/diff/4001/5010#newcode20
chrome/common/sandbox_mac_fontloading_unittest.mm:20: bool
CGFontFromFontcontainer(ATSFontContainerRef container, CGFontRef* out) {
CGFontFromFontContainer? It seems that "font container" is two words.

[jeremy, 2010-06-15 07:03:10]

All fixed, ready for another look...

[jam, 2010-06-15 07:38:29]

I know next to nothing about fonts, so I defer to Adam.

[Avi (use Gerrit), 2010-06-15 13:57:47]

LGTM with slight nit

http://codereview.chromium.org/2804001/diff/11001/12005
File chrome/common/font_descriptor_mac_unittest.mm (right):

http://codereview.chromium.org/2804001/diff/11001/12005#newcode26
chrome/common/font_descriptor_mac_unittest.mm:26: LOG(ERROR) << "ATSUFontIDs for
"
Change the error to refer to ATSFontRefs too.

http://codereview.chromium.org/2804001/diff/11001/12007
File chrome/common/font_loader_mac.mm (right):

http://codereview.chromium.org/2804001/diff/11001/12007#newcode107
chrome/common/font_loader_mac.mm:107: // This is the value used by WebKit when
activating remote fonts.
Nice comment; I like it.

[agl, 2010-06-15 14:10:14]

http://codereview.chromium.org/2804001/diff/11001/12009
File chrome/common/render_messages_internal.h (right):

http://codereview.chromium.org/2804001/diff/11001/12009#newcode1442
chrome/common/render_messages_internal.h:1442:
IPC_SYNC_MESSAGE_CONTROL1_2(ViewHostMsg_LoadFont,
Am I just being dumb, or is the implementation of this message missing from this
CL?

http://codereview.chromium.org/2804001/diff/11001/12011
File ipc/ipc_message_utils.h (right):

http://codereview.chromium.org/2804001/diff/11001/12011#newcode239
ipc/ipc_message_utils.h:239: const char *data;
Seems over complex:

const char* data;
int data_size;
if (!m->ReadData(iter, &data, &data_size) ||
    data_size != sizeof(param_type)) {
  NOTREACHED();
  return false;
}

memcpy(r, data, sizeof(param_type));
return true;

[jeremy, 2010-06-15 14:26:55]

http://codereview.chromium.org/2804001/diff/11001/12009
File chrome/common/render_messages_internal.h (right):

http://codereview.chromium.org/2804001/diff/11001/12009#newcode1442
chrome/common/render_messages_internal.h:1442:
IPC_SYNC_MESSAGE_CONTROL1_2(ViewHostMsg_LoadFont,
Due to the way the WebKit and Chrome code interact I can't land the
implementation together with the definition since it depends on code on the
WebKit side.

But this basically makes a straightforward call to font_loader.mm .

http://codereview.chromium.org/2804001/diff/11001/12011
File ipc/ipc_message_utils.h (right):

http://codereview.chromium.org/2804001/diff/11001/12011#newcode239
ipc/ipc_message_utils.h:239: const char *data;
Agreed, I wanted to match the definition for double, I'll change both before
landing.