Compact JS resource better by using rjsmin.

Compact JS resource better by using rjsmin.

We have a jsminimizer (not very good but we have it) in blink so we
should use it.

BUG=359583
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=172748

[Daniel Bratell, 2014-04-25 09:49:38]

abarth, this is a followup from https://codereview.chromium.org/214003005/ where
I posted an improved minimizer in make-file-arrays. As was concluded in that
review was that long-term we want to move to grd and nobody wanted to review
minimizer code.

So this is a short-term fix to reuse the devtools minimizer for make-file-array
as well. The gain is about 7 KB (less than in the other review but not enough to
make a fuzz).

I had to slightly reformat rjsmin to be able to move it and get it through the
presubmitter but it's really just a moved file.

https://codereview.chromium.org/252633006/diff/1/Source/build/scripts/rjsmin.py
File Source/build/scripts/rjsmin.py (right):

https://codereview.chromium.org/252633006/diff/1/Source/build/scripts/rjsmin....
Source/build/scripts/rjsmin.py:93: xrange = range  # pylint: disable = W0622
The changes I've done in this file is just to get it through the presubmitter.

[abarth-chromium, 2014-04-25 16:13:31]

Why not just move these resource to GRD?  I'm not sure why we need a short-term
solution here instead of just making the system actually work the way we want it
to work.

[Daniel Bratell, 2014-04-25 17:05:07]

On 2014/04/25 16:13:31, abarth wrote:
> Why not just move these resource to GRD?  I'm not sure why we need a
short-term
> solution here instead of just making the system actually work the way we want
it
> to work.

Basically because this patch exists. It removes one js minimizer from the code
base so even if it's not what we wish for you could argue it moves things in the
right direction. I do plan to look more at the grd stuff in the future but I
have a too long to-do list with stuff constantly being inserted at the top so I
have absolutely no idea when I will get there.

[abarth-chromium, 2014-04-25 22:16:57]

Ok.  LGTM.  I guess this CL is a step forward in that it at least replaces a
home-grown JS minifier with a library.  The cost is a bunch of ugly code to
change the import paths.  I'm not super excited about the CL, but I think it's
worth landing.

[pfeldman, 2014-04-26 05:02:19]

To avoid the confusion: it does not replace home-grown minifier with the
library, it just moves the library from one place to another and starts using it
when embedding resources as headers in core.

We still want grd, but given the size of the patch, the fact that it does the
job and saves bytes, it is a reasonable thing to land.

[abarth-chromium, 2014-04-26 17:08:35]

https://codereview.chromium.org/252633006/diff/1/Source/web/scripts/make-file...
File Source/web/scripts/make-file-arrays.py (left):

https://codereview.chromium.org/252633006/diff/1/Source/web/scripts/make-file...
Source/web/scripts/make-file-arrays.py:70: content = empty_line.sub('\n',
content)
It replaces this one, but may this one is too simple to properly call a
minifier.  :)

[Daniel Bratell, 2014-04-28 09:53:14]

On 2014/04/26 17:08:35, abarth wrote:
>
https://codereview.chromium.org/252633006/diff/1/Source/web/scripts/make-file...
> File Source/web/scripts/make-file-arrays.py (left):
> 
>
https://codereview.chromium.org/252633006/diff/1/Source/web/scripts/make-file...
> Source/web/scripts/make-file-arrays.py:70: content = empty_line.sub('\n',
> content)
> It replaces this one, but may this one is too simple to properly call a
> minifier.  :)

The old code is simple (just a few regexps) but the new code (rjsmin) is
actually only a few regexps, though larger. They have a big effect though. I
think they strip about 30% of the scripts away, mostly comments and repeated
whitespace.

But thanks for the review! I'm off to learn grit next, but don't hold your
breath please.

[Daniel Bratell, 2014-04-28 09:53:21]

The CQ bit was checked by bratell@opera.com

[commit-bot: I haz the power, 2014-04-28 09:53:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bratell@opera.com/252633006/1

[commit-bot: I haz the power, 2014-04-28 11:05:37]

Change committed as 172748