Add a content-security-policy (CSP) flag.

Add a content-security-policy (CSP) flag.

https://codereview.chromium.org/11183040/ contains before and after comparisons.

BUG=http://dartbug.com/3436

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

[floitsch, 2012-10-16 18:15:36]

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
File lib/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:279: // Use the newly
created object as prototype. In Chrome this creates a
Thanks to Erik Corry for this tip. Without these lines some benchmarks go down
by 40%.

[kasperl, 2012-10-17 09:37:11]

Maybe you should add a small comparison between the code with and without the
CSP flag so we can all see it?

LGTM.

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
File lib/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/apiimpl.dart:34: enableContentSecurityPolicy:
This is probably fine, but essentially you could also make this --disable-eval
because that is what you need to do to make the code adhere to the content
security policy.

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
File lib/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:103:
compiler.internalError("Dynamic Getters/Setters are unused in CSP");
Getters/Setters -> getters/setters

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:130: if
(useContentSecurityPolicy) {
It feels a little bit like subclassing would help here. The different functions
(defineClassFunction, protoSupportCheck, finishIsolateConstructorFunction)
depend on each other and having the CSP-enabled code next to each other might
make it easier to maintain.

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:272: var isolateProperties =
oldIsolate.${namer.ISOLATE_PROPERTIES};
Isn't there a way to construct the isolate object in a more static way?

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:281: function t(){}
Maybe give t a better name? Space after ).

[floitsch, 2012-10-17 21:15:00]

PTAL.
https://codereview.chromium.org/11183040/ contains before and after comparisons.

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
File lib/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/apiimpl.dart:34: enableContentSecurityPolicy:
On 2012/10/17 09:37:11, kasperl wrote:
> This is probably fine, but essentially you could also make this --disable-eval
> because that is what you need to do to make the code adhere to the content
> security policy.

Done.

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
File lib/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:103:
compiler.internalError("Dynamic Getters/Setters are unused in CSP");
On 2012/10/17 09:37:11, kasperl wrote:
> Getters/Setters -> getters/setters

Refactored.

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:130: if
(useContentSecurityPolicy) {
On 2012/10/17 09:37:11, kasperl wrote:
> It feels a little bit like subclassing would help here. The different
functions
> (defineClassFunction, protoSupportCheck, finishIsolateConstructorFunction)
> depend on each other and having the CSP-enabled code next to each other might
> make it easier to maintain.

Done.

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:272: var isolateProperties =
oldIsolate.${namer.ISOLATE_PROPERTIES};
On 2012/10/17 09:37:11, kasperl wrote:
> Isn't there a way to construct the isolate object in a more static way? 
There is, but since it's not necessary.

https://codereview.chromium.org/11188004/diff/4001/lib/compiler/implementatio...
lib/compiler/implementation/js_backend/emitter.dart:281: function t(){}
On 2012/10/17 09:37:11, kasperl wrote:
> Maybe give t a better name? Space after ).

Done.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/js_backend/emitter.dart (left):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter.dart:729: void
emitInstanceMembers(ClassElement classElement,
has been moved to above. No changes in this function.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter.dart:628: void
emitInstanceMembers(ClassElement classElement,
emitInstanceMembers was just moved.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter.dart:767: void
generateCheckedSetter(Element member,
generateCheckedSetter lost the boolean check which has been moved into
"canGenerateCheckedSetter" (just above).

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/js_backend/emitter_no_eval.dart (right):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:51: new
forceEfficientMap;
It is necessary to instantiate an object. Otherwise the prototype object is not
optimized.

[kasperl, 2012-10-18 09:36:21]

LGTM!

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/apiimpl.dart:34: disableEval: hasOption(options,
'--disable-eval'),
I think this would be even nicer if it was called 

   --disallow-unsafe-eval

because that matches the CSP restriction description better. See
http://www.w3.org/TR/CSP/.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/dart2js.dart (right):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/dart2js.dart:399: Disables dynamic generation of
code in the generated output. This is
Maybe get some of the text from http://www.w3.org/TR/CSP/ and/or link to it?

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/js_backend/emitter_no_eval.dart (right):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:29: return 'var
$supportsProtoName = !!{}.__proto__;';
Missing newline at end?

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:51: new
forceEfficientMap;
On 2012/10/17 21:15:01, floitsch wrote:
> It is necessary to instantiate an object. Otherwise the prototype object is
not
> optimized.

Cool. Good to know.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:65: function
$mangledName(self, target) { this.self = self; this.target = target; },
I'd prefer to stuff the 'function $mangledName(self, target) {' on the previous
line and add newline after the ;.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:92: buffer.add(")
{");
Newline after {.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:94: buffer.add("
this.$field = $field;");
Two space indent and newline after ;.

[floitsch, 2012-10-18 11:08:11]

thanks.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/apiimpl.dart:34: disableEval: hasOption(options,
'--disable-eval'),
On 2012/10/18 09:36:21, kasperl wrote:
> I think this would be even nicer if it was called 
> 
>    --disallow-unsafe-eval
> 
> because that matches the CSP restriction description better. See
> http://www.w3.org/TR/CSP/.

Done.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/dart2js.dart (right):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/dart2js.dart:399: Disables dynamic generation of
code in the generated output. This is
On 2012/10/18 09:36:21, kasperl wrote:
> Maybe get some of the text from http://www.w3.org/TR/CSP/ and/or link to it?

Done.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
File lib/compiler/implementation/js_backend/emitter_no_eval.dart (right):

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:29: return 'var
$supportsProtoName = !!{}.__proto__;';
On 2012/10/18 09:36:21, kasperl wrote:
> Missing newline at end?

Done.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:65: function
$mangledName(self, target) { this.self = self; this.target = target; },
On 2012/10/18 09:36:21, kasperl wrote:
> I'd prefer to stuff the 'function $mangledName(self, target) {' on the
previous
> line and add newline after the ;.

Done.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:92: buffer.add(")
{");
On 2012/10/18 09:36:21, kasperl wrote:
> Newline after {.

Done.

https://codereview.chromium.org/11188004/diff/13001/lib/compiler/implementati...
lib/compiler/implementation/js_backend/emitter_no_eval.dart:94: buffer.add("
this.$field = $field;");
On 2012/10/18 09:36:21, kasperl wrote:
> Two space indent and newline after ;.

Done.