Cleanup: Slight simplification in the PDFiumEngine dtor.

Cleanup: Slight simplification in the PDFiumEngine dtor.

The functions being called already checks for NULL input.

Committed: https://crrev.com/1626f004512fd5b52d80a18dda9c910825d8cc03
Cr-Commit-Position: refs/heads/master@{#330214}

[Lei Zhang, 2015-05-15 06:53:35]

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

[Tom Sepez, 2015-05-15 15:06:31]

https://codereview.chromium.org/1131093004/diff/20001/pdf/pdfium/pdfium_engin...
File pdf/pdfium/pdfium_engine.cc (right):

https://codereview.chromium.org/1131093004/diff/20001/pdf/pdfium/pdfium_engin...
pdf/pdfium/pdfium_engine.cc:666: FPDFDOC_ExitFormFillEnvironment(form_);
Curious about the ordering here, in that the same block in embedder_test.cpp
does the ExitFormFillEnvironment before the CloseDocument.  My intuition would
tell me that the FF env's lifetime should be nested within the document's
lifetime, but I've not actually looked.

[Lei Zhang, 2015-05-15 21:23:09]

https://codereview.chromium.org/1131093004/diff/20001/pdf/pdfium/pdfium_engin...
File pdf/pdfium/pdfium_engine.cc (right):

https://codereview.chromium.org/1131093004/diff/20001/pdf/pdfium/pdfium_engin...
pdf/pdfium/pdfium_engine.cc:666: FPDFDOC_ExitFormFillEnvironment(form_);
On 2015/05/15 15:06:31, Tom Sepez wrote:
> Curious about the ordering here, in that the same block in embedder_test.cpp
> does the ExitFormFillEnvironment before the CloseDocument.  My intuition would
> tell me that the FF env's lifetime should be nested within the document's
> lifetime, but I've not actually looked.

It used to be the other way around, but changed here. See the discussion with
Bo:
https://codereview.chromium.org/705823002/diff/60001/pdf/pdfium/pdfium_engine...

Calling FPDFDOC_InitFormFillEnvironment(doc_) does end up creating pointers that
point to |doc_|, but as long as we call FPDFDOC_ExitFormFillEnvironment()
immediately after FPDF_CloseDocument(), there's no opportunity to accidentally
use |doc_|. At least that's how it looks on the master branch.

[Tom Sepez, 2015-05-15 21:59:35]

On 2015/05/15 21:23:09, Lei Zhang wrote:
>
https://codereview.chromium.org/1131093004/diff/20001/pdf/pdfium/pdfium_engin...
> File pdf/pdfium/pdfium_engine.cc (right):
> 
>
https://codereview.chromium.org/1131093004/diff/20001/pdf/pdfium/pdfium_engin...
> pdf/pdfium/pdfium_engine.cc:666: FPDFDOC_ExitFormFillEnvironment(form_);
> On 2015/05/15 15:06:31, Tom Sepez wrote:
> > Curious about the ordering here, in that the same block in embedder_test.cpp
> > does the ExitFormFillEnvironment before the CloseDocument.  My intuition
would
> > tell me that the FF env's lifetime should be nested within the document's
> > lifetime, but I've not actually looked.
> 
> It used to be the other way around, but changed here. See the discussion with
> Bo:
>
https://codereview.chromium.org/705823002/diff/60001/pdf/pdfium/pdfium_engine...
> 
> Calling FPDFDOC_InitFormFillEnvironment(doc_) does end up creating pointers
that
> point to |doc_|, but as long as we call FPDFDOC_ExitFormFillEnvironment()
> immediately after FPDF_CloseDocument(), there's no opportunity to accidentally
> use |doc_|. At least that's how it looks on the master branch.
Alright, I guess. LGTM.

[Lei Zhang, 2015-05-15 22:09:33]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2015-05-15 22:09:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1131093004/20001

[commit-bot: I haz the power, 2015-05-15 22:17:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-18 11:28:21]

Patchset 2 (id:??) landed as
https://crrev.com/1626f004512fd5b52d80a18dda9c910825d8cc03
Cr-Commit-Position: refs/heads/master@{#330214}