Change the forced clear order of CPDF_DocPageData::Clear, and remove m_pDocument in CPDF_Color

Change the forced clear order of CPDF_DocPageData::Clear, and remove m_pDocument in CPDF_Color

If not forced clear, the counter of CPDF_Pattern will take care the count of referenced instance.
When forced clear, clear color first to make sure pattern still exists when referencing it.

BUG=411154
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/9175beec2686231fe284e8c8095d04668817509b

[Bo Xu, 2014-09-08 22:19:39]

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

[Bo Xu, 2014-09-08 22:19:39]

Hi Tom, please review this one, thanks.

[Tom Sepez, 2014-09-08 22:43:09]

On 2014/09/08 22:19:39, Bo Xu wrote:
> Hi Tom, please review this one, thanks.

It looks like the get calls eventually return something like

    &CPDF_PageModule::m_StockGrayCS;

which will then get updated when you assign m_pDocument.  Is is possible that
the PageModule might be used by more than one document so that its stock CS
could get unexpected documents assigned to it?

If not, then it seems like the m_pDocument should be set up by some constructor
somewhere.

[Bo Xu, 2014-09-09 00:16:54]

On 2014/09/08 22:43:09, Tom Sepez wrote:
> On 2014/09/08 22:19:39, Bo Xu wrote:
> > Hi Tom, please review this one, thanks.
> 
> It looks like the get calls eventually return something like
> 
>     &CPDF_PageModule::m_StockGrayCS;
> 
> which will then get updated when you assign m_pDocument.  Is is possible that
> the PageModule might be used by more than one document so that its stock CS
> could get unexpected documents assigned to it?
> 
> If not, then it seems like the m_pDocument should be set up by some
constructor
> somewhere.

Jun and I had some discussion and we agree with you, the m_StockGrayCS are like
constants and should be shared in multiple documents, if any.

Rechecking the issue, the root cause as stated in
https://codereview.chromium.org/439693002, is that pattern has been forced clear
before color. So just changing the order of how they forced clear would solve
the issue as in Patch 2. I tested this on previous related issues and this seems
to work.

With this, we don't need to have the m_pDocument in CPDF_Color anymore.

[Tom Sepez, 2014-09-09 23:59:04]

LGTM with nit.

https://codereview.chromium.org/554913003/diff/20001/core/src/fpdfapi/fpdf_pa...
File core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp (right):

https://codereview.chromium.org/554913003/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp:1295: if (pPattern &&
pvalue->m_pPattern->m_pDocument) {
nit: (pPattern && pPattern->m_pDocument)

https://codereview.chromium.org/554913003/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp:1296: CPDF_DocPageData
*pPageData = pvalue->m_pPattern->m_pDocument->GetPageData();
nit: ditto

[Bo Xu, 2014-09-10 00:04:43]

https://codereview.chromium.org/554913003/diff/20001/core/src/fpdfapi/fpdf_pa...
File core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp (right):

https://codereview.chromium.org/554913003/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp:1295: if (pPattern &&
pvalue->m_pPattern->m_pDocument) {
On 2014/09/09 23:59:04, Tom Sepez wrote:
> nit: (pPattern && pPattern->m_pDocument)

Done.

https://codereview.chromium.org/554913003/diff/20001/core/src/fpdfapi/fpdf_pa...
core/src/fpdfapi/fpdf_page/fpdf_page_colors.cpp:1296: CPDF_DocPageData
*pPageData = pvalue->m_pPattern->m_pDocument->GetPageData();
On 2014/09/09 23:59:04, Tom Sepez wrote:
> nit: ditto

Done.

[Bo Xu, 2014-09-10 01:39:27]

Committed patchset #3 (id:40001) manually as 9175bee (presubmit successful).