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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1915323002: Remove no-op ShowFileDialog(s). (Closed)

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

Description

Remove no-op ShowFileDialog(s). Two different versions, one in xfa and one in fpdfsdk, both do nothing. Committed: https://pdfium.googlesource.com/pdfium/+/186a78eb0a821a2f1feef524cb590d0fea2009ba

Patch Set 1 #

Total comments: 3

Patch Set 2 : More code removed. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -79 lines) Patch
M fpdfsdk/fpdfxfa/fpdfxfa_app.cpp View 1 chunk +0 lines, -7 lines 0 comments Download
M fpdfsdk/fpdfxfa/include/fpdfxfa_app.h View 1 chunk +0 lines, -4 lines 0 comments Download
M fpdfsdk/include/fsdk_mgr.h View 1 chunk +0 lines, -7 lines 0 comments Download
M xfa/fxfa/app/xfa_ffimageedit.cpp View 1 2 chunks +6 lines, -57 lines 0 comments Download
M xfa/fxfa/include/fxfa.h View 1 chunk +0 lines, -4 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
Tom Sepez
Lei, dead code.
4 years, 8 months ago (2016-04-25 22:58:20 UTC) #2
Tom Sepez
https://codereview.chromium.org/1915323002/diff/1/xfa/fxfa/app/xfa_ffimageedit.cpp File xfa/fxfa/app/xfa_ffimageedit.cpp (left): https://codereview.chromium.org/1915323002/diff/1/xfa/fxfa/app/xfa_ffimageedit.cpp#oldcode115 xfa/fxfa/app/xfa_ffimageedit.cpp:115: return TRUE; This is always hit .
4 years, 8 months ago (2016-04-25 22:59:09 UTC) #3
dsinclair
https://codereview.chromium.org/1915323002/diff/1/xfa/fxfa/app/xfa_ffimageedit.cpp File xfa/fxfa/app/xfa_ffimageedit.cpp (left): https://codereview.chromium.org/1915323002/diff/1/xfa/fxfa/app/xfa_ffimageedit.cpp#oldcode109 xfa/fxfa/app/xfa_ffimageedit.cpp:109: pAppProvider->LoadString(XFA_IDS_ImageFilter, wsFilter); This can also be removed I think. ...
4 years, 8 months ago (2016-04-26 00:05:40 UTC) #5
Tom Sepez
https://codereview.chromium.org/1915323002/diff/1/xfa/fxfa/app/xfa_ffimageedit.cpp File xfa/fxfa/app/xfa_ffimageedit.cpp (left): https://codereview.chromium.org/1915323002/diff/1/xfa/fxfa/app/xfa_ffimageedit.cpp#oldcode109 xfa/fxfa/app/xfa_ffimageedit.cpp:109: pAppProvider->LoadString(XFA_IDS_ImageFilter, wsFilter); On 2016/04/26 00:05:40, dsinclair wrote: > This ...
4 years, 8 months ago (2016-04-26 16:13:02 UTC) #6
dsinclair
lgtm
4 years, 8 months ago (2016-04-26 16:20:13 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1915323002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1915323002/20001
4 years, 8 months ago (2016-04-26 16:20:17 UTC) #9
commit-bot: I haz the power
4 years, 8 months ago (2016-04-26 16:32:17 UTC) #11
Message was sent while issue was closed. 
Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/186a78eb0a821a2f1feef524cb590d0fea20...

Powered by Google App Engine
This is Rietveld 408576698