Remove opaque alpha channel special case

third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp

Index: third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp b/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
index 4426dfd7af5dff491a2e3b93404f2ee0c944a1eb..eb6c8ee85adc5e68667b8b6817e6c3551bc891ab 100644
--- a/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
@@ -146,7 +146,7 @@ bool BMPImageReader::DecodeBMP(bool only_size) {
   // If the image has an AND mask and there was no alpha data, process the
   // mask.
   if (is_in_ico_ && !decoding_and_mask_ &&
-      ((info_header_.bi_bit_count < 16) || !bit_masks_[3] ||
+      ((info_header_.bi_bit_count < 16) || IsAlphaSupported() ||

[scroggo_chromium, 2017/05/30 19:45:17]

Now that you flipped IsAlphaSupported() back, don't you want !IsAlphaSupported()
here?

[cblume, 2017/05/30 23:45:14]

Done.

        !seen_non_zero_alpha_pixel_)) {
     // Reset decoding coordinates to start of image.
     coord_.SetX(0);
@@ -817,20 +817,20 @@ BMPImageReader::ProcessingResult BMPImageReader::ProcessNonRLEData(
         // until we actually see a non-zero alpha value; at that point,
         // reset any previously-decoded pixels to fully transparent and
         // continue decoding based on the real alpha channel values.
-        // As an optimization, avoid calling SetHasAlpha(true) for
-        // images where all alpha values are 255; opaque images are
-        // faster to draw.
-        int alpha = GetAlpha(pixel);
-        if (!seen_non_zero_alpha_pixel_ && !alpha) {
-          seen_zero_alpha_pixel_ = true;
-          alpha = 255;
-        } else {
-          seen_non_zero_alpha_pixel_ = true;
-          if (seen_zero_alpha_pixel_) {
-            buffer_->ZeroFillPixelData();
-            seen_zero_alpha_pixel_ = false;
-          } else if (alpha != 255)
-            buffer_->SetHasAlpha(true);
+        int alpha = 255;
+        seen_zero_alpha_pixel_ = true;

[scroggo_chromium, 2017/05/30 19:45:17]

Now this always gets set to true before we read it. I don't see how this could
be the intended behavior.

(I already brought this up in
https://codereview.chromium.org/2756463003/diff/160001/third_party/WebKit/Sou...)

[cblume, 2017/05/30 23:37:34]

Done.

+        if (IsAlphaSupported()) {
+          alpha = GetAlpha(pixel);
+          if (!seen_non_zero_alpha_pixel_ && !alpha) {
+            alpha = 255;
+          } else {
+            if (seen_zero_alpha_pixel_) {

[scroggo_chromium, 2017/05/30 19:45:17]

This will always be true, since we always set it to true on line 821.

[cblume, 2017/05/30 23:37:34]

Done.

+              buffer_->ZeroFillPixelData();
+              seen_zero_alpha_pixel_ = false;

[scroggo_chromium, 2017/05/30 19:45:17]

No one ever reads this value, since we set it to true again on line 821 before
we ever read it.

[cblume, 2017/05/30 23:37:34]

Done.

+            } else {

[scroggo_chromium, 2017/05/30 19:45:17]

We will never hit this branch.

[cblume, 2017/05/30 23:37:34]

Done.

+              buffer_->SetHasAlpha(true);
+            }
+          }
         }
 
         SetRGBA(GetComponent(pixel, 0), GetComponent(pixel, 1),