code refactoring in import_inliner

code refactoring in import_inliner

I'm adding link rel=stylesheet support and implemented these changes first
to make it easier to understand the invariants.

R=sigmund@google.com

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

[Jennifer Messerly, 2014-02-12 00:48:47]

there should be no behavior changes here; I was mainly trying to make the async
code look more sync-y where possible, so I could figure out how to add link
rel=stylesheet support

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
File pkg/polymer/lib/src/build/import_inliner.dart (right):

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
pkg/polymer/lib/src/build/import_inliner.dart:49: return _visitImports(doc, id,
transform, seen, imported, scriptIds);
visit imports now produces the two primary outputs directly -- the imported
content and the script IDs.

(I tried to do the content insertion inside of _visitImports too, but that
didn't work out since it's tricky to keep a pointer for where to insert into the
document. However link rel=stylesheets in the body will likely work this way.)

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
pkg/polymer/lib/src/build/import_inliner.dart:50: }).then((importsFound) {
switched to this style to reduce nesting caused by chained futures.

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
pkg/polymer/lib/src/build/import_inliner.dart:61: JSON.encode(scriptIds,
toEncodable: (id) => id.serialize())));
an extra JSON.encode in the !importsFound case is probably fine, given all of
the work this code is doing :)

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
pkg/polymer/lib/src/build/import_inliner.dart:77: return
Future.forEach(document.queryAll('link'), (tag) {
instead of two back to back for loops, it now just has one

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
pkg/polymer/lib/src/build/import_inliner.dart:83: tag.remove();
remove the tag here, rather than another pass later

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
pkg/polymer/lib/src/build/import_inliner.dart:84: if (id == null ||
!seen.add(id) ||
Set.add now returns a boolean (true if it was added)

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
pkg/polymer/lib/src/build/import_inliner.dart:97: _extractScripts(id,
transform.logger, importedDoc, scripts);
FYI -- I'm changing this back to sourceId, not imported doc's ID, since we
already ran UrlNormalizer. Not sure if it matters (the tests work either way)

https://codereview.chromium.org/159353005/diff/1/pkg/polymer/lib/src/build/im...
pkg/polymer/lib/src/build/import_inliner.dart:114: static void
_extractScripts(id, logger, document, List scriptIds) {
this function seemed pretty self contained

[Jennifer Messerly, 2014-02-12 00:50:39]

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

https://codereview.chromium.org/159353005/diff/30001/pkg/polymer/lib/src/buil...
pkg/polymer/lib/src/build/import_inliner.dart:77: return
Future.forEach(document.queryAll('link'), (tag) {
it occurs to me that pulling this forEach outside of _visitImports might make
the code even cleaner... it's the only place we use "document"

[Jennifer Messerly, 2014-02-12 01:17:19]

PTAL ... introduced an explicit closure object so there is less context being
passed around to each function call.

also pulled out _inlineImport into a function ... that's where I plan to
introduce _inlineStylesheet

[Siggi Cherem (dart-lang), 2014-02-12 01:32:20]

lgtm

[Siggi Cherem (dart-lang), 2014-02-12 01:32:32]

(nice cleanup!)

[Jennifer Messerly, 2014-02-12 01:52:38]

Committed patchset #3 manually as r32587 (presubmit successful).