Cross frame access tests

Cross frame access tests

We're allowing illegal cross-frame access on both Dartium and Frog right now.  I'm working on fixing the latter.

If these tests look good, I'll set the status file accordingly and land.

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

[vsm, 2012-02-23 06:00:38]

https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
File client/tests/client/dom/CrossFrameTest.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
client/tests/client/dom/CrossFrameTest.dart:8: test('contentWindow', () {
Passes on Dartium, fails with Frog.

https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
client/tests/client/dom/CrossFrameTest.dart:20: test('contentDocument', () {
Fails on both.

https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
File client/tests/client/dom/InnerFrameTest.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
client/tests/client/dom/InnerFrameTest.dart:33: Expect.isNull(div);
Fails on Dartium.

[vsm, 2012-02-24 03:00:34]

Stephen,

I added Frog code to return a wrapped window.  This gets tests passing.

There's still some TODO's I need to fill in.  I'm surprised I need to explicitly
throw an exception on the document access.  It wasn't working if I just left it
undefined.

On 2012/02/23 06:00:38, vsm wrote:
>
https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
> File client/tests/client/dom/CrossFrameTest.dart (right):
> 
>
https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
> client/tests/client/dom/CrossFrameTest.dart:8: test('contentWindow', () {
> Passes on Dartium, fails with Frog.
> 
>
https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
> client/tests/client/dom/CrossFrameTest.dart:20: test('contentDocument', () {
> Fails on both.
> 
>
https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
> File client/tests/client/dom/InnerFrameTest.dart (right):
> 
>
https://chromiumcodereview.appspot.com/9453004/diff/1/client/tests/client/dom...
> client/tests/client/dom/InnerFrameTest.dart:33: Expect.isNull(div);
> Fails on Dartium.

[sra1, 2012-02-24 03:38:30]

LGTM modulo comments.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
File client/dom/frog/dom_frog.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3007: Window get _contentWindow() native """
For short ones that fit on the same line I just go with 'return xxx;';

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3017: // TODO(vsm): Unify with Dartium version.
The file(s) containing this function is missing from the CL, so I'm making
comments here.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3018: class _DOMWindowCrossFrameJs implements
DOMType, DOMWindow {
Drop the 'Js'.  That is for native classes.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3039: var get document() { throw new Exception();
}
This is weird.
I wonder if it is related to the top level definition of get:document.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3054: void postMessage(Dynamic message, String
targetOrigin, [List messagePorts = null]) {
Line length

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/scripts/s...
File client/dom/scripts/systemfrog.py (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/scripts/s...
client/dom/scripts/systemfrog.py:15: _overridden_members = set([
Call this _dom_frog_omitted_members.
With everyone currently doing import * the names need to be generator specific.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
File client/tests/client/dom/CrossFrameTest.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
client/tests/client/dom/CrossFrameTest.dart:13: // Test this field to ensure a
valid Dart wrapper.
Since all native classes implement this, it is not distinguishing between the
wrapper and a native object.
Since dartObjectLocalStorages is unused unless the wrapping dart:html library
has wrapped it, it will always be null.
I want to get rid of dartObjectLocalStorage if possible after the direct
dart:html lands.

Is there something else you can test?

If the issue is lack of access to private methods, I think it is OK for dart:dom
to export a public testing class like this:

class DOMTesting {
  bool static isDOMWindowCrossFrame(object) => object is __DOMWindowCrossFrame;
}

Longer term the language needs to make it possible for tests to be in library /
package / whatever.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
File client/tests/client/dom/InnerFrameTest.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
client/tests/client/dom/InnerFrameTest.dart:25: iframe.addEventListener('load',
(e) => callbackDone(), false);
Will the script have completed by this time, or does it also run onload,
possibly after this?

[antonm, 2012-02-24 09:28:54]

Which of the tests fails on Dartium?

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
File client/tests/client/dom/InnerFrameTest.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
client/tests/client/dom/InnerFrameTest.dart:6: if (window != window.top) {
nit: I think it should be !==

[podivilov, 2012-02-24 12:18:58]

lgtm

[antonm, 2012-02-24 15:31:23]

Thanks a lot for clarifications, Vijay

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
File client/tests/client/dom/CrossFrameTest.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
client/tests/client/dom/CrossFrameTest.dart:26: });
nit: -2 spaces, please

[vsm, 2012-02-26 01:52:10]

Thanks, landed.

Opened http://code.google.com/p/dart/issues/detail?id=1858 for Dartium bugs.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
File client/dom/frog/dom_frog.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3007: Window get _contentWindow() native """
On 2012/02/24 03:38:30, sra1 wrote:
> For short ones that fit on the same line I just go with 'return xxx;';

Done.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3017: // TODO(vsm): Unify with Dartium version.
File added.

On 2012/02/24 03:38:30, sra1 wrote:
> The file(s) containing this function is missing from the CL, so I'm making
> comments here.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3018: class _DOMWindowCrossFrameJs implements
DOMType, DOMWindow {
On 2012/02/24 03:38:30, sra1 wrote:
> Drop the 'Js'.  That is for native classes.

Done.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3039: var get document() { throw new Exception();
}
I'm going to remove this and mark as a bug.

On 2012/02/24 03:38:30, sra1 wrote:
> This is weird.
> I wonder if it is related to the top level definition of get:document.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/frog/dom_...
client/dom/frog/dom_frog.dart:3054: void postMessage(Dynamic message, String
targetOrigin, [List messagePorts = null]) {
On 2012/02/24 03:38:30, sra1 wrote:
> Line length

Done.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/scripts/s...
File client/dom/scripts/systemfrog.py (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/dom/scripts/s...
client/dom/scripts/systemfrog.py:15: _overridden_members = set([
On 2012/02/24 03:38:30, sra1 wrote:
> Call this _dom_frog_omitted_members.
> With everyone currently doing import * the names need to be generator
specific.

Done, though we should consider making some of this general.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
File client/tests/client/dom/CrossFrameTest.dart (right):

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
client/tests/client/dom/CrossFrameTest.dart:13: // Test this field to ensure a
valid Dart wrapper.
On 2012/02/24 03:38:30, sra1 wrote:
> Since all native classes implement this, it is not distinguishing between the
> wrapper and a native object.
> Since dartObjectLocalStorages is unused unless the wrapping dart:html library
> has wrapped it, it will always be null.
> I want to get rid of dartObjectLocalStorage if possible after the direct
> dart:html lands.
> 
> Is there something else you can test?
> 
> If the issue is lack of access to private methods, I think it is OK for
dart:dom
> to export a public testing class like this:
> 
> class DOMTesting {
>   bool static isDOMWindowCrossFrame(object) => object is
__DOMWindowCrossFrame;
> }
> 
> Longer term the language needs to make it possible for tests to be in library
/
> package / whatever.

This is effectively verifying that the object belongs to this frame - i.e., it
inherits from this frame's Object proto.

Without window wrapper, this check failed as the window object was not decorated
with a dartObjectLocalStorage getter.

https://chromiumcodereview.appspot.com/9453004/diff/5001/client/tests/client/...
client/tests/client/dom/CrossFrameTest.dart:26: });
On 2012/02/24 15:31:24, antonm wrote:
> nit: -2 spaces, please

Done.