Apply content-security-policy to chrome://plugins page. This involves

Apply content-security-policy to chrome://plugins page.  This involves
converting all inline javascript to external .js files, since CSP can't
determine whether inline script is legitimate or an XSS.

We move the jstemplate_compiled.js file to the share resources directory,
since several WEBUI components will need this, and update the compile.py
script that produces it to write to this new location.

We move the javascript out of plugins.html to plugins.js; note that the
sections at line 130 and line 251 are new.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=87358

[Tom Sepez, 2011-05-27 18:42:38]

Needs another reviewer.  OWNERS file says arv, and he's not around.  Thoughts?

[abarth-chromium, 2011-05-27 18:48:06]

We should figure out who should review this patch with arv out of pocket.  Maybe
"svn blame" to see how usually works on these files?

http://codereview.chromium.org/7086003/diff/3002/chrome/browser/resources/plu...
File chrome/browser/resources/plugins.js (right):

http://codereview.chromium.org/7086003/diff/3002/chrome/browser/resources/plu...
chrome/browser/resources/plugins.js:133: links[i].onclick = function () {
Maybe addEventListener('click') ?  I'm not sure what style we're supposed to use
here.

http://codereview.chromium.org/7086003/diff/3002/chrome/browser/resources/plu...
chrome/browser/resources/plugins.js:138: links =
document.getElementsByClassName('enable-2');
Can we give these more semantic names than "enable-2" ?

[Tom Sepez, 2011-05-27 18:53:02]

On 2011/05/27 18:48:06, abarth wrote:
> We should figure out who should review this patch with arv out of pocket. 
Maybe
> "svn blame" to see how usually works on these files?
> 
>
http://codereview.chromium.org/7086003/diff/3002/chrome/browser/resources/plu...
> File chrome/browser/resources/plugins.js (right):
> 
>
http://codereview.chromium.org/7086003/diff/3002/chrome/browser/resources/plu...
> chrome/browser/resources/plugins.js:133: links[i].onclick = function () {
> Maybe addEventListener('click') ?  I'm not sure what style we're supposed to
use
> here.
> 

Both styles seem to be in wide use in the resources/ directory.
>
http://codereview.chromium.org/7086003/diff/3002/chrome/browser/resources/plu...
> chrome/browser/resources/plugins.js:138: links =
> document.getElementsByClassName('enable-2');
> Can we give these more semantic names than "enable-2" ?

Sure.

[Tom Sepez, 2011-05-27 19:09:20]

+tfarina for plugins.html

[Evan Martin, 2011-05-27 19:13:24]

http://codereview.chromium.org/7086003/diff/3003/chrome/third_party/jstemplat...
File chrome/third_party/jstemplate/README.chromium (right):

http://codereview.chromium.org/7086003/diff/3003/chrome/third_party/jstemplat...
chrome/third_party/jstemplate/README.chromium:5: so as to allow this to reside
in the shared resource bundle.
This feels a bit fragile.  Why not let the compile command accept the output
file on the command line?

[Tom Sepez, 2011-05-27 19:16:27]

On 2011/05/27 19:13:24, Evan Martin wrote:
>
http://codereview.chromium.org/7086003/diff/3003/chrome/third_party/jstemplat...
> File chrome/third_party/jstemplate/README.chromium (right):
> 
>
http://codereview.chromium.org/7086003/diff/3003/chrome/third_party/jstemplat...
> chrome/third_party/jstemplate/README.chromium:5: so as to allow this to reside
> in the shared resource bundle.
> This feels a bit fragile.  Why not let the compile command accept the output
> file on the command line?

We could do this, but this way it's obvious from inspection where the output
needs to go.  It would be a shame to generate a new file in this directory and
not have it picked up.

[tfarina, 2011-05-27 19:20:26]

bauerb or viettrungluu are better reviewers for plugins.html than me.

[Tom Sepez, 2011-05-27 19:21:45]

+bauerb +viettrungluu as reviewers.

[Bernhard Bauer, 2011-05-27 20:01:29]

LGTM.

http://codereview.chromium.org/7086003/diff/3003/chrome/browser/resources/plu...
File chrome/browser/resources/plugins.html (right):

http://codereview.chromium.org/7086003/diff/3003/chrome/browser/resources/plu...
chrome/browser/resources/plugins.html:9: <meta http-equiv="X-WebKit-CSP"
content="object-src 'none'; script-src chrome://resources 'self' 'unsafe-eval'">
Why do we allow unsafe eval?

http://codereview.chromium.org/7086003/diff/3003/chrome/browser/ui/webui/plug...
File chrome/browser/ui/webui/plugins_ui.cc (right):

http://codereview.chromium.org/7086003/diff/3003/chrome/browser/ui/webui/plug...
chrome/browser/ui/webui/plugins_ui.cc:104: SendFromResourceBundle(request_id,
idr);
We should think about a more general way to do this, that would work for other
Web UI pages as well.

[Tom Sepez, 2011-05-27 20:07:36]

On 2011/05/27 20:01:29, Bernhard Bauer wrote:
> LGTM.
> 
>
http://codereview.chromium.org/7086003/diff/3003/chrome/browser/resources/plu...
> File chrome/browser/resources/plugins.html (right):
> 
>
http://codereview.chromium.org/7086003/diff/3003/chrome/browser/resources/plu...
> chrome/browser/resources/plugins.html:9: <meta http-equiv="X-WebKit-CSP"
> content="object-src 'none'; script-src chrome://resources 'self'
'unsafe-eval'">
> Why do we allow unsafe eval?
> 
The jstemplate.js uses eval to process some of its js* attributes it encounters
on the template nodes/
>
http://codereview.chromium.org/7086003/diff/3003/chrome/browser/ui/webui/plug...
> File chrome/browser/ui/webui/plugins_ui.cc (right):
> 
>
http://codereview.chromium.org/7086003/diff/3003/chrome/browser/ui/webui/plug...
> chrome/browser/ui/webui/plugins_ui.cc:104: SendFromResourceBundle(request_id,
> idr);
> We should think about a more general way to do this, that would work for other
> Web UI pages as well.

Yep, the ChromeWebUIDataSource class was introduced a few CLs ago to reduce the
duplication.  There may be other opportunities here as well.

[Tom Sepez, 2011-05-31 19:02:16]

Adam, you ok with this now?
Evan, you ok with what I said in response to your comment?

(I want to see the bots go green, rather than presuming its just bot weirdness
...)

[Evan Martin, 2011-05-31 19:12:08]

On 2011/05/31 19:02:16, Tom Sepez wrote:
> Adam, you ok with this now?
> Evan, you ok with what I said in response to your comment?

I still think it is very fragile to output files in some relative directory --
for example, it will break if any of the involved directories are renamed.

I guess this isn't run as part of the build process, so maybe it doesn't matter
too much.

I think I would feel better if this just wrote to stdout and the user of it was
required to explicitly route it into whichever path they needed chose.

Or at least print 'wrote output into $out' so it's clear what happened .

[abarth-chromium, 2011-05-31 19:17:06]

On 2011/05/31 19:02:16, Tom Sepez wrote:
> Adam, you ok with this now?

Yep.  I'm happy with this whenever Evan's happy with it.

[Tom Sepez, 2011-05-31 19:47:14]

> I think I would feel better if this just wrote to stdout and the user of it
was
> required to explicitly route it into whichever path they needed chose.

Done.  See comments in README.chromium

[Evan Martin, 2011-05-31 20:37:11]

LGTM