Added script optimize-ico-files.

Added script optimize-ico-files.

This is like the existing optimize-png-files script, but works on ICO
files. It extracts the PNGs out of an ICO file, crushes them using
optimize-png-files, and rebuilds the ICO.

BUG=332639
Committed: https://crrev.com/580c7f1ad8a1d229b7309f99ae3a6a2fd07b3dcc
Cr-Commit-Position: refs/heads/master@{#343854}

[Matt Giuca, 2015-08-10 05:49:10]

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

[Matt Giuca, 2015-08-10 05:49:10]

Oops, I uploaded this 19 months ago and forgot to mail it out ¯_(ツ)_/¯

Did a big cleanup, now I'm happy with the code.

[oshima, 2015-08-18 01:41:26]

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
File tools/resources/optimize-ico-files.py (right):

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:32: """Represents an invalid ICO file."""
2 indent

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:34: def is_png(png_data):
looks like google style guide recommends underscore, while many chromium python
scripts including this directory use pascal case. Let's be consistent.

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:51: stdout=sys.stderr)
pass optimization level. you can just pass -o2, or add an option to this script.

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:128: size = len(icon_data)
new_size ? (just to be clear)

[Matt Giuca, 2015-08-18 04:43:45]

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
File tools/resources/optimize-ico-files.py (right):

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:32: """Represents an invalid ICO file."""
On 2015/08/18 01:41:26, oshima wrote:
> 2 indent

https://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Indentation says
4 spaces.

As below, maybe it's better to use 2-space for consistency with the rest of the
Chromium scripts. I thought the consistency rule was per-file, and if you're
starting a new script, you should follow the style guide.

I can change it, up to you.

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:34: def is_png(png_data):
On 2015/08/18 01:41:26, oshima wrote:
> looks like google style guide recommends underscore, while many chromium
python
> scripts including this directory use pascal case. Let's be consistent.

See previous comment.

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:51: stdout=sys.stderr)
On 2015/08/18 01:41:26, oshima wrote:
> pass optimization level. you can just pass -o2, or add an option to this
script.

I tested this and found that -o2 took a LOT longer (maybe 10x) and made exactly
no difference (for the small icons I tested). So I definitely don't want to make
it the default. I'll make it an option.

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:128: size = len(icon_data)
On 2015/08/18 01:41:26, oshima wrote:
> new_size ? (just to be clear)

Done.

[Matt Giuca, 2015-08-18 04:50:42]

Added instructions to README on preparing icons.

[oshima, 2015-08-18 06:36:35]

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
File tools/resources/optimize-ico-files.py (right):

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:32: """Represents an invalid ICO file."""
On 2015/08/18 04:43:45, Matt Giuca wrote:
> On 2015/08/18 01:41:26, oshima wrote:
> > 2 indent
> 
> https://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Indentation
says
> 4 spaces.
> 
> As below, maybe it's better to use 2-space for consistency with the rest of
the
> Chromium scripts. I thought the consistency rule was per-file, and if you're
> starting a new script, you should follow the style guide.
> 
> I can change it, up to you.

Actually, https://www.chromium.org/chromium-os/python-style-guidelines says we
should use 2 indent and PascalCase.

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:51: stdout=sys.stderr)
On 2015/08/18 04:43:45, Matt Giuca wrote:
> On 2015/08/18 01:41:26, oshima wrote:
> > pass optimization level. you can just pass -o2, or add an option to this
> script.
> 
> I tested this and found that -o2 took a LOT longer (maybe 10x) and made
exactly
> no difference (for the small icons I tested). So I definitely don't want to
make
> it the default. I'll make it an option.

Yep, -o2 is more for batch operation so option is file.

[Matt Giuca, 2015-08-18 07:26:22]

Note: The changes to the README file also.

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
File tools/resources/optimize-ico-files.py (right):

https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
tools/resources/optimize-ico-files.py:32: """Represents an invalid ICO file."""
On 2015/08/18 06:36:34, oshima wrote:
> On 2015/08/18 04:43:45, Matt Giuca wrote:
> > On 2015/08/18 01:41:26, oshima wrote:
> > > 2 indent
> > 
> > https://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Indentation
> says
> > 4 spaces.
> > 
> > As below, maybe it's better to use 2-space for consistency with the rest of
> the
> > Chromium scripts. I thought the consistency rule was per-file, and if you're
> > starting a new script, you should follow the style guide.
> > 
> > I can change it, up to you.
> 
> Actually, https://www.chromium.org/chromium-os/python-style-guidelines says we
> should use 2 indent and PascalCase. 

Hmm, that's a strange place to have the policy! (It's in ChromiumOS, not
Chromium.) I couldn't find it when I looked.

OK, updated to 2-space indents and TitleCase.

[oshima, 2015-08-18 07:48:05]

lgtm

On 2015/08/18 07:26:22, Matt Giuca wrote:
> Note: The changes to the README file also.
> 
>
https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
> File tools/resources/optimize-ico-files.py (right):
> 
>
https://codereview.chromium.org/130583003/diff/110001/tools/resources/optimiz...
> tools/resources/optimize-ico-files.py:32: """Represents an invalid ICO
file."""
> On 2015/08/18 06:36:34, oshima wrote:
> > On 2015/08/18 04:43:45, Matt Giuca wrote:
> > > On 2015/08/18 01:41:26, oshima wrote:
> > > > 2 indent
> > > 
> > >
https://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Indentation
> > says
> > > 4 spaces.
> > > 
> > > As below, maybe it's better to use 2-space for consistency with the rest
of
> > the
> > > Chromium scripts. I thought the consistency rule was per-file, and if
you're
> > > starting a new script, you should follow the style guide.
> > > 
> > > I can change it, up to you.
> > 
> > Actually, https://www.chromium.org/chromium-os/python-style-guidelines says
we
> > should use 2 indent and PascalCase. 
> 
> Hmm, that's a strange place to have the policy! (It's in ChromiumOS, not
> Chromium.) I couldn't find it when I looked.

Oh, maybe that's really cros specific. If that's the case, i'm fine with either
way.
Sorry for confusion.

> 
> OK, updated to 2-space indents and TitleCase.

[Matt Giuca, 2015-08-18 07:55:25]

On 2015/08/18 07:48:05, oshima wrote:
> Oh, maybe that's really cros specific. If that's the case, i'm fine with
either
> way.
> Sorry for confusion.

Well, judging by the other .py scripts in Chromium, that is the de-facto style
if not the official style, so I'll leave it at 2-space.

[Matt Giuca, 2015-08-18 07:55:31]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2015-08-18 07:55:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/130583003/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/130583003/190001

[commit-bot: I haz the power, 2015-08-18 08:03:18]

Committed patchset #9 (id:190001)

[commit-bot: I haz the power, 2015-08-18 08:04:01]

Patchset 9 (id:??) landed as
https://crrev.com/580c7f1ad8a1d229b7309f99ae3a6a2fd07b3dcc
Cr-Commit-Position: refs/heads/master@{#343854}