Cleanup CPDF_ApSettings and CJS_Parameters.

Cleanup CPDF_ApSettings and CJS_Parameters.

R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/56e3da27d8bed8a6c03274b7b946e3f6d6ac11f4

[Lei Zhang, 2015-11-10 07:33:12]

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

[Tom Sepez, 2015-11-10 17:28:04]

https://codereview.chromium.org/1432863004/diff/1/core/include/fpdfdoc/fpdf_d...
File core/include/fpdfdoc/fpdf_doc.h (left):

https://codereview.chromium.org/1432863004/diff/1/core/include/fpdfdoc/fpdf_d...
core/include/fpdfdoc/fpdf_doc.h:1113: operator CPDF_Dictionary*() const { return
m_pDict; }
You removed this, right?  Are we sure there aren't c-style casts like
(CPDF_Dictionary*)pApSettings that still compile but whose meaning has changed
as a result of this deletion?

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
File core/src/fpdfdoc/doc_formcontrol.cpp (left):

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
core/src/fpdfdoc/doc_formcontrol.cpp:191: CPDF_ApSettings
CPDF_FormControl::GetMK(FX_BOOL bCreate) {
Confirmed bCreate is always passed as FALSE.

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
File core/src/fpdfdoc/doc_formcontrol.cpp (right):

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
core/src/fpdfdoc/doc_formcontrol.cpp:198: CPDF_ApSettings mk = GetMK();
nit: how about just return GetMK().GetRotation(); and so forth for these?

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
core/src/fpdfdoc/doc_formcontrol.cpp:328: FX_BOOL
CPDF_ApSettings::HasMKEntry(const CFX_ByteStringC& csEntry) const {
can this be just bool?

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
core/src/fpdfdoc/doc_formcontrol.cpp:329: return m_pDict ?
m_pDict->KeyExist(csEntry) : FALSE;
nit: just return m_pDict && m_pDict->KeyExit(csEntry);

https://codereview.chromium.org/1432863004/diff/1/fpdfsdk/src/javascript/JS_V...
File fpdfsdk/src/javascript/JS_Value.h (right):

https://codereview.chromium.org/1432863004/diff/1/fpdfsdk/src/javascript/JS_V...
fpdfsdk/src/javascript/JS_Value.h:91: using CJS_Parameters =
std::vector<CJS_Value>;
nit: up next, remove CS_Parameters and just use std::vector<CJS_Value>
everywhere.

[Tom Sepez, 2015-11-10 17:31:20]

https://codereview.chromium.org/1432863004/diff/1/core/include/fpdfdoc/fpdf_d...
File core/include/fpdfdoc/fpdf_doc.h (left):

https://codereview.chromium.org/1432863004/diff/1/core/include/fpdfdoc/fpdf_d...
core/include/fpdfdoc/fpdf_doc.h:1113: operator CPDF_Dictionary*() const { return
m_pDict; }
On 2015/11/10 17:28:04, Tom Sepez wrote:
> You removed this, right?  Are we sure there aren't c-style casts like
> (CPDF_Dictionary*)pApSettings that still compile but whose meaning has changed
> as a result of this deletion?
Guess not:

$ git grep '(\s*CPDF_Dictionary\s*\*\s*)'
core/include/fpdfapi/fpdf_resource.h:  void
LoadFontDescriptor(CPDF_Dictionary*);

[Lei Zhang, 2015-11-11 02:27:56]

https://codereview.chromium.org/1432863004/diff/1/core/include/fpdfdoc/fpdf_d...
File core/include/fpdfdoc/fpdf_doc.h (left):

https://codereview.chromium.org/1432863004/diff/1/core/include/fpdfdoc/fpdf_d...
core/include/fpdfdoc/fpdf_doc.h:1113: operator CPDF_Dictionary*() const { return
m_pDict; }
On 2015/11/10 17:28:04, Tom Sepez wrote:
> You removed this, right?  Are we sure there aren't c-style casts like
> (CPDF_Dictionary*)pApSettings that still compile but whose meaning has changed
> as a result of this deletion?

Acknowledged.

https://codereview.chromium.org/1432863004/diff/1/core/include/fpdfdoc/fpdf_d...
core/include/fpdfdoc/fpdf_doc.h:1113: operator CPDF_Dictionary*() const { return
m_pDict; }
On 2015/11/10 17:31:20, Tom Sepez wrote:
> On 2015/11/10 17:28:04, Tom Sepez wrote:
> > You removed this, right?  Are we sure there aren't c-style casts like
> > (CPDF_Dictionary*)pApSettings that still compile but whose meaning has
changed
> > as a result of this deletion?
> Guess not:
> 
> $ git grep '(\s*CPDF_Dictionary\s*\*\s*)'
> core/include/fpdfapi/fpdf_resource.h:  void
> LoadFontDescriptor(CPDF_Dictionary*);

cs.chromium.org's xref also shows all uses of this operator are in
CPDF_FormControl::GetMK().

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
File core/src/fpdfdoc/doc_formcontrol.cpp (left):

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
core/src/fpdfdoc/doc_formcontrol.cpp:191: CPDF_ApSettings
CPDF_FormControl::GetMK(FX_BOOL bCreate) {
On 2015/11/10 17:28:04, Tom Sepez wrote:
> Confirmed bCreate is always passed as FALSE.

Acknowledged.

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
File core/src/fpdfdoc/doc_formcontrol.cpp (right):

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
core/src/fpdfdoc/doc_formcontrol.cpp:198: CPDF_ApSettings mk = GetMK();
On 2015/11/10 17:28:04, Tom Sepez wrote:
> nit: how about just return GetMK().GetRotation(); and so forth for these?

Done.

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
core/src/fpdfdoc/doc_formcontrol.cpp:328: FX_BOOL
CPDF_ApSettings::HasMKEntry(const CFX_ByteStringC& csEntry) const {
On 2015/11/10 17:28:04, Tom Sepez wrote:
> can this be just bool?

Done.

https://codereview.chromium.org/1432863004/diff/1/core/src/fpdfdoc/doc_formco...
core/src/fpdfdoc/doc_formcontrol.cpp:329: return m_pDict ?
m_pDict->KeyExist(csEntry) : FALSE;
On 2015/11/10 17:28:04, Tom Sepez wrote:
> nit: just return m_pDict && m_pDict->KeyExit(csEntry);

Done.

https://codereview.chromium.org/1432863004/diff/1/fpdfsdk/src/javascript/JS_V...
File fpdfsdk/src/javascript/JS_Value.h (right):

https://codereview.chromium.org/1432863004/diff/1/fpdfsdk/src/javascript/JS_V...
fpdfsdk/src/javascript/JS_Value.h:91: using CJS_Parameters =
std::vector<CJS_Value>;
On 2015/11/10 17:28:04, Tom Sepez wrote:
> nit: up next, remove CS_Parameters and just use std::vector<CJS_Value>
> everywhere.

Acknowledged.

[Tom Sepez, 2015-11-11 17:42:50]

lgtm

[Lei Zhang, 2015-11-11 18:10:49]

Committed patchset #3 (id:40001) manually as
56e3da27d8bed8a6c03274b7b946e3f6d6ac11f4 (presubmit successful).