Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(46)

Issue 2408163003: Fixup formfiller cleanup (Closed)

Created:
4 years, 2 months ago by dsinclair
Modified:
4 years, 2 months ago
Reviewers:
Lei Zhang, Tom Sepez
CC:
pdfium-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Fixup formfiller cleanup The CFFL_InteractiveFormFiller must be cleaned up before the environment because the destruction of the formfiller will trigger the destruction of the formfiller widgets. Some of those widgets may require stopping timers, which requires accessing the environment. BUG=chromium:654272, chromium:653459 Committed: https://pdfium.googlesource.com/pdfium/+/709f5a9301e91365ab87610993c497e386504ead

Patch Set 1 #

Patch Set 2 : Make sure annotation handler manager is destroyed at the right time. #

Total comments: 6

Patch Set 3 : Comment cleanup #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -0 lines) Patch
M fpdfsdk/cpdfsdk_formfillenvironment.cpp View 1 2 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (9 generated)
dsinclair
PTAL. This fix is speculative as I can not repro the crash.
4 years, 2 months ago (2016-10-11 15:25:09 UTC) #4
Tom Sepez
Ok, LGTM. Let's see how it does.
4 years, 2 months ago (2016-10-11 16:27:03 UTC) #7
dsinclair
thestig@ will this conflict with your pending CL https://codereview.chromium.org/2406893003/ ?
4 years, 2 months ago (2016-10-11 17:24:26 UTC) #8
Lei Zhang
Will this fix bug 653459 / bug 653506 as well? I think our CLs are ...
4 years, 2 months ago (2016-10-11 19:36:06 UTC) #9
Lei Zhang
On 2016/10/11 19:36:06, Lei Zhang wrote: > Will this fix bug 653459 / bug 653506 ...
4 years, 2 months ago (2016-10-11 19:39:30 UTC) #10
dsinclair
On 2016/10/11 19:39:30, Lei Zhang wrote: > On 2016/10/11 19:36:06, Lei Zhang wrote: > > ...
4 years, 2 months ago (2016-10-11 19:41:22 UTC) #11
dsinclair
PTAL. Updated to also fix 653459.
4 years, 2 months ago (2016-10-11 20:05:13 UTC) #13
Lei Zhang
LGTM, but please update the comments. https://codereview.chromium.org/2408163003/diff/20001/fpdfsdk/cpdfsdk_formfillenvironment.cpp File fpdfsdk/cpdfsdk_formfillenvironment.cpp (right): https://codereview.chromium.org/2408163003/diff/20001/fpdfsdk/cpdfsdk_formfillenvironment.cpp#newcode44 fpdfsdk/cpdfsdk_formfillenvironment.cpp:44: // it is ...
4 years, 2 months ago (2016-10-11 20:54:52 UTC) #14
dsinclair
https://codereview.chromium.org/2408163003/diff/20001/fpdfsdk/cpdfsdk_formfillenvironment.cpp File fpdfsdk/cpdfsdk_formfillenvironment.cpp (right): https://codereview.chromium.org/2408163003/diff/20001/fpdfsdk/cpdfsdk_formfillenvironment.cpp#newcode44 fpdfsdk/cpdfsdk_formfillenvironment.cpp:44: // it is cleaned up before the CFFL_InteractiveFormFiller. On ...
4 years, 2 months ago (2016-10-11 21:02:00 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2408163003/40001
4 years, 2 months ago (2016-10-11 21:02:22 UTC) #18
commit-bot: I haz the power
4 years, 2 months ago (2016-10-11 21:21:19 UTC) #20
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/709f5a9301e91365ab87610993c497e38650...

Powered by Google App Engine
This is Rietveld 408576698