Compact JS resource better.

Compact JS resource better.

Some code written in JavaScript used about 100KB of binary size. Part of
it was unnecessary whitespace so this will compact whitespace harder than
before.

BUG=312586
BUG=359583

[Daniel Bratell, 2014-04-03 15:06:15]

This is a stab at making the binary slightly smaller (10 KB or so) by removing
whitespace in embedded javascript programs.

I would prefer to use something like Closure but as I wrote in the bug, it's not
that easy. And I couldn't find any other system/library that could be used
either. There were a few but they were either in the wrong language or just not
useful.

abarth, I'm asking you to review since few people have touched this code.

I'm open to other suggestions.

[abarth-chromium, 2014-04-03 15:53:26]

Can we move these resources out of the binary into the resource bundle? 
Chromium has a system for storing resources based on GRD files.  One feature of
that system is that it knows how to minify scripts, etc.

Rather than building a new JS minifier, it seems better to integrate with the
rest of the system and improve it globally for the product.

[abarth-chromium, 2014-04-03 15:53:47]

lgtm

[commit-bot: I haz the power, 2014-04-03 15:53:51]

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

[abarth-chromium, 2014-04-03 15:54:11]

Sorry, not lgtm.  I didn't mean to click that button :(

[abarth-chromium, 2014-04-03 15:54:14]

The CQ bit was unchecked by abarth@chromium.org

[abarth-chromium, 2014-04-03 15:55:01]

Oh man, I've really made a mess of your CL.  Sorry.  :(

I'm not really opposed to this change, I just think we can do better.

I added pdr because I think he's looked at moving some resources into the GRD
system before.

[Daniel Bratell, 2014-04-03 17:59:33]

On 2014/04/03 15:55:01, abarth wrote:
> Oh man, I've really made a mess of your CL.  Sorry.  :(
> 
> I'm not really opposed to this change, I just think we can do better.
> 
> I added pdr because I think he's looked at moving some resources into the GRD
> system before.

Yes, that might be much better. Is that where the devtools are? I've wondered
why they didn't show up.

I've tried this out in Opera and it seems to work but it's more of a itchy thing
than a huge win.

[pdr., 2014-04-03 19:58:44]

On 2014/04/03 17:59:33, Daniel Bratell wrote:
> On 2014/04/03 15:55:01, abarth wrote:
> > Oh man, I've really made a mess of your CL.  Sorry.  :(
> > 
> > I'm not really opposed to this change, I just think we can do better.
> > 
> > I added pdr because I think he's looked at moving some resources into the
GRD
> > system before.
> 
> Yes, that might be much better. Is that where the devtools are? I've wondered
> why they didn't show up.
> 
> I've tried this out in Opera and it seems to work but it's more of a itchy
thing
> than a huge win.

I've written a similar patch (https://codereview.chromium.org/107713007/) and
found it only saved a fraction of a percent. I don't think this patch is bad
implementation-wise, but I think it's optimizing at the wrong level, and adding
a decent amount of complexity tax. We should be loading this data (ideally in a
compressed form) from somewhere other than c++ arrays :)

[Daniel Bratell, 2014-04-03 21:10:39]

On 2014/04/03 19:58:44, pdr wrote:
> On 2014/04/03 17:59:33, Daniel Bratell wrote:
> > On 2014/04/03 15:55:01, abarth wrote:
> > > Oh man, I've really made a mess of your CL.  Sorry.  :(
> > > 
> > > I'm not really opposed to this change, I just think we can do better.
> > > 
> > > I added pdr because I think he's looked at moving some resources into the
> GRD
> > > system before.
> > 
> > Yes, that might be much better. Is that where the devtools are? I've
wondered
> > why they didn't show up.
> > 
> > I've tried this out in Opera and it seems to work but it's more of a itchy
> thing
> > than a huge win.
> 
> I've written a similar patch (https://codereview.chromium.org/107713007/) and
> found it only saved a fraction of a percent. I don't think this patch is bad
> implementation-wise, but I think it's optimizing at the wrong level, and
adding
> a decent amount of complexity tax. We should be loading this data (ideally in
a
> compressed form) from somewhere other than c++ arrays :)

I think it's absolutely the right thing to do to compress js resources into the
most optimal form so I don't doubt for a second that it ought to be done. The
question is more one of "where" and "how" and "who" and "when". As for saving a
fraction of a percent, I think that is what it will take. My view of Chromium is
that the whole project is generally spacey but it will have to be addressed 100
KB at a time because the spaciness was added 100 KB at a time.

There seems to be roughly 2-300 KB whitespace in the embedded JavaScript
programs (interpolated from how much this patch strips away) so that us one of
the more obvious things to address. But maybe this discussion should be in the
bug rather than in the code review.

[pdr., 2014-04-03 21:24:19]

On 2014/04/03 21:10:39, Daniel Bratell wrote:
> On 2014/04/03 19:58:44, pdr wrote:
> > On 2014/04/03 17:59:33, Daniel Bratell wrote:
> > > On 2014/04/03 15:55:01, abarth wrote:
> > > > Oh man, I've really made a mess of your CL.  Sorry.  :(
> > > > 
> > > > I'm not really opposed to this change, I just think we can do better.
> > > > 
> > > > I added pdr because I think he's looked at moving some resources into
the
> > GRD
> > > > system before.
> > > 
> > > Yes, that might be much better. Is that where the devtools are? I've
> wondered
> > > why they didn't show up.
> > > 
> > > I've tried this out in Opera and it seems to work but it's more of a itchy
> > thing
> > > than a huge win.
> > 
> > I've written a similar patch (https://codereview.chromium.org/107713007/)
and
> > found it only saved a fraction of a percent. I don't think this patch is bad
> > implementation-wise, but I think it's optimizing at the wrong level, and
> adding
> > a decent amount of complexity tax. We should be loading this data (ideally
in
> a
> > compressed form) from somewhere other than c++ arrays :)
> 
> I think it's absolutely the right thing to do to compress js resources into
the
> most optimal form so I don't doubt for a second that it ought to be done. The
> question is more one of "where" and "how" and "who" and "when". As for saving
a
> fraction of a percent, I think that is what it will take. My view of Chromium
is
> that the whole project is generally spacey but it will have to be addressed
100
> KB at a time because the spaciness was added 100 KB at a time.
> 
> There seems to be roughly 2-300 KB whitespace in the embedded JavaScript
> programs (interpolated from how much this patch strips away) so that us one of
> the more obvious things to address. But maybe this discussion should be in the
> bug rather than in the code review.

My understanding is that we already have this technology as part of the GRD
system which is used for chromium's strings. Can you investigate whether that is
feasible for this data too?

[Nils Barth (inactive), 2014-04-04 02:23:29]

Hi Daniel,
As Adam and Philip note, we'd prefer to do this by using
GRD resources, which should work better and reduce complexity.
The bug for that is:
Replace embedding resources by ad hoc Python with GRD resources
https://code.google.com/p/chromium/issues/detail?id=312586

If you're interested, there's some grit documentation here:
https://code.google.com/p/grit-i18n/w/list
...especially the GritUsersGuide:
https://code.google.com/p/grit-i18n/wiki/GritUsersGuide

Thanks for looking at this!
(We've been wanting to fix this for a while, but haven't had
anyone particularly gung-ho about it.)

[Daniel Bratell, 2014-04-25 09:51:31]

On 2014/04/04 02:23:29, Nils Barth wrote:
> Hi Daniel,
> As Adam and Philip note, we'd prefer to do this by using
> GRD resources, which should work better and reduce complexity.
> The bug for that is:
> Replace embedding resources by ad hoc Python with GRD resources
> https://code.google.com/p/chromium/issues/detail?id=312586
> 
> If you're interested, there's some grit documentation here:
> https://code.google.com/p/grit-i18n/w/list
> ...especially the GritUsersGuide:
> https://code.google.com/p/grit-i18n/wiki/GritUsersGuide
> 
> Thanks for looking at this!
> (We've been wanting to fix this for a while, but haven't had
> anyone particularly gung-ho about it.)

Converting to grd doesn't seem straight forward. And the documentation is mostly
about how to store translatable strings, not javascript programs that need
minimizing.

As a short-term solution I've put a fix in
https://codereview.chromium.org/252633006/ to use rjsmin (as used in devtools)
in make-file-arrays as well. If that is accepted I will close this one.

[abarth-chromium, 2014-04-25 16:14:38]

On 2014/04/25 09:51:31, Daniel Bratell wrote:
> Converting to grd doesn't seem straight forward. And the documentation is
mostly
> about how to store translatable strings, not javascript programs that need
> minimizing.

We use GRD for lots of JavaScript resources. If GRD doesn't understand
minimization, we should teach it.  That will benefit the entire project whereas
rolling a Blink-only solution just makes it harder to do the right thing in the
end.