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

Issue 2330043002: Avoid static initializers and global variables in 'pdfium_test'. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 2 weeks ago by tonikitoo
Modified:
8 months, 1 week ago
CC:
pdfium-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Avoid static initializers and global variables in 'pdfium_test'. This CL aims to follow up on thestig's past-land feedback receveid on https://codereview.chromium.org/2277063003/#msg15 . In order to avoid static initializers and global data declaration, CL adds a data struct named PDFiumTest_FPDF_FORMFILLINFO, which has FPDF_FORMFILLINFO as the first struct member definition. This allows the use of reinterpret_cast to/from PDFiumTest_FPDF_FORMFILLINFO and PDF_FORMFILLINFO. PDFiumTest_FPDF_FORMFILLINFO also two more struct members, 'loadedPages' and 'formHandle', which were previously global variables. Committed: https://pdfium.googlesource.com/pdfium/+/81d92f8ffc5e632ade8c507b8f2e34c5fe3ca902

Patch Set 1 #

Total comments: 4

Patch Set 2 : addressed tsepez' feedback #

Total comments: 2

Patch Set 3 : removed code left over #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -20 lines) Patch
M samples/pdfium_test.cc View 1 2 7 chunks +34 lines, -20 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 14 (4 generated)
tonikitoo
PTAL
8 months, 2 weeks ago (2016-09-12 15:07:08 UTC) #3
Tom Sepez
https://codereview.chromium.org/2330043002/diff/1/samples/pdfium_test.cc File samples/pdfium_test.cc (right): https://codereview.chromium.org/2330043002/diff/1/samples/pdfium_test.cc#newcode70 samples/pdfium_test.cc:70: typedef struct { nit: In C++, we can avoid ...
8 months, 2 weeks ago (2016-09-12 17:19:41 UTC) #4
tonikitoo
Comments addressed. https://codereview.chromium.org/2330043002/diff/1/samples/pdfium_test.cc File samples/pdfium_test.cc (right): https://codereview.chromium.org/2330043002/diff/1/samples/pdfium_test.cc#newcode70 samples/pdfium_test.cc:70: typedef struct { On 2016/09/12 17:19:41, Tom ...
8 months, 2 weeks ago (2016-09-12 23:41:55 UTC) #5
tonikitoo
Friendly ping.
8 months, 1 week ago (2016-09-20 02:59:03 UTC) #6
dsinclair
https://codereview.chromium.org/2330043002/diff/20001/samples/pdfium_test.cc File samples/pdfium_test.cc (right): https://codereview.chromium.org/2330043002/diff/20001/samples/pdfium_test.cc#newcode658 samples/pdfium_test.cc:658: memset(&form_callbacks, '\0', sizeof(platform_callbacks)); Shouldn't this be form_callbacks?
8 months, 1 week ago (2016-09-21 17:25:13 UTC) #7
tonikitoo
https://codereview.chromium.org/2330043002/diff/20001/samples/pdfium_test.cc File samples/pdfium_test.cc (right): https://codereview.chromium.org/2330043002/diff/20001/samples/pdfium_test.cc#newcode658 samples/pdfium_test.cc:658: memset(&form_callbacks, '\0', sizeof(platform_callbacks)); On 2016/09/21 17:25:13, dsinclair wrote: > ...
8 months, 1 week ago (2016-09-21 18:21:35 UTC) #8
Tom Sepez
lgtm
8 months, 1 week ago (2016-09-21 18:52:59 UTC) #9
dsinclair
lgtm
8 months, 1 week ago (2016-09-21 18:53:48 UTC) #11
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/2330043002/40001
8 months, 1 week ago (2016-09-21 18:53:58 UTC) #12
commit-bot: I haz the power
8 months, 1 week ago (2016-09-21 19:44:59 UTC) #14
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/81d92f8ffc5e632ade8c507b8f2e34c5fe3c...
Sign in to reply to this message.

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