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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 837723009: Add tests for GetNamedDests() API. (Closed)

Created:
5 years, 11 months ago by Tom Sepez
Modified:
5 years, 11 months ago
Reviewers:
Bo Xu, jam
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Add tests for GetNamedDests() API. Follow-on work from patch at https://codereview.chromium.org/845643008. This incorporates that patch, plus adds tests for it and similar conditions. A few changes are introduced to correct expected behaviour. This also incoprorates Deepak's fix in https://codereview.chromium.org/845643008/ This incorporates a formerly unlanded tool to generate small valid PDF files from template input, which is needed to cover all the error cases here. See https://codereview.chromium.org/791993006/ BUG=450133 R=bo_xu@foxitsoftware.com Committed: https://pdfium.googlesource.com/pdfium/+/9dcd7b841a6ec11b3cfff39f8d2d1f033dea9705

Patch Set 1 #

Patch Set 2 : renumber to shorten xref table #

Patch Set 3 : 80 columns #

Total comments: 1

Patch Set 4 : Fix Dest item syntax #

Patch Set 5 : Remove stray pasted comment. #

Patch Set 6 : More complex nametree, fix comments, rebase. #

Total comments: 3

Patch Set 7 : Rebase, adjust buffer_length type. #

Total comments: 2

Patch Set 8 : fix typo #

Patch Set 9 : Fix min() #

Unified diffs Side-by-side diffs Delta from patch set Stats (+349 lines, -1 line) Patch
M fpdfsdk/src/fpdfview.cpp View 1 2 3 4 5 6 2 chunks +4 lines, -1 line 0 comments Download
M fpdfsdk/src/fpdfview_embeddertest.cpp View 1 2 3 4 5 6 7 8 2 chunks +100 lines, -0 lines 0 comments Download
A testing/resources/named_dests.in View 1 2 3 4 5 1 chunk +69 lines, -0 lines 0 comments Download
A testing/resources/named_dests.pdf View 1 2 3 4 5 6 7 8 1 chunk +87 lines, -0 lines 0 comments Download
A testing/tools/fixup_pdf_template.py View 1 chunk +89 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (1 generated)
Tom Sepez
Bo, John, please review.
5 years, 11 months ago (2015-01-20 21:31:28 UTC) #2
Tom Sepez
On 2015/01/20 21:31:28, Tom Sepez wrote: Bo, in particular, you might want to run the ...
5 years, 11 months ago (2015-01-20 21:36:32 UTC) #3
Bo Xu
The string length bug is fixed in https://codereview.chromium.org/860973002/ https://codereview.chromium.org/837723009/diff/40001/fpdfsdk/src/fpdfview.cpp File fpdfsdk/src/fpdfview.cpp (right): https://codereview.chromium.org/837723009/diff/40001/fpdfsdk/src/fpdfview.cpp#newcode820 fpdfsdk/src/fpdfview.cpp:820: DLLEXPORT ...
5 years, 11 months ago (2015-01-21 00:04:12 UTC) #4
Tom Sepez
> https://codereview.chromium.org/837723009/diff/40001/fpdfsdk/src/fpdfview.cpp#newcode820 > fpdfsdk/src/fpdfview.cpp:820: DLLEXPORT FPDF_DEST STDCALL > FPDF_GetNamedDest(FPDF_DOCUMENT document, int index, void* buffer, unsigned ...
5 years, 11 months ago (2015-01-21 00:10:20 UTC) #5
Tom Sepez
Ok, this one should be ready-to-go following the changes Bo's recently landed.
5 years, 11 months ago (2015-01-21 22:25:26 UTC) #6
Bo Xu
https://codereview.chromium.org/837723009/diff/100001/fpdfsdk/src/fpdfview_embeddertest.cpp File fpdfsdk/src/fpdfview_embeddertest.cpp (right): https://codereview.chromium.org/837723009/diff/100001/fpdfsdk/src/fpdfview_embeddertest.cpp#newcode74 fpdfsdk/src/fpdfview_embeddertest.cpp:74: // Try to retrieve the second item with ample ...
5 years, 11 months ago (2015-01-21 22:26:08 UTC) #7
Bo Xu
https://codereview.chromium.org/837723009/diff/120001/fpdfsdk/src/fpdfview_embeddertest.cpp File fpdfsdk/src/fpdfview_embeddertest.cpp (right): https://codereview.chromium.org/837723009/diff/120001/fpdfsdk/src/fpdfview_embeddertest.cpp#newcode131 fpdfsdk/src/fpdfview_embeddertest.cpp:131: Same test case here?
5 years, 11 months ago (2015-01-21 22:29:42 UTC) #8
Tom Sepez
https://codereview.chromium.org/837723009/diff/120001/fpdfsdk/src/fpdfview_embeddertest.cpp File fpdfsdk/src/fpdfview_embeddertest.cpp (right): https://codereview.chromium.org/837723009/diff/120001/fpdfsdk/src/fpdfview_embeddertest.cpp#newcode131 fpdfsdk/src/fpdfview_embeddertest.cpp:131: On 2015/01/21 22:29:42, Bo Xu wrote: > Same test ...
5 years, 11 months ago (2015-01-21 22:36:50 UTC) #9
Bo Xu
LGTM
5 years, 11 months ago (2015-01-21 22:37:50 UTC) #10
Tom Sepez
5 years, 11 months ago (2015-01-21 22:38:38 UTC) #11
Message was sent while issue was closed. 
Committed patchset #9 (id:130006) manually as
9dcd7b841a6ec11b3cfff39f8d2d1f033dea9705 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698