Replace bootstrap logic with 'boot.js', use 'component/dart' mime-type and add

Replace bootstrap logic with 'boot.js', use 'component/dart' mime-type and add
warnings to prepare users for the future breaking change in Dartium.

R=jmesserly@google.com

Committed: https://code.google.com/p/dart/source/detail?r=34753

[Siggi Cherem (dart-lang), 2014-04-04 02:54:14]

we still need to fix html5lib, but this is ready for a first look.

https://codereview.chromium.org/225043004/diff/20001/pkg/pkg.status
File pkg/pkg.status (left):

https://codereview.chromium.org/225043004/diff/20001/pkg/pkg.status#oldcode183
pkg/pkg.status:183: polymer/example/canonicalization/test/dev_test: Fail, OK #
tests development only behavior
now all canonicalization is consistent regardless of whether we are in dev-mode
or deploy-mode

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/example/cano...
File pkg/polymer/example/canonicalization/test/canonicalization1_test.dart
(left):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/example/cano...
pkg/polymer/example/canonicalization/test/canonicalization1_test.dart:5:
#include "vm/globals.h"
where did rietveld came up with this diff?

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/example/cano...
File
pkg/polymer/example/canonicalization2/test/bad_lib_import2_negative_test.dart
(right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/example/cano...
pkg/polymer/example/canonicalization2/test/bad_lib_import2_negative_test.dart:18:
@initMethod
all tests now follow this structure. I actually tried naming this something
other than 'main', but the bots are "smart" and try to filter tests if they
can't find a 'main' method.

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.dart
File pkg/polymer/lib/boot.dart (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.dar...
pkg/polymer/lib/boot.dart:6: /// yet but it will replace boot.js in the near
future (see dartbug.com/18007).
this was working with overriding the package-root, the code here is practically
equivalent to the one in boot.js

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/test/mirror_...
File pkg/polymer/test/mirror_loader_test.dart (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/test/mirror_...
pkg/polymer/test/mirror_loader_test.dart:14: import
'package:polymer/src/mirror_loader.dart';
this test can no longer be written as an end-to-end tests because this needs to
test what happens in 'startPolymer' (which is before any @initMethod is run).
Instead we just invoke it programatically here mocking the situation that we
were testing before and run it as a traditional (non-polymer) test.

[Jennifer Messerly, 2014-04-04 19:26:28]

still looking, but one issue I wanted to raise

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js
File pkg/polymer/lib/boot.js (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js#...
pkg/polymer/lib/boot.js:26: ///   * Use 'component/dart' instead of
'application/dart': Dartium already
based on existing mime content types, I'm not sure we can use "component/"
http://www.iana.org/assignments/media-types/media-types.xhtml#application
http://en.wikipedia.org/wiki/Internet_media_type#Type_application

we may need to get it registered, too, depending on choice:
http://www.iana.org/assignments/media-types/application/vnd-dart


based on that we might want to go with something like:

"application/dart;component=1"

since "component=1" is a parameter it doesn't need to be registered. FWIW,
Dartium does not currently recognize it, I tried and it won't execute scripts
with parameters in the content-type.

Here was also the RFC where they recommend parameters over "x-" types:
http://tools.ietf.org/html/rfc6648#page-4

(Parameters would also be a good place to encode the isolate affinity or other
VM startup options, should we ever choose to add them. Other options include
things like "application/dart.component" or "application/dart+component" but I
think those would need to be registered.)

[Jennifer Messerly, 2014-04-04 19:39:49]

other than the content type issue this LGTM

https://codereview.chromium.org/225043004/diff/20001/pkg/pkg.status
File pkg/pkg.status (left):

https://codereview.chromium.org/225043004/diff/20001/pkg/pkg.status#oldcode183
pkg/pkg.status:183: polymer/example/canonicalization/test/dev_test: Fail, OK #
tests development only behavior
On 2014/04/04 02:54:14, Siggi Cherem (dart-lang) wrote:
> now all canonicalization is consistent regardless of whether we are in
dev-mode
> or deploy-mode

nice!!!

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js
File pkg/polymer/lib/boot.js (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js#...
pkg/polymer/lib/boot.js:72: // seems wrong but it hides a warning in Dartium
(darbug.com/18000).
typo in the darTbug link :)

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js#...
pkg/polymer/lib/boot.js:73: return "data:text/javascript;base64," +
window.btoa(script.textContent);
WOW. That is beyond crazy!

Does it have to be "text/javascript" to avoid the warning?

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/import_inliner.dart (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/import_inliner.dart:23: class _HtmlInliner extends
PolymerTransformer {
do you mind adding a TODO here, that this transformer should move to
web_components? since it's not really a Polymer feature to inline HTML Imports.

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/linter.dart (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/linter.dart:174: _logger.warning(USE_POLYMER_HTML,
span: doc.body.sourceSpan);
lovely! the need for polymer.html has tripped some folks up, great that we have
a warning for it now

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/linter.dart:384: 'initPolymer are now deprecated. ';
maybe replace "are now deprecated" with "will not work" ? ;-)

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/script_compactor.dart (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/script_compactor.dart:149: .whenComplete(() =>
resolver.release());
nice catch!

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:2: version: 0.10.0-pre.7.dev
think it's worth releasing immediately?

[Siggi Cherem (dart-lang), 2014-04-04 21:09:11]

Thanks John.

I noticed there were a couple other examples that needed fixes (polymer_intl and
searchable_list) so I fixed those too.

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js
File pkg/polymer/lib/boot.js (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js#...
pkg/polymer/lib/boot.js:26: ///   * Use 'component/dart' instead of
'application/dart': Dartium already
On 2014/04/04 19:26:29, John Messerly wrote:
> based on existing mime content types, I'm not sure we can use "component/"
> http://www.iana.org/assignments/media-types/media-types.xhtml#application
> http://en.wikipedia.org/wiki/Internet_media_type#Type_application
> 
> we may need to get it registered, too, depending on choice:
> http://www.iana.org/assignments/media-types/application/vnd-dart
> 
> 
> based on that we might want to go with something like:
> 
> "application/dart;component=1"
> 
> since "component=1" is a parameter it doesn't need to be registered. FWIW,
> Dartium does not currently recognize it, I tried and it won't execute scripts
> with parameters in the content-type.
> 
> Here was also the RFC where they recommend parameters over "x-" types:
> http://tools.ietf.org/html/rfc6648#page-4
> 
> (Parameters would also be a good place to encode the isolate affinity or other
> VM startup options, should we ever choose to add them. Other options include
> things like "application/dart.component" or "application/dart+component" but I
> think those would need to be registered.)

Interesting - sounds good to me. I switched to application/dart;component=1. We
might have to change that again in the future though. We need to make sure
Dartium will not start loading them (ignoring the parameter)

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js#...
pkg/polymer/lib/boot.js:72: // seems wrong but it hides a warning in Dartium
(darbug.com/18000).
On 2014/04/04 19:39:50, John Messerly wrote:
> typo in the darTbug link :)

:). Done

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/boot.js#...
pkg/polymer/lib/boot.js:73: return "data:text/javascript;base64," +
window.btoa(script.textContent);
On 2014/04/04 19:39:50, John Messerly wrote:
> WOW. That is beyond crazy!
> 
> Does it have to be "text/javascript" to avoid the warning?

I know, it's pretty weird. I tried a bunch of things (text/plain,
application/vnd-dart, application/postscript, application/xml), but only got JS
and json to work without a warning. Maybe json feels less egregious, so I
switched to that. We could also just keep the warning and live with it until the
bug is fixed.

In playing with this I noticed a bug resolving relative imports, but I don't
think we can fix that so easily, so I filed an issue for it (dartbug.com/18040)

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/import_inliner.dart (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/import_inliner.dart:23: class _HtmlInliner extends
PolymerTransformer {
On 2014/04/04 19:39:50, John Messerly wrote:
> do you mind adding a TODO here, that this transformer should move to
> web_components? since it's not really a Polymer feature to inline HTML
Imports.

Sure thing. I created a bug too: dartbug.com/18037

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
File pkg/polymer/lib/src/build/script_compactor.dart (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/script_compactor.dart:149: .whenComplete(() =>
resolver.release());
On 2014/04/04 19:39:50, John Messerly wrote:
> nice catch!

yeah, Pete noticed this in code_transformers and then I followed. Whenever test
had failures, we were seeing timeouts because of this (we didn't release on a
failing test, and then the following test would be in deadlock)

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:2: version: 0.10.0-pre.7.dev
On 2014/04/04 19:39:50, John Messerly wrote:
> think it's worth releasing immediately?

Ok. As -pre.7 right, or 0.10.0?

[Jennifer Messerly, 2014-04-04 21:26:38]

still looks good!

there are some long lines in the HTML files but I dunno if it's worth fixing.
(FWIW, polymer's HTML doesn't follow 80 col so I may have quite a bit more of
those files soon, since ideally we're checking them in unmodified.)

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:2: version: 0.10.0-pre.7.dev
On 2014/04/04 21:09:11, Siggi Cherem (dart-lang) wrote:
> On 2014/04/04 19:39:50, John Messerly wrote:
> > think it's worth releasing immediately?
> 
> Ok. As -pre.7 right, or 0.10.0?

good question. my vote is -pre.7 and give it a few days of manual testing then
promote if it looks good.

https://codereview.chromium.org/225043004/diff/70001/pkg/polymer/lib/boot.dart
File pkg/polymer/lib/boot.dart (right):

https://codereview.chromium.org/225043004/diff/70001/pkg/polymer/lib/boot.dar...
pkg/polymer/lib/boot.dart:101: if (state.seen.contains(doc)) return
state.scripts;
small thing, Set.add returns true if it added, so you can actually do this in
one line:

if (!state.seen.add(doc)) return state.scripts;


https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart-core.Set...

https://codereview.chromium.org/225043004/diff/70001/pkg/polymer/lib/boot.js
File pkg/polymer/lib/boot.js (right):

https://codereview.chromium.org/225043004/diff/70001/pkg/polymer/lib/boot.js#...
pkg/polymer/lib/boot.js:127: window.addEventListener('HTMLImportsLoaded',
function (e) {
this probably doesn't matter now, but in Chrome 35 I think HTML Imports is
turned on, so we won't need HTML Imports polyfill in Dartium

[Jennifer Messerly, 2014-04-04 21:26:40]

still looks good!

there are some long lines in the HTML files but I dunno if it's worth fixing.
(FWIW, polymer's HTML doesn't follow 80 col so I may have quite a bit more of
those files soon, since ideally we're checking them in unmodified.)

[Siggi Cherem (dart-lang), 2014-04-04 23:09:54]

On 2014/04/04 21:26:38, John Messerly wrote:
> still looks good!
> 
> there are some long lines in the HTML files but I dunno if it's worth fixing.
> (FWIW, polymer's HTML doesn't follow 80 col so I may have quite a bit more of
> those files soon, since ideally we're checking them in unmodified.)

Thanks, I went ahead and fixed those too.

[Siggi Cherem (dart-lang), 2014-04-04 23:10:06]

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/pubspec.yaml
File pkg/polymer/pubspec.yaml (right):

https://codereview.chromium.org/225043004/diff/20001/pkg/polymer/pubspec.yaml...
pkg/polymer/pubspec.yaml:2: version: 0.10.0-pre.7.dev
On 2014/04/04 21:26:39, John Messerly wrote:
> On 2014/04/04 21:09:11, Siggi Cherem (dart-lang) wrote:
> > On 2014/04/04 19:39:50, John Messerly wrote:
> > > think it's worth releasing immediately?
> > 
> > Ok. As -pre.7 right, or 0.10.0?
> 
> good question. my vote is -pre.7 and give it a few days of manual testing then
> promote if it looks good.

sounds good.

https://codereview.chromium.org/225043004/diff/70001/pkg/polymer/lib/boot.dart
File pkg/polymer/lib/boot.dart (right):

https://codereview.chromium.org/225043004/diff/70001/pkg/polymer/lib/boot.dar...
pkg/polymer/lib/boot.dart:101: if (state.seen.contains(doc)) return
state.scripts;
On 2014/04/04 21:26:39, John Messerly wrote:
> small thing, Set.add returns true if it added, so you can actually do this in
> one line:
> 
> if (!state.seen.add(doc)) return state.scripts;
> 
> 
>
https://api.dartlang.org/apidocs/channels/stable/dartdoc-viewer/dart-core.Set...

Done.

https://codereview.chromium.org/225043004/diff/70001/pkg/polymer/lib/boot.js
File pkg/polymer/lib/boot.js (right):

https://codereview.chromium.org/225043004/diff/70001/pkg/polymer/lib/boot.js#...
pkg/polymer/lib/boot.js:127: window.addEventListener('HTMLImportsLoaded',
function (e) {
On 2014/04/04 21:26:39, John Messerly wrote:
> this probably doesn't matter now, but in Chrome 35 I think HTML Imports is
> turned on, so we won't need HTML Imports polyfill in Dartium

Good point. I didn't realize this was an event from the polyfill. I changed
things a bit to depend on the 'load' event instead. <script> tags ensure imports
are resolved first, but this current script tag (boot.js) is in polymer.html, so
it only ensures imports of polymer.html are loaded at the point this code is
first executed.

[Jennifer Messerly, 2014-04-04 23:14:07]

LGTM^2

[Siggi Cherem (dart-lang), 2014-04-04 23:43:53]

Committed patchset #4 manually as r34753 (presubmit successful).

[kasperl, 2014-06-11 07:56:15]

I think we need to be a lot more careful in how we deal with version constraints
for samples that ship with the SDK.

https://codereview.chromium.org/225043004/diff/90001/samples/third_party/todo...
File samples/third_party/todomvc/pubspec.yaml (right):

https://codereview.chromium.org/225043004/diff/90001/samples/third_party/todo...
samples/third_party/todomvc/pubspec.yaml:5: polymer: ">=0.10.0-pre.7 <0.11.0"
Isn't this a pretty broken constraint? Are you guaranteeing no breaking changes
in that whole version range?

This sample will ship with the SDK so people will be downloading the current
version of it in a couple of months. If you've updated polymer before then and
introduced any breaking changes without updating the polymer version to 0.11.0+,
you will have effectively broken the sample for a ton of users.

[Jennifer Messerly, 2014-06-11 17:45:43]

https://codereview.chromium.org/225043004/diff/90001/samples/third_party/todo...
File samples/third_party/todomvc/pubspec.yaml (right):

https://codereview.chromium.org/225043004/diff/90001/samples/third_party/todo...
samples/third_party/todomvc/pubspec.yaml:5: polymer: ">=0.10.0-pre.7 <0.11.0"
On 2014/06/11 07:56:14, kasperl wrote:
> Isn't this a pretty broken constraint? Are you guaranteeing no breaking
changes
> in that whole version range?
> 
> This sample will ship with the SDK so people will be downloading the current
> version of it in a couple of months. If you've updated polymer before then and
> introduced any breaking changes without updating the polymer version to
0.11.0+,
> you will have effectively broken the sample for a ton of users.

this was indeed the issue, yup. originally the 0.10.0-pre period was supposed to
be short, with 0.10 out before the SDK released. We goofed in including another
large breaking change on top of 0.10-pre. Lesson learned :(

[Siggi Cherem (dart-lang), 2014-06-11 19:00:57]

On 2014/06/11 17:45:43, John Messerly wrote:
>
https://codereview.chromium.org/225043004/diff/90001/samples/third_party/todo...
> File samples/third_party/todomvc/pubspec.yaml (right):
> 
>
https://codereview.chromium.org/225043004/diff/90001/samples/third_party/todo...
> samples/third_party/todomvc/pubspec.yaml:5: polymer: ">=0.10.0-pre.7 <0.11.0"
> On 2014/06/11 07:56:14, kasperl wrote:
> > Isn't this a pretty broken constraint? Are you guaranteeing no breaking
> changes
> > in that whole version range?
> > 
> > This sample will ship with the SDK so people will be downloading the current
> > version of it in a couple of months. If you've updated polymer before then
and
> > introduced any breaking changes without updating the polymer version to
> 0.11.0+,
> > you will have effectively broken the sample for a ton of users.
> 
> this was indeed the issue, yup. originally the 0.10.0-pre period was supposed
to
> be short, with 0.10 out before the SDK released. We goofed in including
another
> large breaking change on top of 0.10-pre. Lesson learned :(

Yeah. We are working with Rico to improve our testing infrastructure too.
Ideally we shouldn't have to update the samples when we are modifying a package.
For that we need the tests in the sample to run against the code in published
packages.