Skip to content

postMessage and other cross-frame operations are broken in dart2js #3175

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 May 22, 2012 · 5 comments
Closed

postMessage and other cross-frame operations are broken in dart2js #3175

vsmenon opened this issue May 22, 2012 · 5 comments
Assignees
Labels
closed-duplicate Closed in favor of an existing report P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@vsmenon
Copy link
Member

vsmenon commented May 22, 2012

Posting a message to another frame breaks today in dart2js. The problem is here in the generated code:

Isolate.$defineClass("_DOMWindowCrossFrameImpl", "Object", ["dartObjectLocalStorage", "_lib_window?"], {
 postMessage$3: function(message, targetOrigin, messagePorts) {
  if ($.eqNullB(messagePorts)) {
    this._lib_window.postMessage$2(message, targetOrigin);
  } else {
    this._lib_window.postMessage$3(message, targetOrigin, messagePorts);
  }
 },

The object "this._lib_window" is a Window object in a different frame - which means it is not affected by any patching we need. In particularly, it has no "postMessage$2" object.

This happens to work in Frog since the frog optimizer replaces postMessage$2 with postMessage at compile time.

Marking high as this is coming up in Dromaeo benchmarks.

@rakudrama
Copy link
Member

The cross-frame object can't really have any type since it is impossible to make an is-check work on it, or to have a type-checking.

We will have to generate code like (dart2js syntax):
   JS("void", "#.postMessage(#, #)", this._window, targetOrigin, messagePorts);

An alternative would be to have a new kind of native class somewhat like GWT's overlay types.
The type has no is-check, assignments just have to be believed, but it would permit simpler syntax for method calls, which would have to be always optimized like in frog.

@peter-ahe-google
Copy link
Contributor

As far as I can tell, this is the source from lib/html/frog/html_frog.dart:

  void postMessage(Dynamic message,
                   String targetOrigin,
                   [List messagePorts = null]) {
    if (messagePorts == null) {
      _window.postMessage(message, targetOrigin);
    } else {
      _window.postMessage(message, targetOrigin, messagePorts);
    }
  }

If _window is from a different frame, then it needs to be treated as a native object.

I'm also suspecting that calling the method with two different arguments wouldn't be necessary if this was ordinary Dart code. So this is another hint to me that this may actually be native code.

Based on these observations, I wonder if this would work:

  void postMessage(Dynamic message,
                   String targetOrigin,
                   [List messagePorts = null]) {
    if (messagePorts == null) {
      _pm3(_window, message, targetOrigin);
    } else {
      _pm4(_window, message, targetOrigin, messagePorts);
    }
  }

  _pm3(w, m, t) native "return w.postMessage(m, t);";

  _pm4(w, m, t, p) native "return w.postMessage(m, t, p);";

@rakudrama
Copy link
Member

Hi Peter,

That is what we are doing - we are treating _window like a handle without any members of its own. Small detail - _pm3 etc can be a static member.

We have a manually generated version for postMessage to get some basic Dromaeo tests unblocked, but we need to change the generator to emit the dozens of other methods to get full coverage.

@vsmenon
Copy link
Member Author

vsmenon commented May 29, 2012

We've checked in a temporary fix for postMessage along the lines Peter suggested to get that API working. We need to auto-generate something like this to enable other cross frame window / location / history methods.


Set owner to @vsmenon.
Added Accepted label.

@vsmenon
Copy link
Member Author

vsmenon commented Jun 22, 2012

Added Duplicate label.
Marked as being merged into #3836.

@vsmenon vsmenon added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures closed-duplicate Closed in favor of an existing report labels Jun 22, 2012
@vsmenon vsmenon self-assigned this Jun 22, 2012
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-duplicate Closed in favor of an existing report P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

3 participants