Clean up CPDF_Page.

Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
- Delete CPDF_ParseOptions.
- Fix up CPDF_Pattern.

Committed: https://pdfium.googlesource.com/pdfium/+/5cc24654fb345189140acb4711ff981e1c720951

[Lei Zhang, 2016-04-26 01:15:58]

Description was changed from

==========
Clean up CPDF_Page.
==========

to

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
==========

[Lei Zhang, 2016-04-26 01:15:58]

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

[Lei Zhang, 2016-04-26 01:22:22]

https://codereview.chromium.org/1918113002/diff/20001/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/1918113002/diff/20001/fpdfsdk/fpdfview.cpp#ne...
fpdfsdk/fpdfview.cpp:961: CPDF_Page page(pDoc, pDict, true);
This is the only instance that does not call ParseContent(), BTW.

[dsinclair, 2016-04-26 02:42:26]

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_page.cpp (right):

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_page.cpp:20: const CFX_ByteStringC& name) {
Can this be a ByteString instead of ByteStringC?

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_page.cpp:25: if (pObj)
if (CPDF_Object* pObj = pPageDict->GetDirectObjectBy(name))
  return pObj.

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/pageint.h (right):

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/pageint.h:314: CPDF_ParseOptions m_Options;
Can this be removed?

https://codereview.chromium.org/1918113002/diff/20001/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/1918113002/diff/20001/fpdfsdk/fpdfview.cpp#ne...
fpdfsdk/fpdfview.cpp:960: return false;
Should this be FALSE for consistency?

nit: blank line after braceless if.

[Lei Zhang, 2016-04-26 04:22:49]

Description was changed from

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
==========

to

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
- Delete cpdf_parseoptions.cpp.
==========

[Lei Zhang, 2016-04-26 04:22:58]

Description was changed from

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
- Delete cpdf_parseoptions.cpp.
==========

to

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
- Delete CPDF_ParseOptions.
==========

[Lei Zhang, 2016-04-26 04:24:45]

And the CL got a lot bigger.

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_page.cpp (right):

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_page.cpp:20: const CFX_ByteStringC& name) {
On 2016/04/26 02:42:26, dsinclair wrote:
> Can this be a ByteString instead of ByteStringC?

Done.

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_page.cpp:25: if (pObj)
On 2016/04/26 02:42:26, dsinclair wrote:
> if (CPDF_Object* pObj = pPageDict->GetDirectObjectBy(name))
>   return pObj.

Done.

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/pageint.h (right):

https://codereview.chromium.org/1918113002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/pageint.h:314: CPDF_ParseOptions m_Options;
On 2016/04/26 02:42:26, dsinclair wrote:
> Can this be removed?

Done.

https://codereview.chromium.org/1918113002/diff/20001/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/1918113002/diff/20001/fpdfsdk/fpdfview.cpp#ne...
fpdfsdk/fpdfview.cpp:960: return false;
On 2016/04/26 02:42:26, dsinclair wrote:
> Should this be FALSE for consistency?
> 
> nit: blank line after braceless if.

Whoops, reverted.

[dsinclair, 2016-04-26 14:08:24]

Larger and more awesome. lgtm w/ nits.

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_colorstate.cpp (right):

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_colorstate.cpp:53:
((CPDF_TilingPattern*)pPattern)->colored() && !ret) {
nit: static_cast, and below.

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser.cpp (right):

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:641: CPDF_Array* pArray =
GetObject(1) ? GetObject(1)->GetArray() : NULL;
nit: nullptr

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp (right):

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp:212: }
nit: no {}'s

https://codereview.chromium.org/1918113002/diff/40001/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/1918113002/diff/40001/fpdfsdk/fpdfview.cpp#ne...
fpdfsdk/fpdfview.cpp:962: CPDF_Page page(pDoc, pDict, true);
This isn't a bug that it doesn't call parseContent, yea?

[Lei Zhang, 2016-04-26 17:22:47]

Description was changed from

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
- Delete CPDF_ParseOptions.
==========

to

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
- Delete CPDF_ParseOptions.
- Fix up CPDF_Pattern.
==========

[Lei Zhang, 2016-04-26 17:25:38]

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_colorstate.cpp (right):

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_colorstate.cpp:53:
((CPDF_TilingPattern*)pPattern)->colored() && !ret) {
On 2016/04/26 14:08:23, dsinclair wrote:
> nit: static_cast, and below.

Fixed.

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser.cpp (right):

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:641: CPDF_Array* pArray =
GetObject(1) ? GetObject(1)->GetArray() : NULL;
On 2016/04/26 14:08:23, dsinclair wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp (right):

https://codereview.chromium.org/1918113002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp:212: }
On 2016/04/26 14:08:23, dsinclair wrote:
> nit: no {}'s

Done.

https://codereview.chromium.org/1918113002/diff/40001/fpdfsdk/fpdfview.cpp
File fpdfsdk/fpdfview.cpp (right):

https://codereview.chromium.org/1918113002/diff/40001/fpdfsdk/fpdfview.cpp#ne...
fpdfsdk/fpdfview.cpp:962: CPDF_Page page(pDoc, pDict, true);
On 2016/04/26 14:08:24, dsinclair wrote:
> This isn't a bug that it doesn't call parseContent, yea?

It's not a bug. The ctor sets the page size.

[dsinclair, 2016-04-26 18:40:54]

lgtm++

[Lei Zhang, 2016-04-26 18:45:30]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-04-26 18:45:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1918113002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1918113002/60001

[commit-bot: I haz the power, 2016-04-26 18:46:05]

Description was changed from

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
- Delete CPDF_ParseOptions.
- Fix up CPDF_Pattern.
==========

to

==========
Clean up CPDF_Page.

- Merge CPDF_Page::Load() into ctor.
- Remove always nullptr param for CPDF_Page::ParseContent().
- Remove unneeded indirection in IPDF_RenderModule.
- Delete CPDF_ParseOptions.
- Fix up CPDF_Pattern.

Committed:
https://pdfium.googlesource.com/pdfium/+/5cc24654fb345189140acb4711ff981e1c72...
==========

[commit-bot: I haz the power, 2016-04-26 18:46:07]

Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/5cc24654fb345189140acb4711ff981e1c72...