Sanity check the values of TRUE and FALSE.

Sanity check the values of TRUE and FALSE.

Get rid of cond ? TRUE : FALSE.

R=tsepez@chromium.org

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

[Lei Zhang, 2015-10-15 00:40:52]

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

[Lei Zhang, 2015-10-15 00:40:53]

FX_BOOL isn't dead yet, and we may need TRUE/FALSE for C code.

[Tom Sepez, 2015-10-15 17:08:31]

https://codereview.chromium.org/1405723003/diff/1/core/include/fxcrt/fx_system.h
File core/include/fxcrt/fx_system.h (right):

https://codereview.chromium.org/1405723003/diff/1/core/include/fxcrt/fx_syste...
core/include/fxcrt/fx_system.h:99: static_assert(TRUE == 1,
"true_needs_to_be_one");
If it's not C++, we don't get static_assert, do we?
And we just defined them right above; I doubt we'd muck with them.

https://codereview.chromium.org/1405723003/diff/1/core/include/fxge/fx_dib.h
File core/include/fxge/fx_dib.h (right):

https://codereview.chromium.org/1405723003/diff/1/core/include/fxge/fx_dib.h#...
core/include/fxge/fx_dib.h:199: FX_BOOL IsAlphaMask() const { return m_AlphaFlag
== 1; }
This one feels wrong.  If its a mask, all bits should be independent.  Can you
look into this? Or at least write a comment about if the value is 1, then its
one thing, otherwise its a bitmask.

https://codereview.chromium.org/1405723003/diff/1/core/include/fxge/fx_dib.h#...
core/include/fxge/fx_dib.h:201: FX_BOOL HasAlpha() const { return m_AlphaFlag &
2; }
Actually, what you want is { return !!(m_AlphaFlag & 2); } to force 0/1-ness. 
The !! idiom is well-known so we can use it without elaboration.

https://codereview.chromium.org/1405723003/diff/1/core/src/fpdfapi/fpdf_rende...
File core/src/fpdfapi/fpdf_render/fpdf_render.cpp (right):

https://codereview.chromium.org/1405723003/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render.cpp:762: bGroupTransparent =
Transparency & PDFTRANS_ISOLATED;
Again !!(Transparency & PDFTRANS_ISOLATED);

https://codereview.chromium.org/1405723003/diff/1/core/src/fxcodec/jbig2/JBig...
File core/src/fxcodec/jbig2/JBig2_Context.cpp (right):

https://codereview.chromium.org/1405723003/diff/1/core/src/fxcodec/jbig2/JBig...
core/src/fxcodec/jbig2/JBig2_Context.cpp:384: pPageInfo->m_bIsStriped = (wTemp
>> 15) & 1;
nit: prefer !!(wTemp & 0xSomething)

https://codereview.chromium.org/1405723003/diff/1/core/src/fxge/dib/fx_dib_co...
File core/src/fxge/dib/fx_dib_composite.cpp (right):

https://codereview.chromium.org/1405723003/diff/1/core/src/fxge/dib/fx_dib_co...
core/src/fxge/dib/fx_dib_composite.cpp:4059: FX_BOOL isSrcCmyk = src_format &
0x0400;
You get the picture.

https://codereview.chromium.org/1405723003/diff/1/fpdfsdk/src/fpdfview.cpp
File fpdfsdk/src/fpdfview.cpp (right):

https://codereview.chromium.org/1405723003/diff/1/fpdfsdk/src/fpdfview.cpp#ne...
fpdfsdk/src/fpdfview.cpp:59: return foxit_sandbox_policy & 0x01;
nit: !! would be redundant, but foolproof should we move this to bit 2.

[Lei Zhang, 2015-10-19 08:56:18]

https://codereview.chromium.org/1405723003/diff/1/core/include/fxcrt/fx_system.h
File core/include/fxcrt/fx_system.h (right):

https://codereview.chromium.org/1405723003/diff/1/core/include/fxcrt/fx_syste...
core/include/fxcrt/fx_system.h:99: static_assert(TRUE == 1,
"true_needs_to_be_one");
On 2015/10/15 17:08:31, Tom Sepez wrote:
> If it's not C++, we don't get static_assert, do we?
> And we just defined them right above; I doubt we'd muck with them.

I'll just drop this then.

https://codereview.chromium.org/1405723003/diff/1/core/include/fxge/fx_dib.h
File core/include/fxge/fx_dib.h (right):

https://codereview.chromium.org/1405723003/diff/1/core/include/fxge/fx_dib.h#...
core/include/fxge/fx_dib.h:199: FX_BOOL IsAlphaMask() const { return m_AlphaFlag
== 1; }
On 2015/10/15 17:08:31, Tom Sepez wrote:
> This one feels wrong.  If its a mask, all bits should be independent.  Can you
> look into this? Or at least write a comment about if the value is 1, then its
> one thing, otherwise its a bitmask.

I looked at this briefly and added comments. With our current test repo, we only
ever see values of 0-2, though 4 should be possible with a test case.

https://codereview.chromium.org/1405723003/diff/1/core/include/fxge/fx_dib.h#...
core/include/fxge/fx_dib.h:201: FX_BOOL HasAlpha() const { return m_AlphaFlag &
2; }
On 2015/10/15 17:08:31, Tom Sepez wrote:
> Actually, what you want is { return !!(m_AlphaFlag & 2); } to force 0/1-ness. 
> The !! idiom is well-known so we can use it without elaboration.

Done.

https://codereview.chromium.org/1405723003/diff/1/core/src/fpdfapi/fpdf_rende...
File core/src/fpdfapi/fpdf_render/fpdf_render.cpp (right):

https://codereview.chromium.org/1405723003/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render.cpp:762: bGroupTransparent =
Transparency & PDFTRANS_ISOLATED;
On 2015/10/15 17:08:31, Tom Sepez wrote:
> Again !!(Transparency & PDFTRANS_ISOLATED);

Done.

https://codereview.chromium.org/1405723003/diff/1/core/src/fxcodec/jbig2/JBig...
File core/src/fxcodec/jbig2/JBig2_Context.cpp (right):

https://codereview.chromium.org/1405723003/diff/1/core/src/fxcodec/jbig2/JBig...
core/src/fxcodec/jbig2/JBig2_Context.cpp:384: pPageInfo->m_bIsStriped = (wTemp
>> 15) & 1;
On 2015/10/15 17:08:31, Tom Sepez wrote:
> nit: prefer !!(wTemp & 0xSomething)

Done.

https://codereview.chromium.org/1405723003/diff/1/core/src/fxge/dib/fx_dib_co...
File core/src/fxge/dib/fx_dib_composite.cpp (right):

https://codereview.chromium.org/1405723003/diff/1/core/src/fxge/dib/fx_dib_co...
core/src/fxge/dib/fx_dib_composite.cpp:4059: FX_BOOL isSrcCmyk = src_format &
0x0400;
On 2015/10/15 17:08:31, Tom Sepez wrote:
> You get the picture.

Done.

https://codereview.chromium.org/1405723003/diff/1/fpdfsdk/src/fpdfview.cpp
File fpdfsdk/src/fpdfview.cpp (right):

https://codereview.chromium.org/1405723003/diff/1/fpdfsdk/src/fpdfview.cpp#ne...
fpdfsdk/src/fpdfview.cpp:59: return foxit_sandbox_policy & 0x01;
On 2015/10/15 17:08:31, Tom Sepez wrote:
> nit: !! would be redundant, but foolproof should we move this to bit 2.

Done.

[Tom Sepez, 2015-10-19 16:28:17]

lgtm

[Lei Zhang, 2015-10-19 19:00:08]

Committed patchset #2 (id:20001) manually as
aff4635dad81bc319266d9d84b81552580cd2b65 (presubmit successful).