Allocate m_pCompData when |m_nComponents| is updated.

Allocate m_pCompData when |m_nComponents| is updated.

When |m_nComponents| is changed from loading stream information,
previously allocated memory that depends on |m_nComponents| needes to be freed
and allocated again to enforce memory size consistency.


BUG=409695
R=tsepez@chromium.org

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

[Bo Xu, 2014-09-03 00:23:46]

bo_xu@foxitsoftware.com changed reviewers:
+ jun_fang@foxitsoftware.com, tsepez@chromium.org

[Bo Xu, 2014-09-03 00:23:46]

Hi Tom, please review this one, thanks.

[jun_fang, 2014-09-03 04:17:08]

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

https://codereview.chromium.org/528163002/diff/20001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:185: AllocCompData();
m_pCompData = GetCompData();
if (!m_pCompData) {
    return;
}

It's better to get component data in a distinct way than changing the variable
m_pCompData inside the function.I suggest to rename AllocCompData because this
function do more jobs than only allocating memory.

[Tom Sepez, 2014-09-03 16:59:11]

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:502: void
CPDF_DIBSource::AllocCompData()
nit: maybe AllocCompDataAndDecode().

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:502: void
CPDF_DIBSource::AllocCompData()
maybe this is a bool so we can return FALSE if the alloc it implies fails

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:504: m_pCompData =
FX_Alloc(DIB_COMP_DATA, m_nComponents);
check return value for NULL?

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:539: int min_num,
max_num;
nit:  these declarations can go inside the next loop, eg.
  int min_num = pArray-> ... ;

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:544:
m_pCompData[i].m_ColorKeyMax = FX_MIN((1 << m_bpc) - 1, max_num);
I'd expect MIN to take two arguments?

[Bo Xu, 2014-09-03 18:10:16]

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:502: void
CPDF_DIBSource::AllocCompData()
On 2014/09/03 16:59:11, Tom Sepez wrote:
> maybe this is a bool so we can return FALSE if the alloc it implies fails

Checked PDF reference, this struct contains decode and mask array.

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:502: void
CPDF_DIBSource::AllocCompData()
On 2014/09/03 16:59:11, Tom Sepez wrote:
> maybe this is a bool so we can return FALSE if the alloc it implies fails

I changed the function prototype to return m_pCompData

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:504: m_pCompData =
FX_Alloc(DIB_COMP_DATA, m_nComponents);
On 2014/09/03 16:59:11, Tom Sepez wrote:
> check return value for NULL?

Done.

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:539: int min_num,
max_num;
On 2014/09/03 16:59:11, Tom Sepez wrote:
> nit:  these declarations can go inside the next loop, eg.
>   int min_num = pArray-> ... ;

Done.

https://codereview.chromium.org/528163002/diff/40001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:544:
m_pCompData[i].m_ColorKeyMax = FX_MIN((1 << m_bpc) - 1, max_num);
On 2014/09/03 16:59:11, Tom Sepez wrote:
> I'd expect MIN to take two arguments?

I cleaned this a little bit.

[jun_fang, 2014-09-03 19:12:02]

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:499: m_pCompData =
GetDecodeAndMaskArray(m_bDefaultDecode, m_bColorKey);
check |m_pCompData == NULL|?

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:502: DIB_COMP_DATA*
CPDF_DIBSource::GetDecodeAndMaskArray(FX_BOOL& DefaultDecode, FX_BOOL& ColorKey)
rename DefaultDecode to bDefaultDecode and ColorKey to bColorKey?

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:583: m_pCompData =
GetDecodeAndMaskArray(m_bDefaultDecode, m_bColorKey);
Check |m_pCompData == NULL|?

[Tom Sepez, 2014-09-03 19:22:51]

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:582:
FX_Free(m_pCompData);
nit:  I'd do the free before the assign to m_nComponents, just because ... um
... lets say the free() crashes, and yer digging through a stack trace, and it
might be nice to know the old value ...

[Bo Xu, 2014-09-03 19:36:59]

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:499: m_pCompData =
GetDecodeAndMaskArray(m_bDefaultDecode, m_bColorKey);
On 2014/09/03 19:12:02, jun_fang wrote:
> check |m_pCompData == NULL|?

Done.

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:502: DIB_COMP_DATA*
CPDF_DIBSource::GetDecodeAndMaskArray(FX_BOOL& DefaultDecode, FX_BOOL& ColorKey)
On 2014/09/03 19:12:02, jun_fang wrote:
> rename DefaultDecode to bDefaultDecode and ColorKey to bColorKey?

Done.

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:582:
FX_Free(m_pCompData);
On 2014/09/03 19:22:51, Tom Sepez wrote:
> nit:  I'd do the free before the assign to m_nComponents, just because ... um
> ... lets say the free() crashes, and yer digging through a stack trace, and it
> might be nice to know the old value ... 

Done.

https://codereview.chromium.org/528163002/diff/60001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:583: m_pCompData =
GetDecodeAndMaskArray(m_bDefaultDecode, m_bColorKey);
On 2014/09/03 19:12:02, jun_fang wrote:
> Check |m_pCompData == NULL|?

Done.

[Tom Sepez, 2014-09-03 19:38:59]

lgtm

[jun_fang, 2014-09-03 19:42:17]

https://codereview.chromium.org/528163002/diff/80001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/528163002/diff/80001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:524: DefaultDecode =
FALSE;
bDefaultDecode?

https://codereview.chromium.org/528163002/diff/80001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:552: ColorKey = TRUE;
bColorKey?

[Bo Xu, 2014-09-03 20:12:24]

https://codereview.chromium.org/528163002/diff/80001/core/src/fpdfapi/fpdf_re...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/528163002/diff/80001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:524: DefaultDecode =
FALSE;
On 2014/09/03 19:42:17, jun_fang wrote:
> bDefaultDecode?

Done.

https://codereview.chromium.org/528163002/diff/80001/core/src/fpdfapi/fpdf_re...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:552: ColorKey = TRUE;
On 2014/09/03 19:42:17, jun_fang wrote:
> bColorKey?

Done.

[Bo Xu, 2014-09-03 20:30:42]

Committed patchset #6 (id:100001) manually as bb3c1a2 (presubmit successful).