optimize-ico-files: Automatically convert large BMPs to PNG format.

optimize-ico-files: Automatically convert large BMPs to PNG format.

This ensures that we don't have large uncompressed images in .ico files
which could be stored as PNG. Only compresses large (>=256x256) images
(smaller images must be stored uncompressed for compatibility with
Windows XP).

Images are converted to PNG and then crushed.

BUG=332639
Committed: https://crrev.com/0d459808502b877cd4e9069e1ecb70e28f8a2438
Cr-Commit-Position: refs/heads/master@{#430442}

[Matt Giuca, 2015-08-10 06:57:05]

mgiuca@chromium.org changed reviewers:
+ oshima@chromium.org

[Matt Giuca, 2015-08-10 06:57:05]

Follow-up to https://codereview.chromium.org/130583003.

[Matt Giuca, 2015-08-18 08:34:57]

The precursor CL has landed; PTAL.

Hmm... so this enhancement is, I suppose, not strictly necessary if you follow
the steps I outlined in README.

Unfortunately, ImageMagick makes a mistake converting 8-bit PNGs to BMP/ICO (it
converts them to 32-bit, and I don't know of a way to tell it not to), so I was
forced to use GIMP to convert them back again. In doing so, GIMP automatically
converts the 256x256 image to PNG format. So if you follow my steps, you don't
need optimize-ico-files to perform this PNG conversion, since GIMP does it for
you.

Still, it's nice to have it in here. I'll leave it up to you if you want to
accept this additional feature, or just reject it.

[Matt Giuca, 2015-08-24 00:38:42]

Hi oshima, friendly ping?

[oshima, 2015-10-12 20:34:10]

lgtm

[Matt Giuca, 2016-11-07 05:03:40]

mgiuca@chromium.org changed reviewers:
+ scottmg@chromium.org

[Matt Giuca, 2016-11-07 05:03:41]

+scottmg: Hi, I just found this old CL and I honestly have no idea why I never
landed it.

It seems Oshima is no longer in the OWNERS file so adding you for OWNERS. I
retested it and everything seems fine.

Thanks!

[Matt Giuca, 2016-11-07 05:03:45]

The CQ bit was checked by mgiuca@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-07 05:03:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-07 05:16:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-07 05:16:14]

Dry run: This issue passed the CQ dry run.

[scottmg, 2016-11-07 17:45:35]

https://codereview.chromium.org/1272763003/diff/80001/tools/resources/ico_too...
File tools/resources/ico_tools.py (right):

https://codereview.chromium.org/1272763003/diff/80001/tools/resources/ico_too...
tools/resources/ico_tools.py:257: # with Windows XP.
If this is really the only reason for this restriction, we could remove it now?

[Matt Giuca, 2016-11-08 00:06:31]

https://codereview.chromium.org/1272763003/diff/80001/tools/resources/ico_too...
File tools/resources/ico_tools.py (right):

https://codereview.chromium.org/1272763003/diff/80001/tools/resources/ico_too...
tools/resources/ico_tools.py:257: # with Windows XP.
On 2016/11/07 17:45:35, scottmg wrote:
> If this is really the only reason for this restriction, we could remove it
now?

That's a great suggestion. I think so, but I'd have to do some research and
testing to make sure. (It's a bit scary to remove the uncompressed icons since
they are the lowest common denominator of Windows icons. For example,
compressing all the icons makes them unreadable with the Linux browser; not that
that really matters but it's an example of things that can break.) I've filed a
bug for it (https://crbug.com/663136) but I think we should land this change
first.

[scottmg, 2016-11-08 00:09:37]

OK, lgtm

[Matt Giuca, 2016-11-08 00:17:53]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-11-08 00:17:54]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/1272763003/#ps100001
(title: "Added TODO to compress all the images (no longer need XP).")

[commit-bot: I haz the power, 2016-11-08 00:18:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-08 00:35:50]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-11-08 00:52:42]

Description was changed from

==========
optimize-ico-files: Automatically convert large BMPs to PNG format.

This ensures that we don't have large uncompressed images in .ico files
which could be stored as PNG. Only compresses large (>=256x256) images
(smaller images must be stored uncompressed for compatibility with
Windows XP).

Images are converted to PNG and then crushed.

BUG=332639
==========

to

==========
optimize-ico-files: Automatically convert large BMPs to PNG format.

This ensures that we don't have large uncompressed images in .ico files
which could be stored as PNG. Only compresses large (>=256x256) images
(smaller images must be stored uncompressed for compatibility with
Windows XP).

Images are converted to PNG and then crushed.

BUG=332639

Committed: https://crrev.com/0d459808502b877cd4e9069e1ecb70e28f8a2438
Cr-Commit-Position: refs/heads/master@{#430442}
==========

[commit-bot: I haz the power, 2016-11-08 00:52:43]

Patchset 6 (id:??) landed as
https://crrev.com/0d459808502b877cd4e9069e1ecb70e28f8a2438
Cr-Commit-Position: refs/heads/master@{#430442}