Adding tests for experimental Safe DOM creation technique.

Adding tests for experimental Safe DOM creation technique.

This is just the underlying DOM creation code which should be able to parse HTML with no script execution. I'd like to get this in so we can make sure it works across all supported platforms. This does not do any sanitization or validation.

BUG=
R=jmesserly@google.com

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

[Jennifer Messerly, 2013-05-06 23:28:52]

lgtm

https://codereview.chromium.org/14839006/diff/2001/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/14839006/diff/2001/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:17559: JS('bool',
'("createContextualFragment" in window.Range.prototype)');
just curious, any reason to prefer this pattern over:

!!window.Range.prototype.createContextualFragment

https://codereview.chromium.org/14839006/diff/2001/tests/html/range_test.dart
File tests/html/range_test.dart (right):

https://codereview.chromium.org/14839006/diff/2001/tests/html/range_test.dart...
tests/html/range_test.dart:21: var expectation =
Range.supportsCreateContextualFragment ?
I think, because the above test fails in IE9, there's no way to know if this
part is working on IE9. For example, maybe support returns false, but
"createContextualFragment" doesn't throw on IE9. Maybe it would be better to
split the tests? Or change the supports_createContextualFragment to expect
"false" on IE9?

then again, might not be worth the effort for such small tests

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.dart
File tests/html/safe_dom_test.dart (right):

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:35: return isSafe().then((value) {
Can you "expect" a future? e.g.

expect(isSafe(), completion(true), reason: 'Expected unsafe code executed.');

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:36: expect(value, isTrue, reason: 'Expected unsafe
code executed.');
the "reason" here is backwards? we expected the code is safe, (not executed)

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:39: return ensureSafe();
this looks like dead code

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:45: div.innerHtml = unsafeString;
this is going to need to change if innerHtml is removed? :)

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:53: var fragment =
createContextualFragment('<span>content</span>');
it might be interesting to run it through our existing "new Element.html" tests.
perhaps that makes sense to do once it is changed to use
createContextualFragment under the covers.

[blois, 2013-05-07 00:43:08]

https://codereview.chromium.org/14839006/diff/2001/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/14839006/diff/2001/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:17559: JS('bool',
'("createContextualFragment" in window.Range.prototype)');
On 2013/05/06 23:28:52, John Messerly wrote:
> just curious, any reason to prefer this pattern over:
> 
> !!window.Range.prototype.createContextualFragment

I did some testing a while back and this was actually (slightly) faster overall.

https://codereview.chromium.org/14839006/diff/2001/tests/html/range_test.dart
File tests/html/range_test.dart (right):

https://codereview.chromium.org/14839006/diff/2001/tests/html/range_test.dart...
tests/html/range_test.dart:21: var expectation =
Range.supportsCreateContextualFragment ?
On 2013/05/06 23:28:52, John Messerly wrote:
> I think, because the above test fails in IE9, there's no way to know if this
> part is working on IE9. For example, maybe support returns false, but
> "createContextualFragment" doesn't throw on IE9. Maybe it would be better to
> split the tests? Or change the supports_createContextualFragment to expect
> "false" on IE9?
> 
> then again, might not be worth the effort for such small tests

These are using the individual test config, so only the 'supported' group is
marked as failing. The functional tests will all be executed on all platforms,
so this does validate that it fails on IE9.

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.dart
File tests/html/safe_dom_test.dart (right):

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:35: return isSafe().then((value) {
On 2013/05/06 23:28:52, John Messerly wrote:
> Can you "expect" a future? e.g.
> 
> expect(isSafe(), completion(true), reason: 'Expected unsafe code executed.');
> 

Yes! Never used that construct before :)

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:36: expect(value, isTrue, reason: 'Expected unsafe
code executed.');
On 2013/05/06 23:28:52, John Messerly wrote:
> the "reason" here is backwards? we expected the code is safe, (not executed)

Done.

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:39: return ensureSafe();
On 2013/05/06 23:28:52, John Messerly wrote:
> this looks like dead code

Done.

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:45: div.innerHtml = unsafeString;
On 2013/05/06 23:28:52, John Messerly wrote:
> this is going to need to change if innerHtml is removed? :)

Yeah, this would just use createContextualFragment with a null validator.

But I just tried that, and it's crashing DRT :/

I guess that's why it's good to get testing on all platforms!

https://codereview.chromium.org/14839006/diff/2001/tests/html/safe_dom_test.d...
tests/html/safe_dom_test.dart:53: var fragment =
createContextualFragment('<span>content</span>');
On 2013/05/06 23:28:52, John Messerly wrote:
> it might be interesting to run it through our existing "new Element.html"
tests.
> perhaps that makes sense to do once it is changed to use
> createContextualFragment under the covers.

Yep, this is primarily to make sure that the parsing is safe and works
everywhere.

[Jennifer Messerly, 2013-05-07 00:56:27]

https://codereview.chromium.org/14839006/diff/2001/tests/html/range_test.dart
File tests/html/range_test.dart (right):

https://codereview.chromium.org/14839006/diff/2001/tests/html/range_test.dart...
tests/html/range_test.dart:21: var expectation =
Range.supportsCreateContextualFragment ?
On 2013/05/07 00:43:08, blois wrote:
> On 2013/05/06 23:28:52, John Messerly wrote:
> > I think, because the above test fails in IE9, there's no way to know if this
> > part is working on IE9. For example, maybe support returns false, but
> > "createContextualFragment" doesn't throw on IE9. Maybe it would be better to
> > split the tests? Or change the supports_createContextualFragment to expect
> > "false" on IE9?
> > 
> > then again, might not be worth the effort for such small tests
> 
> These are using the individual test config, so only the 'supported' group is
> marked as failing. The functional tests will all be executed on all platforms,
> so this does validate that it fails on IE9.

oh, awesome!

[blois, 2013-05-07 18:32:54]

Committed patchset #2 manually as r22473 (presubmit successful).