Move legacyCreateTypeface to SkFontStyle.

content/common/font_config_ipc_linux.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/common/font_config_ipc_linux.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <stdint.h>
 #include <sys/mman.h>
@@ skipping 46 matching lines @@
 FontConfigIPC::FontConfigIPC(int fd)
     : fd_(fd)
     , mapped_typefaces_(kMaxMappedTypefaces) {
 }
 
 FontConfigIPC::~FontConfigIPC() {
   CloseFD(fd_);
 }
 
 bool FontConfigIPC::matchFamilyName(const char familyName[],
-                                    SkTypeface::Style requestedStyle,
+                                    SkFontStyle requestedStyle,
                                     FontIdentity* outFontIdentity,
                                     SkString* outFamilyName,
-                                    SkTypeface::Style* outStyle) {
+                                    SkFontStyle* outStyle) {
   TRACE_EVENT0("sandbox_ipc", "FontConfigIPC::matchFamilyName");
   size_t familyNameLen = familyName ? strlen(familyName) : 0;
   if (familyNameLen > kMaxFontFamilyLength)
     return false;
 
   base::Pickle request;
   request.WriteInt(METHOD_MATCH);
   request.WriteData(familyName, familyNameLen);
-  request.WriteUInt32(requestedStyle);
+  skia::WriteSkFontStyle(&request, requestedStyle);
 
   uint8_t reply_buf[2048];
   const ssize_t r = base::UnixDomainSocket::SendRecvMsg(
       fd_, reply_buf, sizeof(reply_buf), NULL, request);
   if (r == -1)
     return false;
 
   base::Pickle reply(reinterpret_cast<char*>(reply_buf), r);
   base::PickleIterator iter(reply);
   bool result;
   if (!iter.ReadBool(&result))
     return false;
   if (!result)
     return false;
 
   SkString     reply_family;
   FontIdentity reply_identity;
-  uint32_t     reply_style;
+  SkFontStyle  reply_style;
   if (!skia::ReadSkString(&iter, &reply_family) ||
       !skia::ReadSkFontIdentity(&iter, &reply_identity) ||
-      !iter.ReadUInt32(&reply_style)) {
+      !skia::ReadSkFontStyle(&iter, &reply_style)) {
     return false;
   }
 
   if (outFontIdentity)
     *outFontIdentity = reply_identity;
   if (outFamilyName)
     *outFamilyName = reply_family;
   if (outStyle)
-    *outStyle = static_cast<SkTypeface::Style>(reply_style);
+    *outStyle = reply_style;

[dcheng, 2016/04/12 00:01:24]

I don't think I conveyed my intent clearly: if there are explicit FromUint32()
and ToUint32() methods SkFontStyle (which replace the uint32_t conversion
constructor and operator), then we can just write this as:
  uint32_t reply_style_value;
  if (... || !iter.ReadUInt32(&reply_style_value)) return false;
  if (outStyle)
    *outStyle = SkFontStyle::FromUint32(reply_style_value);

Similarly, the other serialization bit can just look like this:
  request.WriteUint32(requestedStyle.ToUint32());

[bungeman-chromium, 2016/04/12 16:35:36]

This is a hard sell to Skia though. The uint32_t conversion constructor and
operator were added in the Skia CL to make this CL simpler so that how to
properly serialize it could go in another CL. However, as you pointed out, it
isn't ideal to hide what's going on that way, and also these 'invisible'
explicit conversions were actually already causing the introduction of bugs in
Skia (my reviewer really didn't like them too). But at least they were
'invisible' and not likely to be used. Adding named ReadUInt32 and FromUInt32
methods is quite visible and rather limiting to future design. It has been and
will be a lot of work to go from SkTypeface::Style to SkFontStyle mostly because
SkTypeface::Style implementation was overly exposed and couldn't be updated. In
other words, Skia has been bitten by fixed sizes before, and doesn't want to
have public methods to expose those sizes, because those public methods will end
up abused by some user of Skia other than Chromium in some strange way.
base::TimeTicks has the advantage that it's a type in an application (rather
than a library).

Is there any particular reason why the current proposal (PS6,
skia/ext/skia_utils_base) is undesirable? It is currently 48/64bits instead of
32bits, but that could be fixed (essentially be ReadUInt32 and FromUInt32 but on
the Chromium side, not constraining SkFontStyle from changing its layout, and
not going to be used for other purposes). Mostly I did it this way because it
seems like it should make the fewest people uncomfortable, and also follows the
example of SkFontIdentity which seems to be fairly similar. On the other hand,
if you're not comfortable with the current proposal I'll try to see if there is
some other way. If adding ReadUInt32 and FromUInt32 to SkFontStyle is the only
way to get this in, I'll try to fight for that, but I'd prefer not to if it can
be avoided.

 
   return true;
 }
 
 static void DestroyMemoryMappedFile(const void*, void* context) {
   base::ThreadRestrictions::ScopedAllowIO allow_munmap;
   delete static_cast<base::MemoryMappedFile*>(context);
 }
 
 SkMemoryStream* FontConfigIPC::mapFileDescriptorToStream(int fd) {
@@ skipping 49 matching lines @@
   if (!typeface_stream)
     return nullptr;
   skia::RefPtr<SkTypeface> typeface_from_stream = skia::AdoptRef(
       SkTypeface::CreateFromStream(typeface_stream, identity.fTTCIndex));
   auto mapped_typefaces_insert_it =
       mapped_typefaces_.Put(identity, typeface_from_stream);
   return SkSafeRef(mapped_typefaces_insert_it->second.get());
 }
 
 }  // namespace content