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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 1 week ago by tonikitoo
Modified:
9 months, 1 week 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
Commit queue not available (can’t edit this change).

Messages

Total messages: 20 (5 generated)
tonikitoo
PTAL
9 months, 1 week ago (2016-08-22 20:02:12 UTC) #3
Tom Sepez
Can we add a test case? Otherwise looks good.
9 months, 1 week 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 ...
9 months, 1 week 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 ...
9 months, 1 week 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 ...
9 months, 1 week 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: ...
9 months, 1 week 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 ...
9 months, 1 week ago (2016-08-23 19:41:14 UTC) #9
Tom Sepez
> > Loading it with PDFium ToT does not show the highlight annotation. Loading it ...
9 months, 1 week 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 ...
9 months, 1 week 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 ...
9 months, 1 week 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 ...
9 months, 1 week ago (2016-08-24 17:11:52 UTC) #14
Tom Sepez
Ok, LGTM. Thanks for your work.
9 months, 1 week 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
9 months, 1 week 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 ...
9 months, 1 week ago (2016-08-24 17:26:46 UTC) #18
commit-bot: I haz the power
9 months, 1 week 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06