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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1395353004: Merge to XFA: Introduce CPDFDocumentFromFPDFDocument(). (Closed)

Created:
5 years, 2 months ago by Tom Sepez
Modified:
5 years, 2 months ago
Reviewers:
Lei Zhang
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@xfa
Target Ref:
refs/heads/xfa
Visibility:
Public.

Description

Merge to XFA: Introduce CPDF_DocumentFromFPDFDocument(). Manual merges everywhere. Original Review URL: https://codereview.chromium.org/1395493007 . (cherry picked from commit bb51c4fb6bc6095984c303c95e5336f83e66bc67) Original Review URL: https://codereview.chromium.org/1396283006 . (cherry picked from commit 86adb658843658969041a13bf6cf0cc950ffe8ed) R=thestig@chromium.org Committed: https://pdfium.googlesource.com/pdfium/+/471a1034e70ddcfc93ce2a93b28a8385109e83de

Patch Set 1 #

Patch Set 2 : Handle null doc. #

Patch Set 3 : Rebase, fix layering. #

Patch Set 4 : Fix build. #

Total comments: 11

Patch Set 5 : Nits. #

Patch Set 6 : Remove another pointless local var. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -128 lines) Patch
M fpdfsdk/include/fsdk_define.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M fpdfsdk/src/fpdf_dataavail.cpp View 1 2 3 4 1 chunk +2 lines, -4 lines 0 comments Download
M fpdfsdk/src/fpdf_ext.cpp View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M fpdfsdk/src/fpdfdoc.cpp View 1 2 10 chunks +37 lines, -30 lines 0 comments Download
M fpdfsdk/src/fpdfeditimg.cpp View 1 2 1 chunk +4 lines, -3 lines 0 comments Download
M fpdfsdk/src/fpdfeditpage.cpp View 1 2 2 chunks +5 lines, -10 lines 0 comments Download
M fpdfsdk/src/fpdfppo.cpp View 1 2 2 chunks +21 lines, -20 lines 0 comments Download
M fpdfsdk/src/fpdfview.cpp View 1 2 3 4 5 9 chunks +43 lines, -58 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
Tom Sepez
Lei, for review. https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdfdoc.cpp File fpdfsdk/src/fpdfdoc.cpp (left): https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdfdoc.cpp#oldcode110 fpdfsdk/src/fpdfdoc.cpp:110: CPDF_Document* pDoc = (CPDF_Document*)document; note: this ...
5 years, 2 months ago (2015-10-15 22:08:23 UTC) #2
Lei Zhang
lgtm https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdf_dataavail.cpp File fpdfsdk/src/fpdf_dataavail.cpp (right): https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdf_dataavail.cpp#newcode125 fpdfsdk/src/fpdf_dataavail.cpp:125: return ((CPDF_Parser*)pDoc->GetParser())->GetFirstPageNo(); BTW, this cast is pointless. I ...
5 years, 2 months ago (2015-10-15 22:44:27 UTC) #3
Tom Sepez
Committed patchset #6 (id:100001) manually as 471a1034e70ddcfc93ce2a93b28a8385109e83de (presubmit successful).
5 years, 2 months ago (2015-10-15 23:17:23 UTC) #4
Tom Sepez
5 years, 2 months ago (2015-10-15 23:18:09 UTC) #5
Message was sent while issue was closed. 
https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdf_dataav...
File fpdfsdk/src/fpdf_dataavail.cpp (right):

https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdf_dataav...
fpdfsdk/src/fpdf_dataavail.cpp:125: return
((CPDF_Parser*)pDoc->GetParser())->GetFirstPageNo();
On 2015/10/15 22:44:26, Lei Zhang wrote:
> BTW, this cast is pointless. I need some shades so I can ignore other things.
Done.  Also a subtle error where the !pDoc not !doc should be the conditional
expression.

https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdfview.cpp
File fpdfsdk/src/fpdfview.cpp (right):

https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdfview.cp...
fpdfsdk/src/fpdfview.cpp:385: *fileVersion = 0;
On 2015/10/15 22:44:27, Lei Zhang wrote:
> There's a subtle behavior change, but I don't think we care since it was
already
> inconsistent before.
Yes, intentional, was trying to avoid an uninitialized read by callers who don't
check return status.

https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdfview.cp...
fpdfsdk/src/fpdfview.cpp:390: CPDF_Parser* pParser =
(CPDF_Parser*)pDoc->GetParser();
On 2015/10/15 22:44:27, Lei Zhang wrote:
> Same pointless cast? More below.
And you know what we do to pointless casts.

https://codereview.chromium.org/1395353004/diff/60001/fpdfsdk/src/fpdfview.cp...
fpdfsdk/src/fpdfview.cpp:672: CPDF_Document* pDoc =
CPDFDocumentFromFPDFDocument(document);
On 2015/10/15 22:44:27, Lei Zhang wrote:
> Do we even need the variable anymore?

Done.

Powered by Google App Engine
This is Rietveld 408576698