Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(16)

Issue 2265313002: Lazy generate an "AP" when an Annot's hidden state changes (Closed)

Created:
2 years, 1 month ago by tonikitoo
Modified:
2 years ago
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Lazy generate an "AP" when an Annot's hidden state changes Now that Document::getAnnot works and annotation instances can have its properties changed, consider the following scenario: - A PDF content has an annotation without AP and CPVT_GenerateAP is called to generate one. - However the annotation also has its hidden flag set (/F 2), and CPVT_GenerateAP bails out earlier, not generating an AP. - When the PDF's Javascript runs, it acquires an instance of this annotation object, bounded to JS using Document::getAnnot(), and set its "hidden" flag to false. - At this point, the annotation should get drawn, but it does not because its "AP" was never generated. CL fixes this scenario by making PDFium able to lazy generate APs, if needed. BUG=pdfium:492 Committed: https://pdfium.googlesource.com/pdfium/+/ade4b495433751ac853f2d677b9e1da94d0d6bf7

Patch Set 1 #

Patch Set 2 : Some minor clean ups. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -4 lines) Patch
M core/fpdfdoc/cpdf_annot.cpp View 1 3 chunks +15 lines, -4 lines 0 comments Download
M core/fpdfdoc/include/cpdf_annot.h View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (5 generated)
tonikitoo
PTAL
2 years, 1 month ago (2016-08-22 20:02:12 UTC) #3
Tom Sepez
Can we add a test case? Otherwise looks good.
2 years, 1 month ago (2016-08-23 00:12:04 UTC) #4
tonikitoo
Thanks for looking! On 2016/08/23 00:12:04, Tom Sepez (Offsite til 8-23) wrote: > Can we ...
2 years, 1 month ago (2016-08-23 00:50:46 UTC) #5
Lei Zhang
On 2016/08/23 00:50:46, tonikitoo1 wrote: > That being said, how do you guys produce PDFs ...
2 years, 1 month ago (2016-08-23 01:13:42 UTC) #6
tonikitoo
On 2016/08/23 00:12:04, Tom Sepez (Offsite til 8-23) wrote: > Can we add a test ...
2 years, 1 month ago (2016-08-23 18:50:16 UTC) #7
Lei Zhang
On 2016/08/23 18:50:16, tonikitoo1 wrote: > On 2016/08/23 00:12:04, Tom Sepez (Offsite til 8-23) wrote: ...
2 years, 1 month ago (2016-08-23 18:53:00 UTC) #8
tonikitoo
On 2016/08/23 18:53:00, Lei Zhang wrote: > On 2016/08/23 18:50:16, tonikitoo1 wrote: > > On ...
2 years, 1 month ago (2016-08-23 19:41:14 UTC) #9
Tom Sepez
> > Loading it with PDFium ToT does not show the highlight annotation. Loading it ...
2 years ago (2016-08-24 16:25:37 UTC) #11
tonikitoo
On 2016/08/24 16:25:37, Tom Sepez wrote: > > > > Loading it with PDFium ToT ...
2 years ago (2016-08-24 16:32:25 UTC) #12
Tom Sepez
Ah, so there's probably a missing call in pdfium_test to draw the annotations. If you're ...
2 years ago (2016-08-24 16:41:31 UTC) #13
tonikitoo
On 2016/08/24 16:41:31, Tom Sepez wrote: > Ah, so there's probably a missing call in ...
2 years ago (2016-08-24 17:11:52 UTC) #14
Tom Sepez
Ok, LGTM. Thanks for your work.
2 years ago (2016-08-24 17:22:57 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/2265313002/20001
2 years ago (2016-08-24 17:26:23 UTC) #17
tonikitoo
On 2016/08/24 17:22:57, Tom Sepez wrote: > Ok, LGTM. Thanks for your work. Thanks for ...
2 years ago (2016-08-24 17:26:46 UTC) #18
commit-bot: I haz the power
2 years ago (2016-08-24 17:37:07 UTC) #20
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/ade4b495433751ac853f2d677b9e1da94d0d...

Powered by Google App Engine
This is Rietveld 408576698