Add a PRESUBMIT check for broken .ico files.

tools/resources/ico_tools.py

Index: tools/resources/ico_tools.py
diff --git a/tools/resources/ico_tools.py b/tools/resources/ico_tools.py
index 9c3128b56a7e0d422a0ab6565b38f0cc8dfad6d0..db0cda11853c200926737890d2921f6b9ac627ee 100644
--- a/tools/resources/ico_tools.py
+++ b/tools/resources/ico_tools.py
@@ -70,6 +70,11 @@ def OptimizePng(png_data, optimization_level=None):
       os.unlink(png_filename)
     os.rmdir(temp_dir)
 
+def BytesPerRowBMP(width, bpp):
+  """Computes the number of bytes per row in a Windows BMP image."""
+  # width * bpp / 8, rounded up to the nearest multiple of 4.
+  return int(math.ceil(width * bpp / 32.0)) * 4
+
 def ExportSingleEntry(icon_dir_entry, icon_data, outfile):
   """Export a single icon dir entry to its own ICO file.
 
@@ -171,11 +176,46 @@ def ComputeANDMaskFromAlpha(image_data, width, height):
   and_bytes = ''.join(map(chr, and_bytes))
   return and_bytes
 
-def RebuildANDMask(iconimage):
-  """Rebuild the AND mask in an icon image.
+def CheckANDMaskAgainstAlpha(xor_data, and_data, width, height):
+  """Checks whether an AND mask is "good" for 32-bit BGRA image data.
+
+  This checks that the mask is opaque wherever the alpha channel is not fully
+  transparent. Pixels that violate this condition will show up as black in some
+  contexts in Windows (http://crbug.com/526622). Also checks the inverse
+  condition, that the mask is transparent wherever the alpha channel is fully
+  transparent. While this does not appear to be strictly necessary, it is good
+  practice for backwards compatibility.
+
+  Returns True if the AND mask is "good", False otherwise.
+  """
+  xor_bytes_per_row = width * 4
+  and_bytes_per_row = BytesPerRowBMP(width, 1)
+
+  for y in range(height):
+    for x in range(width):
+      alpha = ord(xor_data[y * xor_bytes_per_row + x * 4 + 3])
+      mask = bool(ord(and_data[y * and_bytes_per_row + x // 8]) &
+                  (1 << (7 - (x % 8))))
+
+      if mask:
+        if alpha > 0:
+          # mask is transparent, alpha is partially or fully opaque. This pixel
+          # can show up as black on Windows due to a rendering bug.
+          return False
+      else:
+        if alpha == 0:
+          # mask is opaque, alpha is transparent. This pixel should be marked as
+          # transparent in the mask, for legacy reasons.
+          return False
+
+  return True
+
+def CheckOrRebuildANDMask(iconimage, rebuild=False):
+  """Checks the AND mask in an icon image for correctness, or rebuilds it.
 
   GIMP (<=2.8.14) creates a bad AND mask on 32-bit icon images (pixels with <50%
-  opacity are marked as transparent, which end up looking black on Windows). So,
+  opacity are marked as transparent, which end up looking black on Windows).
+  With rebuild == False, checks whether the mask is bad. With rebuild == True,
   if this is a 32-bit image, throw the mask away and recompute it from the alpha
   data. (See: https://bugzilla.gnome.org/show_bug.cgi?id=755200)
 
@@ -185,7 +225,8 @@ def RebuildANDMask(iconimage):
       is not 32-bit, this is a no-op).
 
   Returns:
-    An updated |iconimage|, with the AND mask re-computed using
+    If rebuild == False, a bool indicating whether the mask is "good". If
+    rebuild == True, an updated |iconimage|, with the AND mask re-computed using
     ComputeANDMaskFromAlpha.
   """
   # Parse BITMAPINFOHEADER.
@@ -195,20 +236,85 @@ def RebuildANDMask(iconimage):
   if bpp != 32:
     # No alpha channel, so the mask cannot be "wrong" (it is the only source of
     # transparency information).
-    return iconimage
+    return iconimage if rebuild else True
 
   height /= 2
-  xor_size = int(math.ceil(width * bpp / 32.0)) * 4 * height
+  xor_size = BytesPerRowBMP(width, bpp) * height
 
   # num_colors can be 0, implying 2^bpp colors.
   xor_palette_size = (num_colors or (1 << bpp if bpp < 24 else 0)) * 4
   xor_data = iconimage[40 + xor_palette_size :
                        40 + xor_palette_size + xor_size]
 
-  and_data = ComputeANDMaskFromAlpha(xor_data, width, height)
+  if rebuild:
+    and_data = ComputeANDMaskFromAlpha(xor_data, width, height)
+
+    # Replace the AND mask in the original icon data.
+    return iconimage[:40 + xor_palette_size + xor_size] + and_data
+  else:
+    and_data = iconimage[40 + xor_palette_size + xor_size:]
+    return CheckANDMaskAgainstAlpha(xor_data, and_data, width, height)
+
+def LintIcoFile(infile):
+  """Read an ICO file and check whether it is acceptable.
+
+  This checks for:
+  - Basic structural integrity of the ICO.
+  - Large BMPs that could be converted to PNGs.
+  - 32-bit BMPs with buggy AND masks.
+
+  It will *not* check whether PNG images have been compressed sufficiently.
+
+  Args:
+    infile: The file to read from. Must be a seekable file-like object
+      containing a Microsoft ICO file.
+
+  Returns:
+    A sequence of strings, containing error messages. An empty sequence
+    indicates a good icon.
+  """
+  filename = os.path.basename(infile.name)
+  icondir = infile.read(6)
+  zero, image_type, num_images = struct.unpack('<HHH', icondir)
+  if zero != 0:
+    yield 'Invalid ICO: First word must be 0.'
+    return
+
+  if image_type not in (1, 2):
+    yield 'Invalid ICO: Image type must be 1 or 2.'
+    return
+
+  # Read and unpack each ICONDIRENTRY.
+  icon_dir_entries = []
+  for i in range(num_images):
+    icondirentry = infile.read(16)
+    icon_dir_entries.append(struct.unpack('<BBBBHHLL', icondirentry))
+
+  # Read each icon's bitmap data.
+  current_offset = infile.tell()
+  icon_bitmap_data = []
+  for i in range(num_images):
+    width, height, num_colors, r1, r2, r3, size, _ = icon_dir_entries[i]
+    width = width or 256
+    height = height or 256
+    offset = current_offset
+    icon_data = infile.read(size)
+    if len(icon_data) != size:
+      yield 'Invalid ICO: Unexpected end of file'
+      return
+
+    entry_is_png = IsPng(icon_data)
+    logging.debug('%s entry #%d: %dx%d, %d bytes (%s)', filename, i + 1, width,
+                  height, size, 'PNG' if entry_is_png else 'BMP')
+
+    if not entry_is_png:
+      if width >= 256 or height >= 256:
+        yield ('Entry #%d is a large image in uncompressed BMP format. It '
+               'should be in PNG format.' % (i + 1))
 
-  # Replace the AND mask in the original icon data.
-  return iconimage[:40 + xor_palette_size + xor_size] + and_data
+      if not CheckOrRebuildANDMask(icon_data, rebuild=False):
+        yield ('Entry #%d has a bad mask that will display incorrectly in some '
+               'places in Windows.' % (i + 1))
 
 def OptimizeIcoFile(infile, outfile, optimization_level=None):
   """Read an ICO file, optimize its PNGs, and write the output to outfile.
@@ -259,7 +365,7 @@ def OptimizeIcoFile(infile, outfile, optimization_level=None):
       # all of the images. https://crbug.com/663136
       icon_data = OptimizeBmp(icon_dir_entries[i], icon_data)
     else:
-      new_icon_data = RebuildANDMask(icon_data)
+      new_icon_data = CheckOrRebuildANDMask(icon_data, rebuild=True)
       if new_icon_data != icon_data:
         logging.info('  * Rebuilt AND mask for this image from alpha channel.')
         icon_data = new_icon_data