Add type cast definitions for CPDF_Stream.

Add type cast definitions for CPDF_Stream.

This Cl adds ToStream, CPDF_Object::AsStream and CPDF_Object::IsStream and
updates the src to use them as needed.

BUG=pdfium:201
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/338805f1366dcdf9a5b48cf591541cf98d7490f1

[dsinclair, 2015-10-22 15:44:29]

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

[dsinclair, 2015-10-22 15:44:29]

PTAL.

This one has a bunch of const_cast<CPDF_Stream*>'s in there. We will need to go
through at some point and sort out the const'ness of things.

[Tom Sepez, 2015-10-22 17:16:45]

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_edit/...
File core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_edit/...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:946: return !pDict ||
pDict->GetString(FX_BSTRC("Type")) != FX_BSTRC("XRef");
nit: this is an int32_t function so it probably should return an int32_t rather
than a bool result promoted as such.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp (right):

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp:1032: if (!pStream ||
!pStream->IsStream())
how can a CPDF_Stream not be IsStream() ?

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:789: if
((pDict->KeyExist(FX_BSTRC("Type"))) &&
nit: maybe combine with previous line.  Also check for overparenthesization.

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

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:810: if (!m_pMaskStream)
nit: maybe 
  return ToStream(m_pMaskStream) ? StartLoadMaskDIB() : 1;

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:849: int ret =
m_pMask->StartLoadDIBSource(m_pDocument, ToStream(m_pMaskStream),
I worry about this.  At line 809, we may have assigned a non-stream to
m_pMaskStream.  Now we will test type and pass NULL for that case.  Later on,
some clever code may check that the pointer is some other type, and sloppily
cast it to the type.  Please confirm.  Can we make the m_MaskStream be a stream,
not an object?  Can we force ToStream() on it at the time of assignment?

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

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:411: static void
_DrawFreeGouraudShading(CFX_DIBitmap* pBitmap,
nit: as long as you're mucking with this, can we lose the leading _ ? _CAPITAL
is reserved for the implementation and should not be used.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:419: if
(!pShadingStream->IsStream())
Can a stream be not IsStream() ?  Either this should take an object argument and
we call ToStream here, or the callers should call ToStream() and we do a null
check here.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:455: if
(!pShadingStream->IsStream())
ditto

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:761: if
(!pShadingStream->IsStream())
ditto ditto

[dsinclair, 2015-10-22 17:42:56]

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_edit/...
File core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_edit/...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:946: return !pDict ||
pDict->GetString(FX_BSTRC("Type")) != FX_BSTRC("XRef");
On 2015/10/22 17:16:45, Tom Sepez wrote:
> nit: this is an int32_t function so it probably should return an int32_t
rather
> than a bool result promoted as such.

Done.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp (right):

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp:1032: if (!pStream ||
!pStream->IsStream())
On 2015/10/22 17:16:45, Tom Sepez wrote:
> how can a CPDF_Stream not be IsStream() ?


I'm guessing by LoadAllData((CPDF_Stream*)someObject, ....);

Shouldn't be required now, removed.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:789: if
((pDict->KeyExist(FX_BSTRC("Type"))) &&
On 2015/10/22 17:16:45, Tom Sepez wrote:
> nit: maybe combine with previous line.  Also check for overparenthesization.

This if is already kinda crazy, prefer to leave as two separate ifs.

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

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:810: if (!m_pMaskStream)
On 2015/10/22 17:16:45, Tom Sepez wrote:
> nit: maybe 
>   return ToStream(m_pMaskStream) ? StartLoadMaskDIB() : 1;

Done.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:849: int ret =
m_pMask->StartLoadDIBSource(m_pDocument, ToStream(m_pMaskStream),
On 2015/10/22 17:16:45, Tom Sepez wrote:
> I worry about this.  At line 809, we may have assigned a non-stream to
> m_pMaskStream.  Now we will test type and pass NULL for that case.  Later on,
> some clever code may check that the pointer is some other type, and sloppily
> cast it to the type.  Please confirm.  Can we make the m_MaskStream be a
stream,
> not an object?  Can we force ToStream() on it at the time of assignment?


Done. There are 2 assignments and this one usage. The two assignments are 791
and 809 and 791 is GetStream

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

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:411: static void
_DrawFreeGouraudShading(CFX_DIBitmap* pBitmap,
On 2015/10/22 17:16:45, Tom Sepez wrote:
> nit: as long as you're mucking with this, can we lose the leading _ ? _CAPITAL
> is reserved for the implementation and should not be used. 

Done.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:419: if
(!pShadingStream->IsStream())
On 2015/10/22 17:16:45, Tom Sepez wrote:
> Can a stream be not IsStream() ?  Either this should take an object argument
and
> we call ToStream here, or the callers should call ToStream() and we do a null
> check here.

Done. Probably bad casts in the past.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:455: if
(!pShadingStream->IsStream())
On 2015/10/22 17:16:45, Tom Sepez wrote:
> ditto

Done.

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_pattern.cpp:761: if
(!pShadingStream->IsStream())
On 2015/10/22 17:16:45, Tom Sepez wrote:
> ditto ditto

Done.

[Tom Sepez, 2015-10-22 19:11:55]

lgtm

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

https://codereview.chromium.org/1402413004/diff/1/core/src/fpdfapi/fpdf_rende...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:849: int ret =
m_pMask->StartLoadDIBSource(m_pDocument, ToStream(m_pMaskStream),
On 2015/10/22 17:42:56, dsinclair wrote:
> On 2015/10/22 17:16:45, Tom Sepez wrote:
> > I worry about this.  At line 809, we may have assigned a non-stream to
> > m_pMaskStream.  Now we will test type and pass NULL for that case.  Later
on,
> > some clever code may check that the pointer is some other type, and sloppily
> > cast it to the type.  Please confirm.  Can we make the m_MaskStream be a
> stream,
> > not an object?  Can we force ToStream() on it at the time of assignment?
> 
> 
> Done. There are 2 assignments and this one usage. The two assignments are 791
> and 809 and 791 is GetStream
Ok.  Looks like were type safe.

https://codereview.chromium.org/1402413004/diff/20001/core/src/fpdfapi/fpdf_r...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/1402413004/diff/20001/core/src/fpdfapi/fpdf_r...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:791: m_pMaskStream =
ToStream(m_pDict->GetStream(FX_BSTRC("SMask")));
Hasn't GetStream already enforced that this is a STREAM>

[dsinclair, 2015-10-22 19:33:38]

https://codereview.chromium.org/1402413004/diff/20001/core/src/fpdfapi/fpdf_r...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/1402413004/diff/20001/core/src/fpdfapi/fpdf_r...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:791: m_pMaskStream =
ToStream(m_pDict->GetStream(FX_BSTRC("SMask")));
On 2015/10/22 19:11:55, Tom Sepez wrote:
> Hasn't GetStream already enforced that this is a STREAM>

Done.

[dsinclair, 2015-10-22 19:34:19]

Committed patchset #3 (id:40001) manually as
338805f1366dcdf9a5b48cf591541cf98d7490f1 (presubmit successful).

[Nico, 2016-01-11 23:55:51]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2016-01-11 23:55:51]

https://codereview.chromium.org/1402413004/diff/40001/core/src/fpdfapi/fpdf_e...
File core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/1402413004/diff/40001/core/src/fpdfapi/fpdf_e...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:945: if (pObj->IsStream())
Did you intentionally remove this brace? Before, this block was

  if (a) {
    if (b) {
      c;
    }
    d;
  }

You removed all the braces, turning it into

  if (a)
    if (b)
      c;
    d;

d is still indented as if it belonged to the outer if, but since you removed the
braces it no longer does. This looks like a mistake.

[Tom Sepez, 2016-01-12 00:00:45]

https://codereview.chromium.org/1402413004/diff/40001/core/src/fpdfapi/fpdf_e...
File core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/1402413004/diff/40001/core/src/fpdfapi/fpdf_e...
core/src/fpdfapi/fpdf_edit/fpdf_edit_create.cpp:945: if (pObj->IsStream())
On 2016/01/11 23:55:51, Nico wrote:
> Did you intentionally remove this brace? Before, this block was
> 
>   if (a) {
>     if (b) {
>       c;
>     }
>     d;
>   }
> 
> You removed all the braces, turning it into
> 
>   if (a)
>     if (b)
>       c;
>     d;
> 
> d is still indented as if it belonged to the outer if, but since you removed
the
> braces it no longer does. This looks like a mistake.

Yes, mistake, yes corrected in https://codereview.chromium.org/1544963002

[Lei Zhang, 2016-01-12 00:12:28]

On 2016/01/12 00:00:45, Tom Sepez wrote:
> Yes, mistake, yes corrected in https://codereview.chromium.org/1544963002

I'm going to file a Chromium bug to get this fix merged to M48. M47 is
unaffected, and M49 will just pick up a DEPS roll with the fix.