Skip to content

Support cross-frame dom objects #28326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vsmenon opened this issue Jan 10, 2017 · 7 comments
Closed

Support cross-frame dom objects #28326

vsmenon opened this issue Jan 10, 2017 · 7 comments
Assignees
Labels
dev-compiler-html P1 A high priority bug; for example, a single project is unusable or has many test failures web-dev-compiler

Comments

@vsmenon
Copy link
Member

vsmenon commented Jan 10, 2017

From @jacob314 on April 20, 2016 23:20

DDC support for cross-frame dom objects are incorrect as it depends on dart:js concepts of interceptors. Supporting this correctly will require a way to apply dart extension methods to a new child window when that window object is referenced through dart:html.

(edit by @jmesserly: split custom elements to dart-archive/dev_compiler#521)

Copied from original issue: dart-archive/dev_compiler#517

@vsmenon
Copy link
Member Author

vsmenon commented Jan 10, 2017

From @jmesserly on April 21, 2016 1:2

Supporting this correctly will require a way to apply dart extension methods to a new child window when that window object is referenced through dart:html.

I don't follow the relation to window object or how that comes into play?

@vsmenon
Copy link
Member Author

vsmenon commented Jan 10, 2017

From @jacob314 on April 21, 2016 15:57

dart:html has hooks where every time you have a dom method that returns a window you check whether the window is the window. if it is not the window you wrap it as a limited functionality cross-domain window so that you don't get access to cross-frame objects.
in dart2js this was mainly just to support dartium. in dev compiler, the extension method design means you would need to add a whole ton of extension methods to all objects from the window's context if you wanted them to play nicely with dev compiler.

@vsmenon vsmenon added web-dev-compiler P2 A bug or feature request we're likely to work on labels Jan 10, 2017
@vsmenon
Copy link
Member Author

vsmenon commented Jan 10, 2017

Right, the actual prototype objects are different in each iframe / window. So, the Window or HTMLCanvasElement protos in a frame are different from the ones on the top-level page. Same with native Array for that matter.

The dart.registerExtension commands we execute only affect the top-level context. We'd need to reapply those somehow to the new contexts (windows).

@vsmenon
Copy link
Member Author

vsmenon commented Jan 10, 2017

From @jmesserly on April 21, 2016 17:43

Sorry about that, I was super confused because I didn't see what "cross frame dom objects" had to do with "custom elements" ... I split custom elements into its own bug.

@ochafik
Copy link
Contributor

ochafik commented Jun 9, 2017

We actually hit this with GWT / JSNI code that "talks" to DDC code through the location history API:

public native void pushStateAndDispatchPopState(String url) /*-{
  $wnd.history.pushState(null, '', url);
  $wnd.dispatchEvent(new Event('popstate'));
}-*/;

Turns out GWT often loads its code in a trusted iframe (maybe only in devel?), so the Event class above is built inside the iframe, and reaches our DDC code in the top frame:

window.onPopState.listen((Event event) {
  // Explodes in flight with strong-mode cast failure, as event is not the right kind of Event.
  // ...
});

Our workaround is to change our GWT / JSNI code to ensure we dispatch a top-context Event:

  $wnd.dispatchEvent(new window.top.Event('popstate'));

cc/ @deshankbaranwal @leafpetersen

@vsmenon vsmenon added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Jun 29, 2017
@vsmenon vsmenon assigned vsmenon and unassigned jacob314 Jun 29, 2017
@vsmenon
Copy link
Member Author

vsmenon commented Jul 13, 2017

Here are two different approaches:

  1. A name-based approach: https://codereview.chromium.org/2982723002/

This is follows the dart2js strategy. If a js object prototype doesn't have a registered extension, look for one based on name. This will catch MouseEvent from another frame, but it'll also catch any user-js type called MouseEvent (again, I think consistent with dart2js).

  1. A registration-based approach: https://codereview.chromium.org/2980853002/

This installs on first access to a different frame (as suggested above). A variant on #2 would let users explicitly 'prepare' a frame too. This approach has the downside that a MouseEvent coming from a frame we don't now about won't be correctly recognized.

@jacob314 @jmesserly - thoughts?

Note, I still need to get tests working. The existing cross frame test doesn't work well with our karma framework. My local testing works with both approaches above.

vsmenon added a commit that referenced this issue Jul 19, 2017
This approach installs all Dart extension types onto a window whenever
that window is accessed.

See #28326

[email protected]

Review-Url: https://codereview.chromium.org/2980853002 .
vsmenon added a commit that referenced this issue Jul 24, 2017
This approach installs all Dart extension types onto a window whenever
that window is accessed.

See #28326

[email protected]

Review-Url: https://codereview.chromium.org/2980853002 .
@vsmenon
Copy link
Member Author

vsmenon commented Sep 25, 2017

We landed the registration-based approach and at least one app is using it successfully.

@vsmenon vsmenon closed this as completed Sep 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-compiler-html P1 A high priority bug; for example, a single project is unusable or has many test failures web-dev-compiler
Projects
None yet
Development

No branches or pull requests

3 participants