Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(118)

Unified Diff: sdk/lib/js/dartium/js_dartium.dart

Issue 1415463019: Make Dartium JS interop implementation more robust in the presence of JS paths that touch dart:html… (Closed) Base URL: git@github.com:dart-lang/sdk.git@master
Patch Set: Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | tests/html/js_typed_interop_test.dart » ('j') | tests/html/js_typed_interop_test.dart » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sdk/lib/js/dartium/js_dartium.dart
diff --git a/sdk/lib/js/dartium/js_dartium.dart b/sdk/lib/js/dartium/js_dartium.dart
index b01dce0bc7e242b4bfe24255314940440ad6f79c..83bfba90c02077390e7cda9d9eeb6af410af7cc3 100644
--- a/sdk/lib/js/dartium/js_dartium.dart
+++ b/sdk/lib/js/dartium/js_dartium.dart
@@ -310,8 +310,24 @@ String _getDeclarationName(mirrors.DeclarationMirror declaration) {
final _JS_LIBRARY_PREFIX = "js_library";
final _UNDEFINED_VAR = "_UNDEFINED_JS_CONST";
-String _accessJsPath(String path) =>
- "${_JS_LIBRARY_PREFIX}.context${path.split(".").map((p) => "['$p']").join('')}";
+String _accessJsPath(String path) {
+ var parts = path.split(".");
+ var sb = new StringBuffer();
+ sb
+ ..write('${_JS_LIBRARY_PREFIX}.JsNative.getProperty(' * parts.length)
+ ..write("${_JS_LIBRARY_PREFIX}.context");
+ for (var p in parts) {
+ sb.write(", '$p')");
+ }
+ return sb.toString();
+}
+
+
+String _accessJsPathSetter(String path) {
+ var parts = path.split(".");
+ return "${_JS_LIBRARY_PREFIX}.JsNative.setProperty(${_accessJsPath(parts.getRange(0, parts.length - 1).join('.'))
+ }, '{parts.end}', v)";
+}
@Deprecated("Internal Use Only")
void addMemberHelper(
@@ -331,9 +347,9 @@ void addMemberHelper(
}
sb.write(" ");
if (declaration.isGetter) {
- sb.write("get $name => ${_accessJsPath(path)};");
+ sb.write("get $name => ${_JS_LIBRARY_PREFIX}.maybeWrapTypedInterop(${_accessJsPath(path)});");
} else if (declaration.isSetter) {
- sb.write("set $name(v) => ${_accessJsPath(path)} = v;");
+ sb.write("set $name(v) => ${_JS_LIBRARY_PREFIX}.maybeWrapTypedInterop(${_accessJsPathSetter(path)});");
} else {
sb.write("$name(");
bool hasOptional = false;
@@ -362,6 +378,7 @@ void addMemberHelper(
}
// TODO(jacobr):
sb.write(") => ");
+ sb.write('${_JS_LIBRARY_PREFIX}.maybeWrapTypedInterop(');
if (declaration.isConstructor) {
sb.write("new ${_JS_LIBRARY_PREFIX}.JsObject(");
}
@@ -373,7 +390,7 @@ void addMemberHelper(
if (hasOptional) {
sb.write(".takeWhile((i) => i != ${_UNDEFINED_VAR}).toList()");
}
- sb.write(");");
+ sb.write("));");
}
sb.write("\n");
}
@@ -853,6 +870,10 @@ JsObject get context {
return _cachedContext;
}
+@Deprecated("Internal Use Only")
+maybeWrapTypedInterop(o) =>
+ html_common.wrap_jso_no_SerializedScriptvalue(o);
+
_maybeWrap(o) {
var wrapped = html_common.wrap_jso_no_SerializedScriptvalue(o);
if (identical(wrapped, o)) return o;
@@ -1102,7 +1123,7 @@ class JsObject extends NativeFieldWrapperClass2 {
throwError();
} else {
// TODO(jacobr): should we throw if the JavaScript object doesn't have the property?
- return this[name];
+ return maybeWrapTypedInterop(this._operator_getter(name));
}
} else if (invocation.isSetter) {
if (CHECK_JS_INVOCATIONS) {
@@ -1112,7 +1133,8 @@ class JsObject extends NativeFieldWrapperClass2 {
}
assert(name.endsWith("="));
name = name.substring(0, name.length - 1);
- return this[name] = invocation.positionalArguments.first;
+ return maybeWrapTypedInterop(_operator_setter(
+ name, invocation.positionalArguments.first));
} else {
// TODO(jacobr): also allow calling getters that look like functions.
var matches;
@@ -1121,7 +1143,7 @@ class JsObject extends NativeFieldWrapperClass2 {
if (matches == null ||
!matches.checkInvocation(invocation)) throwError();
}
- var ret = this.callMethod(name, _buildArgs(invocation));
+ var ret = maybeWrapTypedInterop(this._callMethod(name, _buildArgs(invocation)));
if (CHECK_JS_INVOCATIONS) {
if (!matches._checkReturnType(ret)) throwError();
}
@@ -1136,12 +1158,17 @@ class JsObject extends NativeFieldWrapperClass2 {
// Warning: this API is not exposed to dart:js.
@Deprecated("Internal Use Only")
class JsNative {
- static getProperty(JsObject o, name) {
- return o._operator_getter(name);
+ static getProperty(o, name) {
+ o = unwrap_jso(o);
+ return o != null ? o._operator_getter(name) : null;
+ }
+
+ static setProperty(o, name, value) {
+ return unwrap_jso(o)._operator_setter(name, value);
}
- static callMethod(JsObject o, String method, List args) {
- return o._callMethod(method, args);
+ static callMethod(o, String method, List args) {
+ return unwrap_jso(o)._callMethod(method, args);
}
static getArrayIndex(JsArray array, int index) {
« no previous file with comments | « no previous file | tests/html/js_typed_interop_test.dart » ('j') | tests/html/js_typed_interop_test.dart » ('J')

Powered by Google App Engine
This is Rietveld 408576698