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

Issue 1321293003: Fix a #include in fpdf_page_func.cpp. (Closed)

Created:
5 years, 3 months ago by David Lattimore
Modified:
5 years, 3 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

Fix a #include in fpdf_page_func.cpp. Not sure why building with gyp was working despite the missing '../' but it wasn't working in stricter build systems. BUG= R=thestig@chromium.org Committed: https://pdfium.googlesource.com/pdfium/+/fa31d9630aadfe101d3b35e26ce0fc926ec1505b

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fixed include order #

Patch Set 3 : Added a missing include to safe_conversions_impl.h #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/base/numerics/safe_conversions_impl.h View 1 2 2 chunks +1 line, -1 line 0 comments Download

Messages

Total messages: 9 (0 generated)
David Lattimore
5 years, 3 months ago (2015-09-09 00:17:05 UTC) #1
David Lattimore
5 years, 3 months ago (2015-09-09 00:19:29 UTC) #2
Lei Zhang
lgtm It's not gyp's job to catch this. For whatever reason, the #include paths work ...
5 years, 3 months ago (2015-09-09 00:30:47 UTC) #3
David Lattimore
https://codereview.chromium.org/1321293003/diff/1/core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp File core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp (right): https://codereview.chromium.org/1321293003/diff/1/core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp#newcode12 core/src/fpdfapi/fpdf_page/fpdf_page_func.cpp:12: #include "../../../../third_party/base/numerics/safe_conversions_impl.h" On 2015/09/09 00:30:47, Lei Zhang wrote: > ...
5 years, 3 months ago (2015-09-09 00:38:20 UTC) #4
Lei Zhang
++lgtm BTW, we have pdfium bug 65 and 66 open to sanitize the #includes.
5 years, 3 months ago (2015-09-09 00:49:43 UTC) #5
Lei Zhang
There's this CL to land too.
5 years, 3 months ago (2015-09-14 21:42:32 UTC) #6
David Lattimore
Thanks. I just tried building and found that safe_conversions_impl.h wasn't including assert.h. This wasn't causing ...
5 years, 3 months ago (2015-09-14 21:54:11 UTC) #7
Lei Zhang
patch set 3 still lgtm
5 years, 3 months ago (2015-09-17 20:33:43 UTC) #8
David Lattimore
5 years, 3 months ago (2015-09-17 21:39:55 UTC) #9
Message was sent while issue was closed.
Committed patchset #3 (id:30001) manually as
fa31d9630aadfe101d3b35e26ce0fc926ec1505b (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698