Remove some CFX_ArrayTemplate in fpdfapi and fpdfdoc

Remove some CFX_ArrayTemplate in fpdfapi and fpdfdoc

Also use unique_ptr in one spot while we're at it.

Review-Url: https://codereview.chromium.org/2625483002
Committed: https://pdfium.googlesource.com/pdfium/+/d18b8674378b104a9b5bb1f015e3f92fc174673e

[Tom Sepez, 2017-01-09 18:34:53]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 18:34:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom Sepez, 2017-01-09 18:37:06]

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

[Tom Sepez, 2017-01-09 18:37:07]

Dan, for review.

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formcontr...
File core/fpdfdoc/cpdf_formcontrol.h (right):

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formcontr...
core/fpdfdoc/cpdf_formcontrol.h:49: CPDF_FormControl(CPDF_FormField* pField,
CPDF_Dictionary* pWidgetDict);
Note: For MakeUnique<> to be allowed to construct these.

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_interform...
File core/fpdfdoc/cpdf_interform.cpp (right):

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_interform...
core/fpdfdoc/cpdf_interform.cpp:672: m_ControlMap.clear();
Note: Just to keep deletion order for now.

[commit-bot: I haz the power, 2017-01-09 18:50:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-09 18:50:29]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2017-01-10 14:11:11]

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
File core/fpdfdoc/cpdf_formfield.cpp (right):

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
core/fpdfdoc/cpdf_formfield.cpp:249: return it != m_ControlList.end() ? it -
m_ControlList.begin() : -1;
What are your thoughts on std::tie? It would be nice (a some later point) to
stop returning invalid array indexes. We could do something like:

std::pair<bool, size_t> GetControlIndex(pControl) {
  if (!pControl) {
    return std::pair<bool, size>{false, 0};

  auto it = ...
  return std::pair<bool, size>{true, it - m_ControlList.begin() };
}

Then the callers can do something like:

bool found = false;
size_t idx = 0;
std::tie(found, idx) = GetControlIndex(control);

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
core/fpdfdoc/cpdf_formfield.cpp:414: for (const CPDF_FormControl* pControl :
m_ControlList) {
auto?

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield.h
File core/fpdfdoc/cpdf_formfield.h (right):

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
core/fpdfdoc/cpdf_formfield.h:156: CPDF_InterForm* m_pForm;
const?

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
core/fpdfdoc/cpdf_formfield.h:158: std::vector<CPDF_FormControl*> m_ControlList;
It looks like the InterForm owns the control? Should we make this a WeakPtr or
are we depending on all fields being destroyed before the InterForm is
destroyed?

Can we add a comment that the FormControl is owned by the InterForm parent?

[Tom Sepez, 2017-01-10 18:06:15]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 18:06:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom Sepez, 2017-01-10 18:06:27]

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
File core/fpdfdoc/cpdf_formfield.cpp (right):

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
core/fpdfdoc/cpdf_formfield.cpp:249: return it != m_ControlList.end() ? it -
m_ControlList.begin() : -1;
On 2017/01/10 14:11:10, dsinclair wrote:
> What are your thoughts on std::tie? It would be nice (a some later point) to
> stop returning invalid array indexes. We could do something like:
> 
> std::pair<bool, size_t> GetControlIndex(pControl) {
>   if (!pControl) {
>     return std::pair<bool, size>{false, 0};
> 
>   auto it = ...
>   return std::pair<bool, size>{true, it - m_ControlList.begin() };
> }
> 
> Then the callers can do something like:
> 
> bool found = false;
> size_t idx = 0;
> std::tie(found, idx) = GetControlIndex(control);

I really like this approach where there ain't no out parameters. Someday.  Not
sure if I prefer tie() or just 
auto result = GCI(control);
if (result.first) ...

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
core/fpdfdoc/cpdf_formfield.cpp:414: for (const CPDF_FormControl* pControl :
m_ControlList) {
On 2017/01/10 14:11:11, dsinclair wrote:
> auto?

Done.

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield.h
File core/fpdfdoc/cpdf_formfield.h (right):

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
core/fpdfdoc/cpdf_formfield.h:156: CPDF_InterForm* m_pForm;
On 2017/01/10 14:11:11, dsinclair wrote:
> const?

Done.

https://codereview.chromium.org/2625483002/diff/1/core/fpdfdoc/cpdf_formfield...
core/fpdfdoc/cpdf_formfield.h:158: std::vector<CPDF_FormControl*> m_ControlList;
On 2017/01/10 14:11:11, dsinclair wrote:
> It looks like the InterForm owns the control? Should we make this a WeakPtr or
> are we depending on all fields being destroyed before the InterForm is
> destroyed?
> 
Not sure.  Just leaving this alone for the moment.

> Can we add a comment that the FormControl is owned by the InterForm parent?

Done.

[dsinclair, 2017-01-10 18:11:11]

lgtm

[Tom Sepez, 2017-01-10 18:12:56]

The CQ bit was unchecked by tsepez@chromium.org

[Tom Sepez, 2017-01-10 18:12:59]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2017-01-10 18:13:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 18:19:05]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484071979189680,
"parent_rev": "3128d1c45d8bc313abb8aae151f86bbe62c52e56", "commit_rev":
"d18b8674378b104a9b5bb1f015e3f92fc174673e"}

[commit-bot: I haz the power, 2017-01-10 18:19:07]

Description was changed from

==========
Remove some CFX_ArrayTemplate in fpdfapi and fpdfdoc

Also use unique_ptr in one spot while we're at it.
==========

to

==========
Remove some CFX_ArrayTemplate in fpdfapi and fpdfdoc

Also use unique_ptr in one spot while we're at it.

Review-Url: https://codereview.chromium.org/2625483002
Committed:
https://pdfium.googlesource.com/pdfium/+/d18b8674378b104a9b5bb1f015e3f92fc174...
==========

[commit-bot: I haz the power, 2017-01-10 18:19:08]

Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/d18b8674378b104a9b5bb1f015e3f92fc174...