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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1422113003: Fix XFA compilation noise, part 1. (Closed)

Created:
5 years, 1 month ago by Tom Sepez
Modified:
5 years, 1 month 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

Fix XFA compilation noise, part 1. Mainly: volatile/override/method shadowing. unused variables. intialization list init order. R=thestig@chromium.org Committed: https://pdfium.googlesource.com/pdfium/+/3343d146f2d9cd2f9eaa3a40fed3a5b94f3f107e

Patch Set 1 #

Total comments: 18

Patch Set 2 : Nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+159 lines, -263 lines) Patch
M fpdfsdk/include/formfiller/FFL_ComboBox.h View 1 1 chunk +2 lines, -1 line 0 comments Download
M fpdfsdk/include/formfiller/FFL_IFormFiller.h View 1 2 chunks +6 lines, -7 lines 0 comments Download
M fpdfsdk/include/formfiller/FFL_TextField.h View 1 1 chunk +2 lines, -1 line 0 comments Download
M fpdfsdk/include/fsdk_baseannot.h View 5 chunks +4 lines, -7 lines 0 comments Download
M fpdfsdk/include/fsdk_baseform.h View 1 2 chunks +7 lines, -12 lines 0 comments Download
M fpdfsdk/src/fpdfsave.cpp View 3 chunks +8 lines, -17 lines 0 comments Download
M fpdfsdk/src/fpdfxfa/fpdfxfa_page.cpp View 1 chunk +12 lines, -15 lines 0 comments Download
M fpdfsdk/src/fsdk_annothandler.cpp View 1 chunk +0 lines, -3 lines 0 comments Download
M fpdfsdk/src/fsdk_baseannot.cpp View 2 chunks +1 line, -5 lines 0 comments Download
M fpdfsdk/src/fsdk_baseform.cpp View 1 2 chunks +3 lines, -13 lines 0 comments Download
M fpdfsdk/src/javascript/JS_Runtime.h View 1 1 chunk +5 lines, -4 lines 0 comments Download
M xfa/src/fdp/src/tto/fde_textout.h View 1 chunk +0 lines, -1 line 0 comments Download
M xfa/src/fdp/src/tto/fde_textout.cpp View 1 chunk +1 line, -1 line 0 comments Download
M xfa/src/fdp/src/xml/fde_xml.cpp View 2 chunks +5 lines, -4 lines 0 comments Download
M xfa/src/fee/src/fee/fde_txtedtbuf.cpp View 1 chunk +4 lines, -6 lines 0 comments Download
M xfa/src/fee/src/fee/fde_txtedtengine.h View 2 chunks +1 line, -2 lines 0 comments Download
M xfa/src/fee/src/fee/fde_txtedtengine.cpp View 1 7 chunks +8 lines, -17 lines 0 comments Download
M xfa/src/fee/src/fee/fde_txtedtpage.h View 1 chunk +1 line, -1 line 0 comments Download
M xfa/src/fee/src/fee/fde_txtedtpage.cpp View 1 5 chunks +5 lines, -10 lines 0 comments Download
M xfa/src/fee/src/fee/fde_txtedtparag.cpp View 3 chunks +2 lines, -4 lines 0 comments Download
M xfa/src/fgas/src/layout/fx_rtfbreak.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M xfa/src/fgas/src/localization/fx_locale.cpp View 8 chunks +6 lines, -14 lines 0 comments Download
M xfa/src/fgas/src/xml/fx_sax_imp.cpp View 1 chunk +5 lines, -20 lines 0 comments Download
M xfa/src/fwl/src/basewidget/fwl_comboboximp.cpp View 5 chunks +6 lines, -11 lines 0 comments Download
M xfa/src/fwl/src/basewidget/fwl_datetimepickerimp.cpp View 2 chunks +8 lines, -10 lines 0 comments Download
M xfa/src/fwl/src/basewidget/fwl_editimp.cpp View 4 chunks +12 lines, -14 lines 0 comments Download
M xfa/src/fwl/src/basewidget/fwl_listboximp.cpp View 3 chunks +2 lines, -8 lines 0 comments Download
M xfa/src/fwl/src/basewidget/fwl_monthcalendarimp.cpp View 3 chunks +8 lines, -12 lines 0 comments Download
M xfa/src/fwl/src/basewidget/fwl_scrollbarimp.cpp View 2 chunks +4 lines, -4 lines 0 comments Download
M xfa/src/fwl/src/basewidget/fwl_spinbuttonimp.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M xfa/src/fwl/src/basewidget/include/fwl_editimp.h View 1 1 chunk +16 lines, -13 lines 0 comments Download
M xfa/src/fwl/src/basewidget/include/fwl_scrollbarimp.h View 1 chunk +0 lines, -1 line 0 comments Download
M xfa/src/fwl/src/core/fwl_formimp.cpp View 1 chunk +1 line, -1 line 0 comments Download
M xfa/src/fwl/src/core/fwl_noteimp.cpp View 2 chunks +4 lines, -6 lines 0 comments Download
M xfa/src/fwl/src/core/fwl_threadimp.cpp View 1 chunk +1 line, -3 lines 0 comments Download
M xfa/src/fwl/src/core/include/fwl_noteimp.h View 1 chunk +1 line, -1 line 0 comments Download
M xfa/src/fwl/src/theme/formtp.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M xfa/src/fwl/src/theme/pushbuttontp.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M xfa/src/fwl/src/theme/scrollbartp.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M xfa/src/fwl/src/theme/widgettp.cpp View 2 chunks +0 lines, -3 lines 0 comments Download
M xfa/src/fxbarcode/common/BC_CommonPerspectiveTransform.cpp View 1 chunk +6 lines, -5 lines 0 comments Download

Messages

Total messages: 7 (1 generated)
Tom Sepez
Lei, for review.
5 years, 1 month ago (2015-10-30 23:54:05 UTC) #2
Lei Zhang
Not done yet... we should as about Jun about FDE_USEFORMATBLOCK. https://codereview.chromium.org/1422113003/diff/1/xfa/src/fee/src/fee/fde_txtedtengine.cpp File xfa/src/fee/src/fee/fde_txtedtengine.cpp (right): https://codereview.chromium.org/1422113003/diff/1/xfa/src/fee/src/fee/fde_txtedtengine.cpp#newcode34 ...
5 years, 1 month ago (2015-10-31 01:51:42 UTC) #3
Lei Zhang
On 2015/10/31 01:51:42, Lei Zhang wrote: > Not done yet... we should as about Jun ...
5 years, 1 month ago (2015-10-31 01:51:59 UTC) #4
Lei Zhang
lgtm https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/FFL_ComboBox.h File fpdfsdk/include/formfiller/FFL_ComboBox.h (right): https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/FFL_ComboBox.h#newcode58 fpdfsdk/include/formfiller/FFL_ComboBox.h:58: FX_BOOL IsFieldFull(CPDFSDK_PageView* pPageView) override; // CFFL_FormFiller: https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/FFL_IFormFiller.h File ...
5 years, 1 month ago (2015-10-31 02:03:20 UTC) #5
Tom Sepez
Committed patchset #2 (id:20001) manually as 3343d146f2d9cd2f9eaa3a40fed3a5b94f3f107e (presubmit successful).
5 years, 1 month ago (2015-11-02 17:54:59 UTC) #6
Tom Sepez
5 years, 1 month ago (2015-11-02 18:03:25 UTC) #7
Message was sent while issue was closed. 
https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/...
File fpdfsdk/include/formfiller/FFL_ComboBox.h (right):

https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/...
fpdfsdk/include/formfiller/FFL_ComboBox.h:58: FX_BOOL
IsFieldFull(CPDFSDK_PageView* pPageView) override;
On 2015/10/31 02:03:20, Lei Zhang wrote:
> // CFFL_FormFiller:

Done.

https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/...
File fpdfsdk/include/formfiller/FFL_IFormFiller.h (right):

https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/...
fpdfsdk/include/formfiller/FFL_IFormFiller.h:82: void OnPopupPreOpen(void*
pPrivateData,
On 2015/10/31 02:03:20, Lei Zhang wrote:
> combine with block at line 146

Done.

https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/...
File fpdfsdk/include/formfiller/FFL_TextField.h (right):

https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/include/formfiller/...
fpdfsdk/include/formfiller/FFL_TextField.h:59: FX_BOOL
IsFieldFull(CPDFSDK_PageView* pPageView) override;
On 2015/10/31 02:03:20, Lei Zhang wrote:
> // CFFL_FormFiller:

Done.

https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/src/javascript/JS_R...
File fpdfsdk/src/javascript/JS_Runtime.h (right):

https://codereview.chromium.org/1422113003/diff/1/fpdfsdk/src/javascript/JS_R...
fpdfsdk/src/javascript/JS_Runtime.h:60: FX_BOOL GetHValueByName(const
CFX_ByteStringC& utf8Name,
On 2015/10/31 02:03:20, Lei Zhang wrote:
> Add // IJS_Runtime:

Done.

https://codereview.chromium.org/1422113003/diff/1/xfa/src/fee/src/fee/fde_txt...
File xfa/src/fee/src/fee/fde_txtedtengine.cpp (right):

https://codereview.chromium.org/1422113003/diff/1/xfa/src/fee/src/fee/fde_txt...
xfa/src/fee/src/fee/fde_txtedtengine.cpp:34: : m_pTextBreak(NULL),
On 2015/10/31 01:51:41, Lei Zhang wrote:
> nullptr

Done.

https://codereview.chromium.org/1422113003/diff/1/xfa/src/fee/src/fee/fde_txt...
File xfa/src/fee/src/fee/fde_txtedtpage.cpp (right):

https://codereview.chromium.org/1422113003/diff/1/xfa/src/fee/src/fee/fde_txt...
xfa/src/fee/src/fee/fde_txtedtpage.cpp:128: m_pBgnParag(NULL),
On 2015/10/31 01:51:41, Lei Zhang wrote:
> more nullptr ?

Done.

https://codereview.chromium.org/1422113003/diff/1/xfa/src/fee/src/fee/fde_txt...
File xfa/src/fee/src/fee/fde_txtedtpage.h (right):

https://codereview.chromium.org/1422113003/diff/1/xfa/src/fee/src/fee/fde_txt...
xfa/src/fee/src/fee/fde_txtedtpage.h:25: inline FDE_TXTEDT_CHARTYPE
FDE_GetEditSelCharType(FX_WCHAR wChar) {
On 2015/10/31 01:51:42, Lei Zhang wrote:
> Seems a bit big to inline. Move impl to .cpp file?

Yeah, followup.  For now, get rid of static fn in .h file code smell.

https://codereview.chromium.org/1422113003/diff/1/xfa/src/fwl/src/basewidget/...
File xfa/src/fwl/src/basewidget/include/fwl_editimp.h (right):

https://codereview.chromium.org/1422113003/diff/1/xfa/src/fwl/src/basewidget/...
xfa/src/fwl/src/basewidget/include/fwl_editimp.h:24: FWL_ERR
GetClassName(CFX_WideString& wsClass) const override;
On 2015/10/31 01:51:42, Lei Zhang wrote:
> Add: // CFWL_WidgetImp:

Done.

Powered by Google App Engine
This is Rietveld 408576698