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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months, 1 week ago by tonikitoo
Modified:
11 months 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
11 months, 1 week ago (2016-08-22 20:02:12 UTC) #3
Tom Sepez
Can we add a test case? Otherwise looks good.
11 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 ...
11 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 ...
11 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 ...
11 months 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: ...
11 months 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 ...
11 months ago (2016-08-23 19:41:14 UTC) #9
Tom Sepez
> > Loading it with PDFium ToT does not show the highlight annotation. Loading it ...
11 months 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 ...
11 months 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 ...
11 months 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 ...
11 months ago (2016-08-24 17:11:52 UTC) #14
Tom Sepez
Ok, LGTM. Thanks for your work.
11 months 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
11 months 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 ...
11 months ago (2016-08-24 17:26:46 UTC) #18
commit-bot: I haz the power
11 months 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 25c286973