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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1740223002: Replace or remove _FX control-flow defines (Closed)

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

Description

Replace or remove _FX control-flow defines They buy us nothing and obfuscate the logic. R=thestig@chromium.org Committed: https://pdfium.googlesource.com/pdfium/+/dfefa9b2c99e0588bf229e2b92d85c88e5f18a41

Patch Set 1 : remove unused _FX control-flow defines (all red). #

Patch Set 2 : One more unused. #

Patch Set 3 : expand macro and format. #

Patch Set 4 : Remove last #define. #

Patch Set 5 : Two manual changes. #

Total comments: 4

Patch Set 6 : Fix silly stuff. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+187 lines, -134 lines) Patch
M xfa/include/fxgraphics/fx_graphics.h View 1 2 3 1 chunk +0 lines, -30 lines 0 comments Download
M xfa/src/fxfa/src/app/xfa_ffwidget.h View 1 chunk +0 lines, -5 lines 0 comments Download
M xfa/src/fxgraphics/src/fx_graphics.cpp View 1 2 3 4 5 60 chunks +187 lines, -99 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
Tom Sepez
Note: sed for macro expansion, no manual expansion.
4 years, 10 months ago (2016-02-26 18:30:18 UTC) #3
Tom Sepez
On 2016/02/26 18:30:18, Tom Sepez wrote: > Note: sed for macro expansion, no manual expansion. ...
4 years, 10 months ago (2016-02-26 18:38:22 UTC) #4
Lei Zhang
lgtm, but there's a couple more silly bits to fix: https://codereview.chromium.org/1740223002/diff/80001/xfa/src/fxgraphics/src/fx_graphics.cpp File xfa/src/fxgraphics/src/fx_graphics.cpp (right): https://codereview.chromium.org/1740223002/diff/80001/xfa/src/fxgraphics/src/fx_graphics.cpp#newcode502 ...
4 years, 10 months ago (2016-02-26 19:40:55 UTC) #5
Tom Sepez
Committed patchset #6 (id:100001) manually as dfefa9b2c99e0588bf229e2b92d85c88e5f18a41 (presubmit successful).
4 years, 10 months ago (2016-02-26 19:49:52 UTC) #7
Tom Sepez
4 years, 10 months ago (2016-02-26 19:50:09 UTC) #8
Message was sent while issue was closed. 
https://codereview.chromium.org/1740223002/diff/80001/xfa/src/fxgraphics/src/...
File xfa/src/fxgraphics/src/fx_graphics.cpp (right):

https://codereview.chromium.org/1740223002/diff/80001/xfa/src/fxgraphics/src/...
xfa/src/fxgraphics/src/fx_graphics.cpp:502: FX_BOOL result = FX_ERR_Succeeded;
On 2016/02/26 19:40:55, Lei Zhang wrote:
> uh
Uh-oh. Probably Does the opposite of what the want, since succeeded = 0.

https://codereview.chromium.org/1740223002/diff/80001/xfa/src/fxgraphics/src/...
xfa/src/fxgraphics/src/fx_graphics.cpp:647: FX_BOOL result = FX_ERR_Indefinite;
On 2016/02/26 19:40:55, Lei Zhang wrote:
> Combine with line 649.

Done.

Powered by Google App Engine
This is Rietveld 408576698