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

Issue 2482273002: Cleanup CPDF_DeviceCS and friends. (Closed)

Created:
4 years, 1 month ago by Lei Zhang
Modified:
4 years, 1 month ago
Reviewers:
Tom Sepez
CC:
pdfium-reviews_googlegroups.com, Oliver Chang
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 6

Patch Set 2 : nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+108 lines, -103 lines) Patch
M core/fpdfapi/page/cpdf_colorspace.cpp View 1 chunk +2 lines, -3 lines 0 comments Download
M core/fpdfapi/page/fpdf_page_colors.cpp View 1 3 chunks +106 lines, -100 lines 0 comments Download

Messages

Total messages: 21 (13 generated)
Lei Zhang
I started looking at bug 650230 and bug 603518 and figured I'd clean up the ...
4 years, 1 month ago (2016-11-08 20:06:39 UTC) #6
Tom Sepez
https://codereview.chromium.org/2482273002/diff/40001/core/fpdfapi/page/fpdf_page_colors.cpp File core/fpdfapi/page/fpdf_page_colors.cpp (left): https://codereview.chromium.org/2482273002/diff/40001/core/fpdfapi/page/fpdf_page_colors.cpp#oldcode115 core/fpdfapi/page/fpdf_page_colors.cpp:115: ASSERT(m_Family == PDFCS_PATTERN); Looks like we lost this case? ...
4 years, 1 month ago (2016-11-08 20:29:25 UTC) #7
Lei Zhang
https://codereview.chromium.org/2482273002/diff/40001/core/fpdfapi/page/fpdf_page_colors.cpp File core/fpdfapi/page/fpdf_page_colors.cpp (left): https://codereview.chromium.org/2482273002/diff/40001/core/fpdfapi/page/fpdf_page_colors.cpp#oldcode115 core/fpdfapi/page/fpdf_page_colors.cpp:115: ASSERT(m_Family == PDFCS_PATTERN); On 2016/11/08 20:29:25, Tom Sepez wrote: ...
4 years, 1 month ago (2016-11-08 20:38:40 UTC) #8
Tom Sepez
Did you intend to upload another patch set?
4 years, 1 month ago (2016-11-08 21:55:00 UTC) #11
Lei Zhang
On 2016/11/08 21:55:00, Tom Sepez wrote: > Did you intend to upload another patch set? ...
4 years, 1 month ago (2016-11-08 22:15:34 UTC) #12
Tom Sepez
lgtm
4 years, 1 month ago (2016-11-08 22:38:51 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2482273002/60001
4 years, 1 month ago (2016-11-09 00:22:12 UTC) #19
commit-bot: I haz the power
4 years, 1 month ago (2016-11-09 00:22:33 UTC) #21
Message was sent while issue was closed.
Committed patchset #2 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/28c888b8ee6d1da93cffaa8c14833ddc4986...

Powered by Google App Engine
This is Rietveld 408576698