Fix font loading in OOP PDF

Fix font loading in OOP PDF

This does two things
- avoids pdf.dll being loaded into the renderer when OOP PDF is
  being used (this was caused by the patching)
- does the patch in PpapiThread when pdf.dll is loaded there for OOP

The IAT patch to GetFontData allows various users of GDI font methods
to succeed when sandboxed.

R=cpu@chromium.org,raymes@chromium.org,ananta@chromium.org
BUG=448473
Committed: https://crrev.com/55a856eb124f941c191c62dab40055b827186395
Cr-Commit-Position: refs/heads/master@{#311730}

[ananta, 2015-01-15 02:31:22]

Looks good.

https://codereview.chromium.org/854773002/diff/20001/content/ppapi_plugin/ppa...
File content/ppapi_plugin/ppapi_thread.cc (right):

https://codereview.chromium.org/854773002/diff/20001/content/ppapi_plugin/ppa...
content/ppapi_plugin/ppapi_thread.cc:98: // renderer once PDF is always OOP
and/or PDF is made to use Skia instead of GDI
Move the once PDF is always OOP comment portion to the other file.

https://codereview.chromium.org/854773002/diff/20001/content/ppapi_plugin/ppa...
content/ppapi_plugin/ppapi_thread.cc:375: // be removed once we switch PDF to
use Skia.
Some commentary which explains this voodoo to get fonts to work would be
helpful.

[scottmg, 2015-01-15 02:35:38]

https://codereview.chromium.org/854773002/diff/20001/content/ppapi_plugin/ppa...
File content/ppapi_plugin/ppapi_thread.cc (right):

https://codereview.chromium.org/854773002/diff/20001/content/ppapi_plugin/ppa...
content/ppapi_plugin/ppapi_thread.cc:98: // renderer once PDF is always OOP
and/or PDF is made to use Skia instead of GDI
On 2015/01/15 02:31:22, ananta wrote:
> Move the once PDF is always OOP comment portion to the other file.

Done.

https://codereview.chromium.org/854773002/diff/20001/content/ppapi_plugin/ppa...
content/ppapi_plugin/ppapi_thread.cc:375: // be removed once we switch PDF to
use Skia.
On 2015/01/15 02:31:22, ananta wrote:
> Some commentary which explains this voodoo to get fonts to work would be
> helpful. 

Done, above at the PreCache...  call.

[raymes, 2015-01-15 03:08:01]

Thanks Scott!!

I defer to the others - the only part I understand is the pepper API call you
added and that looks fine. Note that it takes a LOGFONTW but I assume in this
case it's the same as LOGFONT

[scottmg, 2015-01-15 03:40:10]

On 2015/01/15 03:08:01, raymes wrote:
> Thanks Scott!!
> 
> I defer to the others - the only part I understand is the pepper API call you
> added and that looks fine. Note that it takes a LOGFONTW but I assume in this
> case it's the same as LOGFONT

I don't understand the locking requirements, do you? I added the
"ppapi::ProxyAutoLock lock;" because the function it's calling does
ProxyAutoUnlock in the body, but I'm honestly not sure what we're serializing
access to. This seemed relatively safe (in that it'll CHECK if the locking is
wrong) but it's obviously not pretty.

[cpu_(ooo_6.6-7.5), 2015-01-15 03:41:28]

lgtm

[raymes, 2015-01-15 03:44:19]

We're serializing access into the pepper API as a whole. This stuff was added so
that you could safely call pepper APIs from multiple threads. Usually the
locking is done automatically but because you're calling a function internally
in Chrome you need to lock yourself. I hope that makes sense.

[scottmg, 2015-01-15 04:04:25]

On 2015/01/15 03:44:19, raymes wrote:
> We're serializing access into the pepper API as a whole. This stuff was added
so
> that you could safely call pepper APIs from multiple threads. Usually the
> locking is done automatically but because you're calling a function internally
> in Chrome you need to lock yourself. I hope that makes sense.

OK, if you're happy, I am. :)

[scottmg, 2015-01-15 04:04:28]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2015-01-15 04:05:07]

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

[commit-bot: I haz the power, 2015-01-15 04:37:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-15 04:37:40]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[scottmg, 2015-01-15 04:40:04]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2015-01-15 04:41:11]

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

[commit-bot: I haz the power, 2015-01-15 05:09:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-15 05:09:54]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[scottmg, 2015-01-15 17:01:53]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2015-01-15 17:02:14]

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

[commit-bot: I haz the power, 2015-01-15 17:02:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-15 17:02:46]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[scottmg, 2015-01-15 20:58:26]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2015-01-15 20:59:43]

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

[commit-bot: I haz the power, 2015-01-15 21:01:16]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-01-15 21:02:30]

Patchset 4 (id:??) landed as
https://crrev.com/55a856eb124f941c191c62dab40055b827186395
Cr-Commit-Position: refs/heads/master@{#311730}

[jam, 2015-02-02 20:51:39]

belated comment: this is a layering violation, since PDF is a chrome thing and
doesn't below in chrome. i'll fix this in a followup

[scottmg, 2015-02-02 20:55:21]

On 2015/02/02 20:51:39, jam wrote:
> belated comment: this is a layering violation, since PDF is a chrome thing and
> doesn't below in chrome. i'll fix this in a followup

Oops, sorry. You mean content shouldn't know about PDF, right?