Inline scripts were extracted from Polymer elements.

Inline scripts were extracted from Polymer elements.

Added a directory 'components-chromium'. The difference
with the 'components' directory is that all inline scripts
were extracted from Polymer elements to separate files. Such
version is needed if we want to use the elements in a web
UI, where inline scripts are forbidden by content security
policy. An extraction was made by the vulcanize tool. I
wrote the script called 'extract_inline_scripts.sh' to
handle all the elements at once. It finds HTML files
containing inline scripts (excluding some HTML files that
are not a components of Polymer) and applies vulcanize to
them.
A JS file extracted from a file named 'foo/bar.html' will
have a name 'foo/bar-extracted.js'.

Also changed the wrong dependency 'core-iconset-svg' in
bower.json and 'core-iconset-svg' was checked out correctly.

Landed manually, because of the bug in the commit queue (http://crbug.com/416255).

R=raymes@chromium.org, rsadam@chromium.org, thestig@chromium.org
TBR=cpu
BUG=415696
TEST=none

Committed: https://crrev.com/7bda37ee9e690822ebcfaed5ab73e1086fa6f285
Cr-Commit-Position: refs/heads/master@{#296721}

[dzhioev (left Google), 2014-09-21 05:17:10]

dzhioev@chromium.org changed reviewers:
+ raymes@chromium.org, rsadam@chromium.org

[dzhioev (left Google), 2014-09-21 05:19:58]

Hi, please review.

[raymes, 2014-09-22 00:44:31]

I saw talk on the thread of a python written vulcanize tool. It seems like it
would be better to run this tool at build-time rather than checking in a
modified version of the library? WDYT?

[dzhioev (left Google), 2014-09-22 01:45:38]

On 2014/09/22 00:44:31, raymes wrote:
> I saw talk on the thread of a python written vulcanize tool. It seems like it
> would be better to run this tool at build-time rather than checking in a
> modified version of the library? WDYT?

I completely agree. And I'm going to contribute to make this possible in M40-41.

Are you talking about the tool developed by trace-viewer team?
(BTW, I started thread [1] about Polymer in Chromium recently. Is this the
thread you are talking about?
If not, please please point me to the thread.)

I took a look at the tool. It's pretty good, but some changes need to be done,
to use it Chromium-wide, because it is not universal enough. Plus we need to
integrate the tool into current build process.

The problem is that I need to use several Polymer elements in M39, and the only 
way I see is checking the preprocessed files into the repository.

[dzhioev (left Google), 2014-09-22 01:47:02]

On 2014/09/22 01:45:38, dzhioev wrote:
> On 2014/09/22 00:44:31, raymes wrote:
> > I saw talk on the thread of a python written vulcanize tool. It seems like
it
> > would be better to run this tool at build-time rather than checking in a
> > modified version of the library? WDYT?
> 
> I completely agree. And I'm going to contribute to make this possible in
M40-41.
> 
> Are you talking about the tool developed by trace-viewer team?
> (BTW, I started thread [1] about Polymer in Chromium recently. Is this the
> thread you are talking about?
> If not, please please point me to the thread.)
> 
> I took a look at the tool. It's pretty good, but some changes need to be done,
> to use it Chromium-wide, because it is not universal enough. Plus we need to
> integrate the tool into current build process.
> 
> The problem is that I need to use several Polymer elements in M39, and the
only 
> way I see is checking the preprocessed files into the repository.

Sorry, I forgot about the link
[1]
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/E7abBtcMBHc

[raymes, 2014-09-22 01:53:35]

On 2014/09/22 01:47:02, dzhioev wrote:
> On 2014/09/22 01:45:38, dzhioev wrote:
> > On 2014/09/22 00:44:31, raymes wrote:
> > > I saw talk on the thread of a python written vulcanize tool. It seems like
> it
> > > would be better to run this tool at build-time rather than checking in a
> > > modified version of the library? WDYT?
> > 
> > I completely agree. And I'm going to contribute to make this possible in
> M40-41.
> > 
> > Are you talking about the tool developed by trace-viewer team?
> > (BTW, I started thread [1] about Polymer in Chromium recently. Is this the
> > thread you are talking about?
> > If not, please please point me to the thread.)

Yep the one Nat wrote.

> > I took a look at the tool. It's pretty good, but some changes need to be
done,
> > to use it Chromium-wide, because it is not universal enough. Plus we need to
> > integrate the tool into current build process.

Ah I see, that would be great to extend it to meet the needs for your project at
least :). Do you think it would take much work?

> > The problem is that I need to use several Polymer elements in M39, and the
> only 
> > way I see is checking the preprocessed files into the repository.

If this is just a temporary measure how about we just run vulcanize on your
particular project and check in the vulcanized version? One fear with modifying
the third party code is that someone else might come through and decide to
depend on it and then we would have to fix their dependency later on.

> Sorry, I forgot about the link
> [1]
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/E7abBtcMBHc

[dzhioev (left Google), 2014-09-22 02:30:40]

On Mon Sep 22 2014 at 5:53:36 AM <raymes@chromium.org> wrote:

> On 2014/09/22 01:47:02, dzhioev wrote:
> > On 2014/09/22 01:45:38, dzhioev wrote:
> > > On 2014/09/22 00:44:31, raymes wrote:
> > > > I saw talk on the thread of a python written vulcanize tool. It seems
> > like
> > it
> > > > would be better to run this tool at build-time rather than checking
> > in a
> > > > modified version of the library? WDYT?
> > >
> > > I completely agree. And I'm going to contribute to make this possible
> in
> > M40-41.
> > >
> > > Are you talking about the tool developed by trace-viewer team?
> > > (BTW, I started thread [1] about Polymer in Chromium recently. Is this
> > the
> > > thread you are talking about?
> > > If not, please please point me to the thread.)
>
> Yep the one Nat wrote.
>
> > > I took a look at the tool. It's pretty good, but some changes need to
> be
> done,
> > > to use it Chromium-wide, because it is not universal enough. Plus we
> > need to
> > > integrate the tool into current build process.
>
> Ah I see, that would be great to extend it to meet the needs for your
> project at
> least :). Do you think it would take much work?
>
 Here is a very "dirty" patch
https://gist.github.com/dzhioev/1229aae33ebbd6e0ed0c after which the tool
is almost able (some manual work is needed still) to produce a working
bundle of all existing polymer elements. The patch is small, but I think a
clean patch will be much bigger.

> > > The problem is that I need to use several Polymer elements in M39, and
> > the
> > only
> > > way I see is checking the preprocessed files into the repository.
>
> If this is just a temporary measure how about we just run vulcanize on your
> particular project and check in the vulcanized version? One fear with
> modifying
> the third party code is that someone else might come through and decide to
> depend on it and then we would have to fix their dependency later on.
>
But if I vulcanize my project, the vulcanized version will contain much of
the polymer code. I afraid the owners of chrome/browser will not be happy.

What if I keep 'components' directory clean, but create
'components-extracted' directory with the modified versions of the
components?

> Sorry, I forgot about the link
> > [1]
>
> https://groups.google.com/a/chromium.org/forum/#!topic/
> chromium-dev/E7abBtcMBHc
>
>
>
> https://codereview.chromium.org/592593002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[raymes, 2014-09-22 03:19:59]

On 2014/09/22 02:30:40, dzhioev wrote:
> On Mon Sep 22 2014 at 5:53:36 AM <mailto:raymes@chromium.org> wrote:
> 
> > On 2014/09/22 01:47:02, dzhioev wrote:
> > > On 2014/09/22 01:45:38, dzhioev wrote:
> > > > On 2014/09/22 00:44:31, raymes wrote:
> > > > > I saw talk on the thread of a python written vulcanize tool. It seems
> > > like
> > > it
> > > > > would be better to run this tool at build-time rather than checking
> > > in a
> > > > > modified version of the library? WDYT?
> > > >
> > > > I completely agree. And I'm going to contribute to make this possible
> > in
> > > M40-41.
> > > >
> > > > Are you talking about the tool developed by trace-viewer team?
> > > > (BTW, I started thread [1] about Polymer in Chromium recently. Is this
> > > the
> > > > thread you are talking about?
> > > > If not, please please point me to the thread.)
> >
> > Yep the one Nat wrote.
> >
> > > > I took a look at the tool. It's pretty good, but some changes need to
> > be
> > done,
> > > > to use it Chromium-wide, because it is not universal enough. Plus we
> > > need to
> > > > integrate the tool into current build process.
> >
> > Ah I see, that would be great to extend it to meet the needs for your
> > project at
> > least :). Do you think it would take much work?
> >
>  Here is a very "dirty" patch
> https://gist.github.com/dzhioev/1229aae33ebbd6e0ed0c after which the tool
> is almost able (some manual work is needed still) to produce a working
> bundle of all existing polymer elements. The patch is small, but I think a
> clean patch will be much bigger.
> 
> > > > The problem is that I need to use several Polymer elements in M39, and
> > > the
> > > only
> > > > way I see is checking the preprocessed files into the repository.
> >
> > If this is just a temporary measure how about we just run vulcanize on your
> > particular project and check in the vulcanized version? One fear with
> > modifying
> > the third party code is that someone else might come through and decide to
> > depend on it and then we would have to fix their dependency later on.
> >
> But if I vulcanize my project, the vulcanized version will contain much of
> the polymer code. I afraid the owners of chrome/browser will not be happy.

Yeah it isn't ideal. The other problem that I forgot about is that someone might
come in and try to change the vulcanized version :(

> What if I keep 'components' directory clean, but create
> 'components-extracted' directory with the modified versions of the
> components?

Something like that sounds good to me for now :)

> > Sorry, I forgot about the link
> > > [1]
> >
> > https://groups.google.com/a/chromium.org/forum/#!topic/
> > chromium-dev/E7abBtcMBHc
> >
> >
> >
> > https://codereview.chromium.org/592593002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[dzhioev (left Google), 2014-09-22 17:02:13]

On 2014/09/22 03:19:59, raymes wrote:
> On 2014/09/22 02:30:40, dzhioev wrote:
> > On Mon Sep 22 2014 at 5:53:36 AM <mailto:raymes@chromium.org> wrote:
> > 
> > > On 2014/09/22 01:47:02, dzhioev wrote:
> > > > On 2014/09/22 01:45:38, dzhioev wrote:
> > > > > On 2014/09/22 00:44:31, raymes wrote:
> > > > > > I saw talk on the thread of a python written vulcanize tool. It
seems
> > > > like
> > > > it
> > > > > > would be better to run this tool at build-time rather than checking
> > > > in a
> > > > > > modified version of the library? WDYT?
> > > > >
> > > > > I completely agree. And I'm going to contribute to make this possible
> > > in
> > > > M40-41.
> > > > >
> > > > > Are you talking about the tool developed by trace-viewer team?
> > > > > (BTW, I started thread [1] about Polymer in Chromium recently. Is this
> > > > the
> > > > > thread you are talking about?
> > > > > If not, please please point me to the thread.)
> > >
> > > Yep the one Nat wrote.
> > >
> > > > > I took a look at the tool. It's pretty good, but some changes need to
> > > be
> > > done,
> > > > > to use it Chromium-wide, because it is not universal enough. Plus we
> > > > need to
> > > > > integrate the tool into current build process.
> > >
> > > Ah I see, that would be great to extend it to meet the needs for your
> > > project at
> > > least :). Do you think it would take much work?
> > >
> >  Here is a very "dirty" patch
> > https://gist.github.com/dzhioev/1229aae33ebbd6e0ed0c after which the tool
> > is almost able (some manual work is needed still) to produce a working
> > bundle of all existing polymer elements. The patch is small, but I think a
> > clean patch will be much bigger.
> > 
> > > > > The problem is that I need to use several Polymer elements in M39, and
> > > > the
> > > > only
> > > > > way I see is checking the preprocessed files into the repository.
> > >
> > > If this is just a temporary measure how about we just run vulcanize on
your
> > > particular project and check in the vulcanized version? One fear with
> > > modifying
> > > the third party code is that someone else might come through and decide to
> > > depend on it and then we would have to fix their dependency later on.
> > >
> > But if I vulcanize my project, the vulcanized version will contain much of
> > the polymer code. I afraid the owners of chrome/browser will not be happy.
> 
> Yeah it isn't ideal. The other problem that I forgot about is that someone
might
> come in and try to change the vulcanized version :(
> 
> > What if I keep 'components' directory clean, but create
> > 'components-extracted' directory with the modified versions of the
> > components?
> 
> Something like that sounds good to me for now :)
> 
Done, preprocessed version is in separate directory now.
> > > Sorry, I forgot about the link
> > > > [1]
> > >
> > > https://groups.google.com/a/chromium.org/forum/#!topic/
> > > chromium-dev/E7abBtcMBHc
> > >
> > >
> > >
> > > https://codereview.chromium.org/592593002/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.

[raymes, 2014-09-23 00:27:50]

raymes@chromium.org changed reviewers:
+ thestig@chromium.org

[raymes, 2014-09-23 00:27:56]

This looks ok to me modulo the comments below. 

A couple of other things I'm not sure about:
1) I'm not sure about checking in these shell scripts here vs in tools/. I think
it is OK here and it makes sense to me but adding thestig@ to give his thoughts
on this type of thing.
2) I'm not a good reviewer for bash/shell style. I'm not sure how strictly we
apply it. It's not mentioned on http://www.chromium.org/developers/coding-style

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/extr...
File third_party/polymer/extract_inline_scripts.sh (right):

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/extr...
third_party/polymer/extract_inline_scripts.sh:32: while read original_html
original_html->original_html_filename

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
File third_party/polymer/reproduce.sh (right):

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
third_party/polymer/reproduce.sh:6: 
Please add a comment inside this script and extract_inline_scripts.sh saying
what they do. Could we call this "modify_components_for_chromium.sh" or
something similar?

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
third_party/polymer/reproduce.sh:16: find components/core-list -type f -exec
chmod -x {} \;
rather than modifying web-animations-js inside components/ why not modify the
version inside components-extracted/? Then all of our local modifications are in
components-extracted/. You could even change the name of components-extracted/
to be components-chromium. WDYT?

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
third_party/polymer/reproduce.sh:17: ./extract_inline_scripts.sh
Would we only every likely call this from here? Should we just inline it? If
there is a good reason to keep it separate that's ok too.

[raymes, 2014-09-23 00:48:10]

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
File third_party/polymer/reproduce.sh (right):

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
third_party/polymer/reproduce.sh:16: find components/core-list -type f -exec
chmod -x {} \;
Oh I see the reason now, because we don't want those files in the source tree at
all. We can't do this then :(

[dzhioev (left Google), 2014-09-23 09:48:54]

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/extr...
File third_party/polymer/extract_inline_scripts.sh (right):

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/extr...
third_party/polymer/extract_inline_scripts.sh:32: while read original_html
On 2014/09/23 00:27:56, raymes wrote:
> original_html->original_html_filename

Done.

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
File third_party/polymer/reproduce.sh (right):

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
third_party/polymer/reproduce.sh:6: 
On 2014/09/23 00:27:56, raymes wrote:
> Please add a comment inside this script and extract_inline_scripts.sh saying
> what they do.
extract_inline_scripts.sh already has a usage message.
Added comment in reproduce.sh.

>Could we call this "modify_components_for_chromium.sh" or
> something similar?
I think reproduce.sh is a right name, because a reproducing of the directory
content is exactly what the script does.

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
third_party/polymer/reproduce.sh:16: find components/core-list -type f -exec
chmod -x {} \;
On 2014/09/23 00:48:10, raymes wrote:
> Oh I see the reason now, because we don't want those files in the source tree
at
> all. We can't do this then :(

Yep

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
third_party/polymer/reproduce.sh:16: find components/core-list -type f -exec
chmod -x {} \;
On 2014/09/23 00:27:56, raymes wrote:
> rather than modifying web-animations-js inside components/ why not modify the
> version inside components-extracted/? Then all of our local modifications are
in
> components-extracted/. You could even change the name of components-extracted/
> to be components-chromium. WDYT?

Renamed components-extracted to components-chromium

https://codereview.chromium.org/592593002/diff/50001/third_party/polymer/repr...
third_party/polymer/reproduce.sh:17: ./extract_inline_scripts.sh
On 2014/09/23 00:27:56, raymes wrote:
> Would we only every likely call this from here? Should we just inline it? If
> there is a good reason to keep it separate that's ok too.

It's pretty large, so I think it's good idea to keep it in a separate file.

[raymes, 2014-09-24 04:22:00]

lgtm thanks!

[rsadam, 2014-09-24 15:03:33]

lgtm, except *.sh, for which I'm also unfamiliar with the style rules for these
in Chromium.

[Lei Zhang, 2014-09-24 17:15:49]

shell scripts lgtm

https://codereview.chromium.org/592593002/diff/60001/third_party/polymer/extr...
File third_party/polymer/extract_inline_scripts.sh (right):

https://codereview.chromium.org/592593002/diff/60001/third_party/polymer/extr...
third_party/polymer/extract_inline_scripts.sh:12: echo ""
nit: just "echo" will do

[dzhioev (left Google), 2014-09-25 16:37:11]

Committed patchset #10 (id:100001) manually as 7bda37e (presubmit successful).

[commit-bot: I haz the power, 2014-09-25 16:37:13]

Patchset 10 (id:??) landed as
https://crrev.com/7bda37ee9e690822ebcfaed5ab73e1086fa6f285
Cr-Commit-Position: refs/heads/master@{#296721}