Added a fallback Win32 font information class for win32k lockdown.

Original patch by forshaw.

Added a fallback Win32 font information class for win32k lockdown.
This is to support running PDFIUM within the Win32k lockdown by
removing dependancies on USER32/GDI for the font information code.
It falls back to using a freetype/directory enumeration implementation
if it detects the win32k system calls have been disabled by policy.

BUG=523278
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/d7ba833df5c9dc70cb94dba2d0aae1339b345ae4

[forshaw, 2015-09-22 07:36:33]

Patchset #1 (id:1) has been deleted

[forshaw, 2015-09-22 07:36:40]

Patchset #1 (id:20001) has been deleted

[forshaw, 2015-09-29 13:01:21]

Patchset #1 (id:40001) has been deleted

[forshaw, 2015-09-29 13:04:05]

forshaw@chromium.org changed reviewers:
+ tsepez@chromium.org

[forshaw, 2015-09-29 13:04:06]

Could you please review this change to PDFium to add a fallback font manager
when running under win32k lockdown.

Thanks.

[Tom Sepez, 2015-09-29 16:26:04]

Thanks.  Wondering how much is windows-specific and how much could be made
common for all platforms.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
File core/src/fxge/win32/fx_win32_device.cpp (right):

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:224: CFX_ByteString face = cstr_face;
nit: prefer CFX_ByteString face(cstr_face);

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:225: int iBaseFont;
nit: move this into the for(), eg for (int iBaseFont = 0; ..

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:226: for (iBaseFont = 0; iBaseFont < 12;
iBaseFont++)
use FX_ArraySize(Base14Substs) in place of hard-coded 12.  At which point
iBaseFont needs to become a size_t, not an int.

nit: prefer ++iBaseFont for increment.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:227: if (face ==
CFX_ByteStringC(Base14Substs[iBaseFont].m_pName)) {
I thought CFX_Bytestring had an operator== method against char*.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:232: if (iBaseFont < 12) {
move this into the loop in place of the break, eg
  ...
  iExact = 1;
  return GetFont(Base14Substs[iBaseFont].m_pWinName);
}

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:251: static FX_DWORD
_Win32GetCharset(int charset) {
These are copied from linux or some such?  Rather than having two copies, can we
make a single one to be used by all platforms in the common file?

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:315: iSimilarValue += 64;
Doesn't this increment get lost when we make the assignment at 317?

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:534: // If GDI disabled then GetDC for
the desktop will fail. Select the fallback
nit: is disabled, then

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:549: } else {
nit: else after return not neeed.

[forshaw, 2015-09-29 16:46:23]

Thanks, looking to see if I can better merge *nix and Windows versions of the
class to increase code reuse without breaking too much.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
File core/src/fxge/win32/fx_win32_device.cpp (right):

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:224: CFX_ByteString face = cstr_face;
On 2015/09/29 16:26:03, Tom Sepez wrote:
> nit: prefer CFX_ByteString face(cstr_face);

Acknowledged.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:225: int iBaseFont;
On 2015/09/29 16:26:03, Tom Sepez wrote:
> nit: move this into the for(), eg for (int iBaseFont = 0; ..

Acknowledged.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:226: for (iBaseFont = 0; iBaseFont < 12;
iBaseFont++)
On 2015/09/29 16:26:03, Tom Sepez wrote:
> use FX_ArraySize(Base14Substs) in place of hard-coded 12.  At which point
> iBaseFont needs to become a size_t, not an int.
> 
> nit: prefer ++iBaseFont for increment.

Acknowledged.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:227: if (face ==
CFX_ByteStringC(Base14Substs[iBaseFont].m_pName)) {
On 2015/09/29 16:26:03, Tom Sepez wrote:
> I thought CFX_Bytestring had an operator== method against char*.

Acknowledged.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:232: if (iBaseFont < 12) {
On 2015/09/29 16:26:03, Tom Sepez wrote:
> move this into the loop in place of the break, eg
>   ...
>   iExact = 1;
>   return GetFont(Base14Substs[iBaseFont].m_pWinName);
> }

Acknowledged.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:251: static FX_DWORD
_Win32GetCharset(int charset) {
On 2015/09/29 16:26:03, Tom Sepez wrote:
> These are copied from linux or some such?  Rather than having two copies, can
we
> make a single one to be used by all platforms in the common file?

Yeah ideally it would be nicer to just have a single implementation across
Linux, OSX and Windows however I didn't want to break too much. I can see if I
can make a common version supporting only the minor changes between platforms.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:315: iSimilarValue += 64;
On 2015/09/29 16:26:03, Tom Sepez wrote:
> Doesn't this increment get lost when we make the assignment at 317?

Looks that way, this was copied from the Linux version so probably a bug there
as well.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:534: // If GDI disabled then GetDC for
the desktop will fail. Select the fallback
On 2015/09/29 16:26:03, Tom Sepez wrote:
> nit: is disabled, then

Acknowledged.

https://codereview.chromium.org/1306883002/diff/60001/core/src/fxge/win32/fx_...
core/src/fxge/win32/fx_win32_device.cpp:549: } else {
On 2015/09/29 16:26:03, Tom Sepez wrote:
> nit: else after return not neeed.

Acknowledged.

[forshaw, 2015-10-01 11:44:27]

Patchset #2 (id:80001) has been deleted

[forshaw, 2015-10-01 12:16:59]

Patchset #2 (id:100001) has been deleted

[forshaw, 2015-10-01 13:07:59]

tsepez@ PTAL. I've moved some code shared between Linux and Windows into the
common base. I don't want to change too much and risk regressions in the Linux
code. The code looks pretty fragile, lots of "hacks" which no doubt have built
up in response to rendering problems. If this is committed perhaps look to the
future of reimplementing a common base using Skia or similar.

[Tom Sepez, 2015-10-01 15:55:56]

lgtm

https://codereview.chromium.org/1306883002/diff/120001/core/src/fxge/ge/fx_ge...
File core/src/fxge/ge/fx_ge_fontmap.cpp (right):

https://codereview.chromium.org/1306883002/diff/120001/core/src/fxge/ge/fx_ge...
core/src/fxge/ge/fx_ge_fontmap.cpp:1436: return NULL;
nit: prefer nullptr to NULL.  We've been changing these as we hit them.

[Tom Sepez, 2015-10-01 16:25:39]

Actually, this seems to have produced different results for the test on linux,
presumably because some other font is being selected under the new patch than
under the old.

You should try running testing/tools/run_corpus_tests.py, both before and after
the patch, and try to account for any differences (the expected results files
match the bots; we're still trying to get font portability for these tests
(sorry)).

[forshaw, 2015-10-05 13:28:23]

tsepez@ PTAL. I worked out what I did wrong, I used the wrong subst table so
this fixes the linux regressions and also fixes some regressions in the Windows
version. Currently there seems to be 4 failures in the original Windows GDI font
info class when running the corpus tests (whether this patch is used or not),
but 0 failures when using the fallback class so that's actually an improvement,
in theory. So I hope it's now suitable for committing.

[Tom Sepez, 2015-10-05 19:54:11]

lgtm

[Tom Sepez, 2015-10-05 19:56:34]

On 2015/10/05 19:54:11, Tom Sepez wrote:
> lgtm
I'm going to land this, stay tuned.

[Tom Sepez, 2015-10-05 19:57:23]

Committed patchset #3 (id:140001) manually as
d7ba833df5c9dc70cb94dba2d0aae1339b345ae4 (presubmit successful).