Add a font API to Pepper and implement on Linux based on agl's ...

Add a font API to Pepper and implement on Linux based on agl's code from http://codereview.chromium.org/2673003.


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

[agl, 2010-06-10 18:39:30]

LGTM for the Linux stuff.

http://codereview.chromium.org/2794004/diff/12001/13003
File chrome/renderer/renderer_main_platform_delegate_linux.cc (right):

http://codereview.chromium.org/2794004/diff/12001/13003#newcode13
chrome/renderer/renderer_main_platform_delegate_linux.cc:13: #include
"chrome/renderer/renderer_sandbox_support_linux.h"
This should probably be removed. Looks like it was left over from my
experimentation.

[brettw, 2010-06-10 18:45:02]

Good thing AGL checked the Linux stuff since I'm not particularly qualified.

LGTM

http://codereview.chromium.org/2794004/diff/12001/13001
File chrome/browser/renderer_host/render_sandbox_host_linux.cc (right):

http://codereview.chromium.org/2794004/diff/12001/13001#newcode9
chrome/browser/renderer_host/render_sandbox_host_linux.cc:9: #include <fcntl.h>
Be sure this is alphabetized.

http://codereview.chromium.org/2794004/diff/12001/13001#newcode33
chrome/browser/renderer_host/render_sandbox_host_linux.cc:33: #include
<fontconfig/fontconfig.h>
This should be up with the system includes.

http://codereview.chromium.org/2794004/diff/12001/13003
File chrome/renderer/renderer_main_platform_delegate_linux.cc (right):

http://codereview.chromium.org/2794004/diff/12001/13003#newcode13
chrome/renderer/renderer_main_platform_delegate_linux.cc:13: #include
"chrome/renderer/renderer_sandbox_support_linux.h"
Is this necessary for this file? If so, you should delete the blank line above
it.

http://codereview.chromium.org/2794004/diff/12001/13004
File chrome/renderer/renderer_sandbox_support_linux.cc (right):

http://codereview.chromium.org/2794004/diff/12001/13004#newcode5
chrome/renderer/renderer_sandbox_support_linux.cc:5: #include <sys/stat.h>
This should go below the .h file corresponding to this .cc file.

http://codereview.chromium.org/2794004/diff/12001/13005
File chrome/renderer/renderer_sandbox_support_linux.h (right):

http://codereview.chromium.org/2794004/diff/12001/13005#newcode36
chrome/renderer/renderer_sandbox_support_linux.h:36: // properties or -1 on
failure.
This is a read-only file descriptor, right? Can you explicitly mention this?

[darin (slow to review), 2010-06-10 18:47:50]

overall, this looks fine to me.  i defer to brettw and agl on the details.

http://codereview.chromium.org/2794004/diff/12001/13008
File third_party/npapi/bindings/npapi_extensions.h (right):

http://codereview.chromium.org/2794004/diff/12001/13008#newcode672
third_party/npapi/bindings/npapi_extensions.h:672: const char* face,
from an API point of view, a struct might be nice for grouping the input
parameters.  NPFontDesc?

[jam, 2010-06-11 17:37:59]

http://codereview.chromium.org/2794004/diff/12001/13001
File chrome/browser/renderer_host/render_sandbox_host_linux.cc (right):

http://codereview.chromium.org/2794004/diff/12001/13001#newcode9
chrome/browser/renderer_host/render_sandbox_host_linux.cc:9: #include <fcntl.h>
On 2010/06/10 18:45:03, brettw wrote:
> Be sure this is alphabetized.

Done.

http://codereview.chromium.org/2794004/diff/12001/13001#newcode33
chrome/browser/renderer_host/render_sandbox_host_linux.cc:33: #include
<fontconfig/fontconfig.h>
On 2010/06/10 18:45:03, brettw wrote:
> This should be up with the system includes.

Done.

http://codereview.chromium.org/2794004/diff/12001/13003
File chrome/renderer/renderer_main_platform_delegate_linux.cc (right):

http://codereview.chromium.org/2794004/diff/12001/13003#newcode13
chrome/renderer/renderer_main_platform_delegate_linux.cc:13: #include
"chrome/renderer/renderer_sandbox_support_linux.h"
On 2010/06/10 18:39:31, agl wrote:
> This should probably be removed. Looks like it was left over from my
> experimentation.

Done.

http://codereview.chromium.org/2794004/diff/12001/13004
File chrome/renderer/renderer_sandbox_support_linux.cc (right):

http://codereview.chromium.org/2794004/diff/12001/13004#newcode5
chrome/renderer/renderer_sandbox_support_linux.cc:5: #include <sys/stat.h>
On 2010/06/10 18:45:03, brettw wrote:
> This should go below the .h file corresponding to this .cc file.

Done.

http://codereview.chromium.org/2794004/diff/12001/13005
File chrome/renderer/renderer_sandbox_support_linux.h (right):

http://codereview.chromium.org/2794004/diff/12001/13005#newcode36
chrome/renderer/renderer_sandbox_support_linux.h:36: // properties or -1 on
failure.
On 2010/06/10 18:45:03, brettw wrote:
> This is a read-only file descriptor, right? Can you explicitly mention this?

Done.

http://codereview.chromium.org/2794004/diff/12001/13008
File third_party/npapi/bindings/npapi_extensions.h (right):

http://codereview.chromium.org/2794004/diff/12001/13008#newcode672
third_party/npapi/bindings/npapi_extensions.h:672: const char* face,
On 2010/06/10 18:47:51, darin wrote:
> from an API point of view, a struct might be nice for grouping the input
> parameters.  NPFontDesc?

Done.

[darin (slow to review), 2010-06-11 20:30:04]

LGTM