Merge to M47: Fix extraction of colour components in CPDF_DIBSource::DownSampleScanline32Bit

Merge to M47: Fix extraction of colour components in CPDF_DIBSource::DownSampleScanline32Bit

Previously, if |m_bpc| was < 8 (e.g. 4), this function may still try to
access the source components as if |m_bpc| == 8. Even when it fell into
the codepath that tried to do the right thing in this case, it was
wrong.

BUG=554151
Review URL: https://codereview.chromium.org/1433423002 .

(cherry picked from commit e21fe98d5b5da7da01503b985b07b90c8e811689)

R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/4ec1953b4ace8248ac443effafa5f651e540d995

[Oliver Chang, 2015-11-13 06:28:23]

Description was changed from

==========
Merge to M47: Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

Previously, if |m_bpc| was < 8 (e.g. 4), this function may still try to
access the source components as if |m_bpc| == 8. Even when it fell into
the codepath that tried to do the right thing in this case, it was
wrong.

BUG=554151

Committed:
https://pdfium.googlesource.com/pdfium/+/9b99615806e358fdb396d1cb162ee2e69c2a...

Review URL: https://codereview.chromium.org/1433423002 .

(cherry picked from commit e21fe98d5b5da7da01503b985b07b90c8e811689)
==========

to

==========
Merge to M47: Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

Previously, if |m_bpc| was < 8 (e.g. 4), this function may still try to
access the source components as if |m_bpc| == 8. Even when it fell into
the codepath that tried to do the right thing in this case, it was
wrong.

BUG=554151

Conflicts in BUILD.gn, pdfium.gyp, and fpdf_render_loadimage.cpp

Committed:
https://pdfium.googlesource.com/pdfium/+/9b99615806e358fdb396d1cb162ee2e69c2a...

Review URL: https://codereview.chromium.org/1433423002 .

(cherry picked from commit e21fe98d5b5da7da01503b985b07b90c8e811689)
==========

[Oliver Chang, 2015-11-13 07:03:29]

Description was changed from

==========
Merge to M47: Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

Previously, if |m_bpc| was < 8 (e.g. 4), this function may still try to
access the source components as if |m_bpc| == 8. Even when it fell into
the codepath that tried to do the right thing in this case, it was
wrong.

BUG=554151

Conflicts in BUILD.gn, pdfium.gyp, and fpdf_render_loadimage.cpp

Committed:
https://pdfium.googlesource.com/pdfium/+/9b99615806e358fdb396d1cb162ee2e69c2a...

Review URL: https://codereview.chromium.org/1433423002 .

(cherry picked from commit e21fe98d5b5da7da01503b985b07b90c8e811689)
==========

to

==========
Merge to M47: Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

Previously, if |m_bpc| was < 8 (e.g. 4), this function may still try to
access the source components as if |m_bpc| == 8. Even when it fell into
the codepath that tried to do the right thing in this case, it was
wrong.

BUG=554151

Committed:
https://pdfium.googlesource.com/pdfium/+/9b99615806e358fdb396d1cb162ee2e69c2a...

Review URL: https://codereview.chromium.org/1433423002 .

(cherry picked from commit e21fe98d5b5da7da01503b985b07b90c8e811689)
==========

[Oliver Chang, 2015-11-13 17:24:02]

ochang@chromium.org changed reviewers:
+ thestig@chromium.org

[Oliver Chang, 2015-11-13 17:24:03]

ptal. had some conflicts (see comments).

https://codereview.chromium.org/1441973002/diff/20001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1441973002/diff/20001/BUILD.gn#newcode142
BUILD.gn:142: 
another weird change picked up by cherry-pick:

++<<<<<<< HEAD
++=======
+ source_set("test_support") {
+   testonly = true
+   sources = [
+     "testing/fx_string_testhelpers.cpp",
+     "testing/fx_string_testhelpers.h",
+     "testing/test_support.cpp",
+     "testing/test_support.h",
+     'testing/utils/path_service.cpp',
+   ]
+   deps = [
+     "//testing/gmock",
+     "//testing/gtest",
+     ":pdfium",
+   ]
+   include_dirs = []
+   if (pdf_enable_v8) {
+     deps += [
+       "//v8",
+       "//v8:v8_libplatform",
+     ]
+     include_dirs += [
+       "//v8",
+       "//v8/include",
+     ]
+   }
+   configs += [ ":pdfium_config" ]
+ }
+ 
++>>>>>>> e21fe98... (Reland) Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

https://codereview.chromium.org/1441973002/diff/20001/BUILD.gn#newcode778
BUILD.gn:778:
"core/src/fpdfapi/fpdf_render/fpdf_render_loadimage_embeddertest.cpp",
++<<<<<<< HEAD
++=======
+     "core/src/fpdfapi/fpdf_render/fpdf_render_load_image_embeddertest.cpp",
+     "core/src/fpdfapi/fpdf_render/fpdf_render_pattern_embeddertest.cpp",
++>>>>>>> e21fe98... (Reland) Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

https://codereview.chromium.org/1441973002/diff/20001/core/src/fpdfapi/fpdf_r...
File core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp (right):

https://codereview.chromium.org/1441973002/diff/20001/core/src/fpdfapi/fpdf_r...
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp:1480: for (FX_DWORD j =
0; j < m_nComponents; ++j) {
++<<<<<<< HEAD
 +          for (int j = 0; j < m_nComponents; ++j) {
++=======
+           for (FX_DWORD j = 0; j < m_nComponents; ++j) {
+             FX_FLOAT component_value =
+                 static_cast<FX_FLOAT>(extracted_components[j]);
++>>>>>>> e21fe98... (Reland) Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

https://codereview.chromium.org/1441973002/diff/20001/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/1441973002/diff/20001/pdfium.gyp#newcode760
pdfium.gyp:760:
'core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_embeddertest.cpp',
++<<<<<<< HEAD
++=======
+         'core/src/fpdfapi/fpdf_render/fpdf_render_loadimage_embeddertest.cpp',
+         'core/src/fpdfapi/fpdf_render/fpdf_render_pattern_embeddertest.cpp',
++>>>>>>> e21fe98... (Reland) Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

[Lei Zhang, 2015-11-13 21:41:20]

If you are confident in the fix, don't bother checking in the tests. We don't
run them anyway.

[Lei Zhang, 2015-11-13 21:42:43]

i.e. if you cut this down to just
core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp, that LGTM.

[Lei Zhang, 2015-11-13 21:43:42]

Description was changed from

==========
Merge to M47: Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

Previously, if |m_bpc| was < 8 (e.g. 4), this function may still try to
access the source components as if |m_bpc| == 8. Even when it fell into
the codepath that tried to do the right thing in this case, it was
wrong.

BUG=554151

Committed:
https://pdfium.googlesource.com/pdfium/+/9b99615806e358fdb396d1cb162ee2e69c2a...

Review URL: https://codereview.chromium.org/1433423002 .

(cherry picked from commit e21fe98d5b5da7da01503b985b07b90c8e811689)
==========

to

==========
Merge to M47: Fix extraction of colour components in
CPDF_DIBSource::DownSampleScanline32Bit

Previously, if |m_bpc| was < 8 (e.g. 4), this function may still try to
access the source components as if |m_bpc| == 8. Even when it fell into
the codepath that tried to do the right thing in this case, it was
wrong.

BUG=554151

Review URL: https://codereview.chromium.org/1433423002 .

(cherry picked from commit e21fe98d5b5da7da01503b985b07b90c8e811689)
==========

[Lei Zhang, 2015-11-13 21:44:02]

I also removed the "Committed:" line from the CL description. It was confusing.

[Oliver Chang, 2015-11-13 22:01:58]

On 2015/11/13 21:44:02, Lei Zhang wrote:
> I also removed the "Committed:" line from the CL description. It was
confusing.

Thanks for the review! I've cut out the tests.

What's the testing/QA process like for PDFium in stable/beta branches? (This
should be fairly safe)

[Lei Zhang, 2015-11-13 22:06:34]

On 2015/11/13 22:01:58, Oliver Chang wrote:
> On 2015/11/13 21:44:02, Lei Zhang wrote:
> > I also removed the "Committed:" line from the CL description. It was
> confusing.
> 
> Thanks for the review! I've cut out the tests.
> 
> What's the testing/QA process like for PDFium in stable/beta branches? (This
> should be fairly safe)

Mostly manual and whatever PDF tests Chromium has. We should consider running
some PDFium tests on Chromium bots, but that's for a separate discussion.

[Oliver Chang, 2015-11-14 00:22:19]

Committed patchset #4 (id:60001) manually as
4ec1953b4ace8248ac443effafa5f651e540d995 (presubmit successful).