Lay groudwork to Closure compile JavaScript

Lay groudwork to Closure compile JavaScript

This CL adds:

- third_party/closure_compiler/

  - compiler/ (latest version of closure compiler)
    - COPYING (copying instructions)
    - compiler.jar (closure compiler as a .jar)
    - README.md (how to use compiler)

  - runner/ (needed to run the compiler in IDE mode)
    - build_runner_jar.py (builds runner.jar)
    - runner.jar (a .jar to execute to compile)
    - src/org/chromium/closure/compiler/Runner.java (a compiler runner)

  - LICENSE (closure compiler's license)
  - OWNERS (reviewers)
  - README.chromium (description of new directory's contents)
  - checker.py (class for checking .js files with settings for chrome code)
  - compile_modules.py (parses modules with sources/dependencies/externs)

BUG=78368
R=darin@chromium.org,arv@chromium.org

[Dan Beam, 2014-07-11 01:11:59]

this rips a lot of code from:
third_party/WebKit/Source/devtools/scripts

I've talked with apavlov@ and vsevik@ and they're not totally convinced it's
worth sharing code yet, but I think eventually we'll be able to factor out some
common functionality.

p.s. if you want me to break off the ui/ changes, let me know.

https://chromiumcodereview.appspot.com/369643002/diff/210001/ui/webui/resourc...
File ui/webui/resources/js/compiled_resources.json (right):

https://chromiumcodereview.appspot.com/369643002/diff/210001/ui/webui/resourc...
ui/webui/resources/js/compiled_resources.json:9: ]
i'm still not sure how i'm going to model dependencies and whether there should
be 1 .json file with modules or multiple per-directory.

i'll probably just copy:
third_party/WebKit/Source/devtools/scripts/frontend_modules.json

[Dan Beam, 2014-07-11 01:29:02]

fyi: here's the design doc
https://docs.google.com/a/google.com/document/d/11BvLXypQDSSrcbllNlGLbBDhLNcV...

[Dan Beam, 2014-07-12 01:21:52]

fyi: the intern that will start helping me with this starts orientation on
Monday

[Dan Beam, 2014-07-14 17:27:48]

+arv/estade/tbreisacher as FYI

[darin (slow to review), 2014-07-15 05:06:31]

This looks reasonable to me, but I'm a bit out of my element. I don't have
enough familiarity with closure compiler to provide you with a meaningful
review. Deferring to arv@. I also think it would be good to send a note to
chromium-dev@ about this change to a) give folks a heads up that they can now
use closure compiler, and b) give folks a heads up so they can complain if you
should be doing something different ;-)

[arv (Not doing code reviews), 2014-07-16 18:33:04]

I've always been ambivalent about Closure Compiler. On one hand it catches
errors but on the other hand it makes you have to learn a new language and a
bunch of new quirks.

https://chromiumcodereview.appspot.com/369643002/diff/250001/third_party/clos...
File third_party/closure_compiler/checker.py (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/third_party/clos...
third_party/closure_compiler/checker.py:33:
"--jscomp_error=checkEventfulObjectDisposal",
We do not depend on dispose pattern. It is a bad pattern after all...

https://chromiumcodereview.appspot.com/369643002/diff/250001/third_party/clos...
third_party/closure_compiler/checker.py:39: "--jscomp_error=duplicateMessage",
We do not use the MSG system

https://chromiumcodereview.appspot.com/369643002/diff/250001/third_party/clos...
third_party/closure_compiler/checker.py:40: "--jscomp_error=es3",
?

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/chromeos/ui_account_tweaks.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/chromeos/ui_account_tweaks.js:129: /** @type {Element}
*/(node), sessionType);
Sad :'(

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/cr.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/cr.js:98: * @return {Function|undefined} The getter for
the property.
This never returns undefined. Maybe add a `throw 'unreachable'` instead?

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/cr.js:129: * @return {Function|undefined} The function to
use as a setter.
same

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/cr.js:191: var kind = /** @type {PropertyKind} */
(opt_kind || PropertyKind.JS);
so the @suppress didn't work?

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/event_tracker.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/event_tracker.js:85: },
useless change

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/i18n_template.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/i18n_template.js:111: var elements =
node.querySelectorAll(selector);
Node does not have querySelectorAll. Do you want to change it to {ParentNode}?

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/load_time_data.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/load_time_data.js:18: function LoadTimeData() {}
I hope we are stripping this code?

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/local_strings.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/local_strings.js:55: templateData: undefined,
why not null?

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/local_strings.js:64: var templateData = this.templateData
|| window['templateData'];
Sad :'(

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/parse_html_subset.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/parse_html_subset.js:79: /** @suppress {missingProperties}
*/
Why is this needed?

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/util.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/util.js:235: var anchor = findAncestor(/** @type {Element}
*/(e.target), function(el) {
e.target might be a Document too.

[apavlov, 2014-07-16 18:56:15]

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/chromeos/ui_account_tweaks.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/chromeos/ui_account_tweaks.js:129: /** @type {Element}
*/(node), sessionType);
On a side note, {Element} means "Element or null", so you should use {!Element}
and such, whenever it makes sense.

[Dan Beam, 2014-07-19 02:28:41]

https://chromiumcodereview.appspot.com/369643002/diff/250001/third_party/clos...
File third_party/closure_compiler/checker.py (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/third_party/clos...
third_party/closure_compiler/checker.py:33:
"--jscomp_error=checkEventfulObjectDisposal",
On 2014/07/16 18:33:02, arv wrote:
> We do not depend on dispose pattern. It is a bad pattern after all...

Done.

https://chromiumcodereview.appspot.com/369643002/diff/250001/third_party/clos...
third_party/closure_compiler/checker.py:39: "--jscomp_error=duplicateMessage",
On 2014/07/16 18:33:02, arv wrote:
> We do not use the MSG system

Done.

https://chromiumcodereview.appspot.com/369643002/diff/250001/third_party/clos...
third_party/closure_compiler/checker.py:40: "--jscomp_error=es3",
On 2014/07/16 18:33:02, arv wrote:
> ?

Done.

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/chromeos/ui_account_tweaks.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/chromeos/ui_account_tweaks.js:129: /** @type {Element}
*/(node), sessionType);
On 2014/07/16 18:56:14, apavlov wrote:
> On a side note, {Element} means "Element or null", so you should use
{!Element}
> and such, whenever it makes sense.

yep, thanks.

I just converted most of the code to use Node instead to make arv@ less :'(.

see https://chromiumcodereview.appspot.com/405743002/ for the split CL that does
this.

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/cr.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/cr.js:98: * @return {Function|undefined} The getter for
the property.
On 2014/07/16 18:33:02, arv wrote:
> This never returns undefined. Maybe add a `throw 'unreachable'` instead?

Done.

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/cr.js:129: * @return {Function|undefined} The function to
use as a setter.
On 2014/07/16 18:33:02, arv wrote:
> same

Done.

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/cr.js:191: var kind = /** @type {PropertyKind} */
(opt_kind || PropertyKind.JS);
On 2014/07/16 18:33:02, arv wrote:
> so the @suppress didn't work?

right, removed @suppress

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/event_tracker.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/event_tracker.js:85: },
On 2014/07/16 18:33:03, arv wrote:
> useless change

Done.

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/i18n_template.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/i18n_template.js:111: var elements =
node.querySelectorAll(selector);
On 2014/07/16 18:33:03, arv wrote:
> Node does not have querySelectorAll. Do you want to change it to {ParentNode}?

MDN says it's on Element...
https://developer.mozilla.org/en-US/docs/Web/API/Element.querySelectorAll

As long as it compiles I don't particularly care.  I don't think Node has
.classList either but it worked in the new CL:
https://chromiumcodereview.appspot.com/405743002/

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/load_time_data.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/load_time_data.js:18: function LoadTimeData() {}
On 2014/07/16 18:33:03, arv wrote:
> I hope we are stripping this code?

we're not using the output yet, so no

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/local_strings.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/local_strings.js:55: templateData: undefined,
On 2014/07/16 18:33:03, arv wrote:
> why not null?

because this is the actual type, see ctor function of LocalStrings(), could
change to:

  this.templateData = opt_templateData || null;

if you'd like

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/local_strings.js:64: var templateData = this.templateData
|| window['templateData'];
On 2014/07/16 18:33:03, arv wrote:
> Sad :'(

actually, this may not be necessary now that I've added externs

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/parse_html_subset.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/parse_html_subset.js:79: /** @suppress {missingProperties}
*/
On 2014/07/16 18:33:03, arv wrote:
> Why is this needed?

.createHTMLDocument()

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/util.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/util.js:235: var anchor = findAncestor(/** @type {Element}
*/(e.target), function(el) {
On 2014/07/16 18:33:03, arv wrote:
> e.target might be a Document too.

Changed to Node (document instanceof Node;  // true)

[arv (Not doing code reviews), 2014-07-21 18:25:31]

Is this just a copy of the files now?

https://codereview.chromium.org/369643002/diff/250001/ui/webui/resources/js/i...
File ui/webui/resources/js/i18n_template.js (right):

https://codereview.chromium.org/369643002/diff/250001/ui/webui/resources/js/i...
ui/webui/resources/js/i18n_template.js:111: var elements =
node.querySelectorAll(selector);
On 2014/07/19 02:28:40, Dan Beam wrote:
> On 2014/07/16 18:33:03, arv wrote:
> > Node does not have querySelectorAll. Do you want to change it to
{ParentNode}?
> 
> MDN says it's on Element...
> https://developer.mozilla.org/en-US/docs/Web/API/Element.querySelectorAll
> 
> As long as it compiles I don't particularly care.  I don't think Node has
> .classList either but it worked in the new CL:
> https://chromiumcodereview.appspot.com/405743002/

MDN is wrong. Shocking!

classList is part of Element
http://dom.spec.whatwg.org/#element

querySelectorAll is part of ParentNode
http://dom.spec.whatwg.org/#parentnode

> As long as it compiles I don't particularly care.

That is how google3 js type annotations got into a mess. It was almost
impossible to fix these things because the amount of invalid type annotations
out there was astonishing.

https://codereview.chromium.org/369643002/diff/250001/ui/webui/resources/js/p...
File ui/webui/resources/js/parse_html_subset.js (right):

https://codereview.chromium.org/369643002/diff/250001/ui/webui/resources/js/p...
ui/webui/resources/js/parse_html_subset.js:79: /** @suppress {missingProperties}
*/
On 2014/07/19 02:28:40, Dan Beam wrote:
> On 2014/07/16 18:33:03, arv wrote:
> > Why is this needed?
> 
> .createHTMLDocument()

I still don't understand why this refactoring is needed? Is there a bug in the
externs we are using?

[Dan Beam, 2014-07-21 22:30:46]

> Is this just a copy of the files now?

not really.

I started with a copy but massaged a lot of stuff (for instance, 2 => 4 \s
indent to match Chrome style).  I removed the devtools specific code and made it
more general.

build_runner_jar.py is pretty close but the rest (checker.py and
compile_modules.py) is a lot different.

Additionally, I added <include> inlining (which I don't know that devtools
uses...)

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
File ui/webui/resources/js/parse_html_subset.js (right):

https://chromiumcodereview.appspot.com/369643002/diff/250001/ui/webui/resourc...
ui/webui/resources/js/parse_html_subset.js:79: /** @suppress {missingProperties}
*/
On 2014/07/21 18:25:31, arv wrote:
> On 2014/07/19 02:28:40, Dan Beam wrote:
> > On 2014/07/16 18:33:03, arv wrote:
> > > Why is this needed?
> > 
> > .createHTMLDocument()
> 
> I still don't understand why this refactoring is needed? Is there a bug in the
> externs we are using?

yes, @suppress => @externs (and added to compiler) here:
https://chromiumcodereview.appspot.com/405743002/

[Dan Beam, 2014-07-21 22:31:13]

+vitalyp@ as FYI

[Dan Beam, 2014-07-22 19:10:25]

ping arv@

[arv (Not doing code reviews), 2014-07-22 21:18:49]

LGTM

[darin (slow to review), 2014-07-22 22:29:33]

LGTM

[Dan Beam, 2014-07-24 02:31:15]

https://code.google.com/p/chromium/issues/detail?id=78368#c50