end_to_end_test detects errors instead of timing out.

end_to_end_test detects errors instead of timing out.

R=kustermann@google.com

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

[blois, 2014-04-01 16:16:25]

changes look fine to me- addEventListener is currently blocked as we don't
natively allow access to cross-frame types. There has been some discussion about
relaxing this restriction.

[kustermann, 2014-04-01 20:27:29]

lgtm

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/end_to_end_te...
File dart/tests/try/end_to_end_test.dart (right):

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/end_to_end_te...
dart/tests/try/end_to_end_test.dart:27: if (contentWindowProxy == null) return;
If contentWindowProxy is null, we should fail, otherwise the test will
eventually time out, right?

[and we don't like tests to time out]

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/end_to_end_te...
dart/tests/try/end_to_end_test.dart:71: installErrorHandlerOn(iframe);
Why are you doing this here and in the onIframeLoaded function?

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/try.status
File dart/tests/try/try.status (right):

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/try.status#ne...
dart/tests/try/try.status:10: end_to_end_test: Fail, OK #
http://dartbug.com/17935
That bug is marked as 'NotPlanned'.

So will this line be in the status file forever?
Is the purpose of this, to ensure it keeps failing?

[ahe, 2014-04-01 21:21:17]

Thank you, Pete and Martin for taking a look!

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/end_to_end_te...
File dart/tests/try/end_to_end_test.dart (right):

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/end_to_end_te...
dart/tests/try/end_to_end_test.dart:27: if (contentWindowProxy == null) return;
On 2014/04/01 20:27:30, kustermann wrote:
> If contentWindowProxy is null, we should fail, otherwise the test will
> eventually time out, right?
> 
> [and we don't like tests to time out]

I can't tell when contentWindowProxy is null. As far as I can gather:

* contentWindowProxy is always null before you add the iframe to the document.

* contentWindowProxy is null if iframe.src is a different origin.

However, it isn't clear from the documentation, so I'm trying to be robust.

I'll sleep on this.

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/end_to_end_te...
dart/tests/try/end_to_end_test.dart:71: installErrorHandlerOn(iframe);
On 2014/04/01 20:27:30, kustermann wrote:
> Why are you doing this here and in the onIframeLoaded function?

As far as I can gather, it matters. I think there are two different
contentWindows, depending on how far the browser has gotten. It's really hard to
find proper documentation about this.

I'll add some comments.

[ahe, 2014-04-03 11:05:34]

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/end_to_end_te...
File dart/tests/try/end_to_end_test.dart (right):

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/end_to_end_te...
dart/tests/try/end_to_end_test.dart:27: if (contentWindowProxy == null) return;
On 2014/04/01 21:21:17, ahe wrote:
> On 2014/04/01 20:27:30, kustermann wrote:
> > If contentWindowProxy is null, we should fail, otherwise the test will
> > eventually time out, right?
> > 
> > [and we don't like tests to time out]
> 
> I can't tell when contentWindowProxy is null. As far as I can gather:
> 
> * contentWindowProxy is always null before you add the iframe to the document.
> 
> * contentWindowProxy is null if iframe.src is a different origin.
> 
> However, it isn't clear from the documentation, so I'm trying to be robust.
> 
> I'll sleep on this.

Changed it to throw an exception.

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/try.status
File dart/tests/try/try.status (right):

https://codereview.chromium.org/220453006/diff/1/dart/tests/try/try.status#ne...
dart/tests/try/try.status:10: end_to_end_test: Fail, OK #
http://dartbug.com/17935
On 2014/04/01 20:27:30, kustermann wrote:
> That bug is marked as 'NotPlanned'.
> 
> So will this line be in the status file forever?

I think so.

> Is the purpose of this, to ensure it keeps failing?

Yes. It's either that or skip it. I'm not keen on skipping tests.

[ahe, 2014-04-03 11:07:12]

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