Remove pageview from map immediately

Remove pageview from map immediately

There seems to be an ownership issue in the page annotation code where removing
the annotations can result in removing the parent page view. This is fine except
that removing the parent page view removes the annotations and you can end up
with a use-after-free.

This CL removes the page view from the documents page map immediately and then
proceeds with the cleanup. Then, if we try to remove that page again it won't
be found and we won't double free.

BUG=chromium:630654
Committed: https://pdfium.googlesource.com/pdfium/+/49dce65dc78bcd5a0c78a8bbdf2809cf20212220

[dsinclair, 2016-07-26 18:06:17]

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

[commit-bot: I haz the power, 2016-07-26 18:06:20]

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

[dsinclair, 2016-07-26 18:07:18]

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

[dsinclair, 2016-07-26 18:07:18]

PTAL.

I'm going to see if I can track down why removing the annotation frees the page,
but this seems like it's a safe thing to do anyway.

[commit-bot: I haz the power, 2016-07-26 18:24:31]

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

[commit-bot: I haz the power, 2016-07-26 18:24:31]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-07-26 18:46:45]

lgtm

BTW, no need for chromium: prefix.

[dsinclair, 2016-07-26 19:09:14]

On 2016/07/26 18:46:45, Lei Zhang wrote:
> lgtm
> 
> BTW, no need for chromium: prefix.


It should be needed, but I don't think codereview is setup quite correctly. In
theory, the default prefix for pdfium should be pdfium: and all bugs would
assume the pdfium issue tracker. chromium: would be required fro chromium bugs.
In reality, I think it's working the other way around.

I figure, prefixing is makes it un-ambiguous which tracker it is, and doesn't
really hurt anything.

[dsinclair, 2016-07-26 19:09:18]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-07-26 19:09:28]

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-07-26 19:09:46]

Description was changed from

==========
Remove pageview from map immediately

There seems to be an ownership issue in the page annotation code where removing
the annotations can result in removing the parent page view. This is fine except
that removing the parent page view removes the annotations and you can end up
with a use-after-free.

This CL removes the page view from the documents page map immediately and then
proceeds with the cleanup. Then, if we try to remove that page again it won't
be found and we won't double free.

BUG=chromium:630654
==========

to

==========
Remove pageview from map immediately

There seems to be an ownership issue in the page annotation code where removing
the annotations can result in removing the parent page view. This is fine except
that removing the parent page view removes the annotations and you can end up
with a use-after-free.

This CL removes the page view from the documents page map immediately and then
proceeds with the cleanup. Then, if we try to remove that page again it won't
be found and we won't double free.

BUG=chromium:630654

Committed:
https://pdfium.googlesource.com/pdfium/+/49dce65dc78bcd5a0c78a8bbdf2809cf2021...
==========

[commit-bot: I haz the power, 2016-07-26 19:09:47]

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

[dsinclair, 2016-07-26 19:19:35]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2188523002/ by dsinclair@chromium.org.

The reason for reverting is: Looks like this broke linux_asan embedder tests.

https://build.chromium.org/p/client.pdfium/builders/linux_asan/builds/1152/st....