Guard against double deletion of page views.

Guard against double deletion of page views.

This CL adds a |IsBeingDestroyed| flag into the CPDFSDK_PageView. We then
bail out of the pageview removal code early if the flag is set.

BUG=chromium:652103
Committed: https://pdfium.googlesource.com/pdfium/+/bcf46238b4533a9da91f4fa5d7248bbc85511dbd

[dsinclair, 2016-10-03 18:13:57]

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

[commit-bot: I haz the power, 2016-10-03 18:14:06]

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

[dsinclair, 2016-10-03 18:17:19]

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

[dsinclair, 2016-10-03 18:17:20]

PTAL.

There are a lot of interactions between the pageview, page and annotations. Each
time one of these gets fixed it seems to cause another issue. So, add a new flag
that explicitly checks if the page view is being deleted and bail early seems
like the safest solution.

[commit-bot: I haz the power, 2016-10-03 18:32:33]

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

[commit-bot: I haz the power, 2016-10-03 18:32:34]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-10-03 18:37:57]

https://codereview.chromium.org/2384243002/diff/1/fpdfsdk/cpdfsdk_document.cpp
File fpdfsdk/cpdfsdk_document.cpp (right):

https://codereview.chromium.org/2384243002/diff/1/fpdfsdk/cpdfsdk_document.cp...
fpdfsdk/cpdfsdk_document.cpp:143: pPageView->SetBeingDestroyed();
Can we this to the dtor? Then the dtor checks for this and bails if set.

[dsinclair, 2016-10-03 18:47:08]

https://codereview.chromium.org/2384243002/diff/1/fpdfsdk/cpdfsdk_document.cpp
File fpdfsdk/cpdfsdk_document.cpp (right):

https://codereview.chromium.org/2384243002/diff/1/fpdfsdk/cpdfsdk_document.cp...
fpdfsdk/cpdfsdk_document.cpp:143: pPageView->SetBeingDestroyed();
On 2016/10/03 18:37:57, Tom Sepez wrote:
> Can we this to the dtor? Then the dtor checks for this and bails if set.

No, because we have to guard the map erasing and the killing of annotations. In
this specific bug, we get the iterator, get the KillFocusAnnotation, then we end
up back in here, get the iterator again and call erase(it). We then pop out the
stack and call erase(it) a second time, and the map doesn't like it when you
erase the same iterator value twice.

Maybe Destroyed was a bad name, IsBeingRemoved?

[Tom Sepez, 2016-10-03 19:03:20]

OK, LGTM.

[dsinclair, 2016-10-03 20:02:04]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-10-03 20:02:12]

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, 2016-10-03 20:02:29]

Description was changed from

==========
Guard against double deletion of page views.

This CL adds a |IsBeingDestroyed| flag into the CPDFSDK_PageView. We then
bail out of the pageview removal code early if the flag is set.

BUG=chromium:652103
==========

to

==========
Guard against double deletion of page views.

This CL adds a |IsBeingDestroyed| flag into the CPDFSDK_PageView. We then
bail out of the pageview removal code early if the flag is set.

BUG=chromium:652103

Committed:
https://pdfium.googlesource.com/pdfium/+/bcf46238b4533a9da91f4fa5d7248bbc8551...
==========

[commit-bot: I haz the power, 2016-10-03 20:02:30]

Committed patchset #1 (id:1) as
https://pdfium.googlesource.com/pdfium/+/bcf46238b4533a9da91f4fa5d7248bbc8551...