Use observed pointers in CPDFSDK_AnnotIterator.

Use observed pointers in CPDFSDK_AnnotIterator.

Rename to CPDFSDK_AnnotIteration, as it is now an ordered set of
annots, and not the iterator itself.

Review-Url: https://codereview.chromium.org/2626073005
Committed: https://pdfium.googlesource.com/pdfium/+/d805eec52f6ac574918748c4270873e7e5cde596

[Tom Sepez, 2017-01-11 20:53:31]

Description was changed from

==========
Use observed pointers in CPDFSDK_AnnotIterator.
==========

to

==========
Use observed pointers in CPDFSDK_AnnotIterator.

Rename to CPDFSDK_AnnotIteration, as it is now an ordered set of
annots, and not the iterator itself.
==========

[Tom Sepez, 2017-01-11 20:59:43]

https://codereview.chromium.org/2626073005/diff/20001/fpdfsdk/cpdfsdk_annotit...
File fpdfsdk/cpdfsdk_annotiteration.h (right):

https://codereview.chromium.org/2626073005/diff/20001/fpdfsdk/cpdfsdk_annotit...
fpdfsdk/cpdfsdk_annotiteration.h:1: // Copyright 2016 PDFium Authors. All rights
reserved.
NOTE: these two may be easier to review in PS#1.

[Tom Sepez, 2017-01-11 21:00:18]

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

[commit-bot: I haz the power, 2017-01-11 21:00:20]

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-11 21:00:50]

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

[Tom Sepez, 2017-01-11 21:00:50]

Dan, for review.

[dsinclair, 2017-01-11 21:22:53]

https://codereview.chromium.org/2626073005/diff/60001/fpdfsdk/cpdfsdk_annotit...
File fpdfsdk/cpdfsdk_annotiteration.cpp (right):

https://codereview.chromium.org/2626073005/diff/60001/fpdfsdk/cpdfsdk_annotit...
fpdfsdk/cpdfsdk_annotiteration.cpp:1: // Copyright 2016 PDFium Authors. All
rights reserved.
2017

https://codereview.chromium.org/2626073005/diff/60001/fpdfsdk/cpdfsdk_annotit...
fpdfsdk/cpdfsdk_annotiteration.cpp:28: std::swap(*it, *copiedList.begin());
Do we really want to swap here? Don't we just want to remove *it from the list
and prepend it to the list?

Otherwise, the original first element will no longer be in LayoutOrder but
inserted somewhere random in the list.

https://codereview.chromium.org/2626073005/diff/60001/fpdfsdk/cpdfsdk_annotit...
File fpdfsdk/cpdfsdk_annotiteration.h (right):

https://codereview.chromium.org/2626073005/diff/60001/fpdfsdk/cpdfsdk_annotit...
fpdfsdk/cpdfsdk_annotiteration.h:1: // Copyright 2016 PDFium Authors. All rights
reserved.
2017

[Tom Sepez, 2017-01-11 21:34:26]

Argh, you're right.

https://codereview.chromium.org/2626073005/diff/60001/fpdfsdk/cpdfsdk_annotit...
File fpdfsdk/cpdfsdk_annotiteration.cpp (right):

https://codereview.chromium.org/2626073005/diff/60001/fpdfsdk/cpdfsdk_annotit...
fpdfsdk/cpdfsdk_annotiteration.cpp:28: std::swap(*it, *copiedList.begin());
On 2017/01/11 21:22:52, dsinclair wrote:
> Do we really want to swap here? Don't we just want to remove *it from the list
> and prepend it to the list?
> 
> Otherwise, the original first element will no longer be in LayoutOrder but
> inserted somewhere random in the list.

Done.

[dsinclair, 2017-01-11 21:44:24]

lgtm w/ question.

https://codereview.chromium.org/2626073005/diff/80001/fpdfsdk/cpdfsdk_annotit...
File fpdfsdk/cpdfsdk_annotiteration.cpp (right):

https://codereview.chromium.org/2626073005/diff/80001/fpdfsdk/cpdfsdk_annotit...
fpdfsdk/cpdfsdk_annotiteration.cpp:28: CPDFSDK_Annot* pSaved = *it;
Is this needed? Won't pSaved == pTopMostAnnot?

[Tom Sepez, 2017-01-11 21:48:52]

https://codereview.chromium.org/2626073005/diff/80001/fpdfsdk/cpdfsdk_annotit...
File fpdfsdk/cpdfsdk_annotiteration.cpp (right):

https://codereview.chromium.org/2626073005/diff/80001/fpdfsdk/cpdfsdk_annotit...
fpdfsdk/cpdfsdk_annotiteration.cpp:28: CPDFSDK_Annot* pSaved = *it;
On 2017/01/11 21:44:24, dsinclair wrote:
> Is this needed? Won't pSaved == pTopMostAnnot?

Done.

[Tom Sepez, 2017-01-11 21:48:57]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2017-01-11 21:48:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/2626073005/#ps100001
(title: "remove local")

[commit-bot: I haz the power, 2017-01-11 21:49:03]

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-11 22:03:54]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484171337347720,
"parent_rev": "5e3121beff936df1b0af3749447eeda1666d5d76", "commit_rev":
"d805eec52f6ac574918748c4270873e7e5cde596"}

[commit-bot: I haz the power, 2017-01-11 22:03:57]

Description was changed from

==========
Use observed pointers in CPDFSDK_AnnotIterator.

Rename to CPDFSDK_AnnotIteration, as it is now an ordered set of
annots, and not the iterator itself.
==========

to

==========
Use observed pointers in CPDFSDK_AnnotIterator.

Rename to CPDFSDK_AnnotIteration, as it is now an ordered set of
annots, and not the iterator itself.

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

[commit-bot: I haz the power, 2017-01-11 22:03:57]

Committed patchset #6 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/d805eec52f6ac574918748c4270873e7e5cd...