wrap_jso should wrap JsArray as well, not always call wrap on its contents

BUG=

Committed: f0e52e4

Review URL: https://codereview.chromium.org/1383903002 .

Author: alan-knight

sdk/lib/html/dartium/html_dartium.dart

@@ -1159,6 +1159,12 @@ wrap_jso(jsObject) {
       return wrapper;
     }
 
+    if (jsObject is js.JsArray) {
+      var wrappingList = new _DartHtmlWrappingList(jsObject);
+      js.setDartHtmlWrapperFor(jsObject, wrappingList);
+      return wrappingList;
+    }
+
     // Try the most general type conversions on it.
     // TODO(alanknight): We may be able to do better. This maintains identity,
     // which is useful, but expensive. And if we nest something that only
@@ -1304,6 +1310,22 @@ convertDartToNative_List(List input) => new js.JsArray()..addAll(input);
 // Conversion function place holder (currently not used in dart2js or dartium).
 List convertDartToNative_StringArray(List<String> input) => input;
 
+/**
+ * Wraps a JsArray and will call wrap_jso on its entries.
+ */
+class _DartHtmlWrappingList extends ListBase {
+  _DartHtmlWrappingList(this._basicList);
+
+  final js.JsArray _basicList;
+
+  operator [](int index) => wrap_jso(_basicList[index]);
+
+  operator []=(int index, value) => _basicList[index] = unwrap_jso(value);
+
+  int get length => _basicList.length;
+  int set length(int newLength) => _basicList.length = newLength;
+}
+
 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
@@ -10138,10 +10160,10 @@ class DirectoryReader extends NativeFieldWrapperClass2 {
 
   void _readEntries(_EntriesCallback successCallback, [_ErrorCallback errorCallback]) {
     if (errorCallback != null) {
-      _blink.BlinkDirectoryReader.instance.readEntries_Callback_2_(unwrap_jso(this), unwrap_jso((entries) => successCallback(entries)), unwrap_jso((error) => errorCallback(wrap_jso(error))));
+      _blink.BlinkDirectoryReader.instance.readEntries_Callback_2_(unwrap_jso(this), unwrap_jso((entries) => successCallback(wrap_jso(entries))), unwrap_jso((error) => errorCallback(wrap_jso(error))));
       return;
     }
-    _blink.BlinkDirectoryReader.instance.readEntries_Callback_1_(unwrap_jso(this), unwrap_jso((entries) => successCallback(entries)));
+    _blink.BlinkDirectoryReader.instance.readEntries_Callback_1_(unwrap_jso(this), unwrap_jso((entries) => successCallback(wrap_jso(entries))));
     return;
   }
 
@@ -18438,7 +18460,7 @@ class FontFaceSetLoadEvent extends Event {
   @DomName('FontFaceSetLoadEvent.fontfaces')
   @DocsEditable()
   @Experimental() // untriaged
-  List<FontFace> get fontfaces => _blink.BlinkFontFaceSetLoadEvent.instance.fontfaces_Getter_(unwrap_jso(this));
+  List<FontFace> get fontfaces => wrap_jso(_blink.BlinkFontFaceSetLoadEvent.instance.fontfaces_Getter_(unwrap_jso(this)));
 
 }
 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
@@ -18668,7 +18690,7 @@ class Gamepad extends NativeFieldWrapperClass2 {
 
   @DomName('Gamepad.axes')
   @DocsEditable()
-  List<num> get axes => _blink.BlinkGamepad.instance.axes_Getter_(unwrap_jso(this));
+  List<num> get axes => wrap_jso(_blink.BlinkGamepad.instance.axes_Getter_(unwrap_jso(this)));
 
   @DomName('Gamepad.connected')
   @DocsEditable()
@@ -22881,7 +22903,7 @@ class InputElement extends HtmlElement implements
   @SupportedBrowser(SupportedBrowser.SAFARI)
   @Experimental()
   // http://www.whatwg.org/specs/web-apps/current-work/multipage/states-of-the-type-attribute.html#concept-input-type-file-selected
-  List<Entry> get entries => _blink.BlinkHTMLInputElement.instance.webkitEntries_Getter_(unwrap_jso(this));
+  List<Entry> get entries => wrap_jso(_blink.BlinkHTMLInputElement.instance.webkitEntries_Getter_(unwrap_jso(this)));
 
   @DomName('HTMLInputElement.webkitdirectory')
   @DocsEditable()
@@ -26134,7 +26156,7 @@ class MessageEvent extends Event {
 
   @DomName('MessageEvent.initMessageEvent')
   @DocsEditable()
-  void _initMessageEvent(String typeArg, bool canBubbleArg, bool cancelableArg, Object dataArg, String originArg, String lastEventIdArg, Window sourceArg, List<MessagePort> messagePorts) => _blink.BlinkMessageEvent.instance.initMessageEvent_Callback_8_(unwrap_jso(this), typeArg, canBubbleArg, cancelableArg, dataArg, originArg, lastEventIdArg, unwrap_jso(sourceArg), messagePorts);
+  void _initMessageEvent(String typeArg, bool canBubbleArg, bool cancelableArg, Object dataArg, String originArg, String lastEventIdArg, Window sourceArg, List<MessagePort> messagePorts) => _blink.BlinkMessageEvent.instance.initMessageEvent_Callback_8_(unwrap_jso(this), typeArg, canBubbleArg, cancelableArg, dataArg, originArg, lastEventIdArg, unwrap_jso(sourceArg), unwrap_jso(messagePorts));
 
 }
 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
@@ -27240,7 +27262,7 @@ class MutationObserver extends NativeFieldWrapperClass2 {
   }
   @DocsEditable()
   static MutationObserver _create(callback) => wrap_jso(_blink.BlinkMutationObserver.instance.constructorCallback_1_((mutations, observer) {
-    callback(mutations, wrap_jso(observer));
+    callback(wrap_jso(mutations), wrap_jso(observer));
   }));
 
   /**

sdk/lib/js/dartium/js_dartium.dart

@@ -807,11 +807,7 @@ class JsArray<E> extends JsObject with ListMixin<E> {
       _checkIndex(index);
     }
 
-    // Lazily create the Dart class that wraps the JS object when the object in
-    // a list if fetched.
-    var wrap_entry = html.wrap_jso(super[index]);
-    super[index] = wrap_entry;
-    return wrap_entry;
+    return super[index];
   }
 
   void operator []=(index, E value) {

sdk/lib/web_sql/dartium/web_sql_dartium.dart

@@ -412,6 +412,6 @@ class SqlTransaction extends NativeFieldWrapperClass2 {
 
   @DomName('SQLTransaction.executeSql')
   @DocsEditable()
-  void executeSql(String sqlStatement, List<Object> arguments, [SqlStatementCallback callback, SqlStatementErrorCallback errorCallback]) => _blink.BlinkSQLTransaction.instance.executeSql_Callback_4_(unwrap_jso(this), sqlStatement, arguments, unwrap_jso((transaction, resultSet) => callback(wrap_jso(transaction), wrap_jso(resultSet))), unwrap_jso((transaction, error) => errorCallback(wrap_jso(transaction), wrap_jso(error))));
+  void executeSql(String sqlStatement, List<Object> arguments, [SqlStatementCallback callback, SqlStatementErrorCallback errorCallback]) => _blink.BlinkSQLTransaction.instance.executeSql_Callback_4_(unwrap_jso(this), sqlStatement, unwrap_jso(arguments), unwrap_jso((transaction, resultSet) => callback(wrap_jso(transaction), wrap_jso(resultSet))), unwrap_jso((transaction, error) => errorCallback(wrap_jso(transaction), wrap_jso(error))));
 
 }

tests/html/html.status

@@ -61,7 +61,6 @@ canvasrenderingcontext2d_test/drawImage_video_element_dataUrl: Pass,Fail # Issue
 [ $compiler == none && ($runtime == drt || $runtime == dartium || $runtime == ContentShellOnAndroid) ]
 # postMessage in dartium always transfers the typed array buffer, never a view
 postmessage_structured_test/typed_arrays: Fail
-postmessage_structured_test/primitives: RuntimeError # Bug 24432
 # Dartium seems to lose the data from the dispatchEvent.
 async_test: Fail # Uses spawn, not implemented from a DOM isolate in Dartium
 keyboard_event_test: Fail # Issue 13902

tests/html/wrapping_collections_test.dart

@@ -0,0 +1,26 @@
+library wrapping_collection_test;
+
+import 'dart:html';
+import 'dart:js' as js;
+import 'package:unittest/unittest.dart';
+import 'package:unittest/html_config.dart';
+
+/// Test that if we access objects through JS-interop we get the
+/// appropriate objects, even if dart:html maps them.
+main() {
+  test("Access through JS-interop", () {
+    var performance = js.context['performance'];
+    var entries = performance.callMethod('getEntries', const []);
+    entries.forEach((x) {
+        expect(x is js.JsObject, isTrue);
+    });
+  });
+
+  test("Access through dart:html", () {
+    var dartPerformance = wrap_jso(js.context['performance']);
+    var dartEntries = dartPerformance.getEntries();
+    dartEntries.forEach((x) {
+        expect(x is PerformanceEntry, isTrue);
+    });
+  });
+}

tools/dom/scripts/generator.py

@@ -515,7 +515,7 @@ def param_name(param_info):
     return ', '.join(map(param_name, self.param_infos[:parameter_count]))
 
   def isCallback(self, type_registry, type_id):
-    if type_id:
+    if type_id and not type_id.endswith('[]'):
       callback_type = type_registry._database._all_interfaces[type_id]
       return callback_type.operations[0].id == 'handleEvent' if len(callback_type.operations) > 0 else False
     else:
@@ -1446,7 +1446,8 @@ def wrap_unwrap_type_blink(return_type, type_registry):
             return_type == 'Future' or
             return_type == 'SqlDatabase' or # renamed to Database
             return_type == 'HTMLElement' or
-            return_type == 'MutationObserver')
+            return_type == 'MutationObserver' or
+            return_type.endswith('[]'))
 
 def wrap_type_blink(return_type, type_registry):
     """Returns True if the type is a blink type that requires wrap_jso but

tools/dom/templates/html/dartium/html_dartium.darttemplate

@@ -404,6 +404,12 @@ wrap_jso(jsObject) {
       return wrapper;
     }
 
+    if (jsObject is js.JsArray) {
+      var wrappingList = new _DartHtmlWrappingList(jsObject);
+      js.setDartHtmlWrapperFor(jsObject, wrappingList);
+      return wrappingList;
+    }
+
     // Try the most general type conversions on it.
     // TODO(alanknight): We may be able to do better. This maintains identity,
     // which is useful, but expensive. And if we nest something that only
@@ -549,6 +555,22 @@ convertDartToNative_List(List input) => new js.JsArray()..addAll(input);
 // Conversion function place holder (currently not used in dart2js or dartium).
 List convertDartToNative_StringArray(List<String> input) => input;
 
+/**
+ * Wraps a JsArray and will call wrap_jso on its entries.
+ */
+class _DartHtmlWrappingList extends ListBase {
+  _DartHtmlWrappingList(this._basicList);
+
+  final js.JsArray _basicList;
+
+  operator [](int index) => wrap_jso(_basicList[index]);
+
+  operator []=(int index, value) => _basicList[index] = unwrap_jso(value);
+
+  int get length => _basicList.length;
+  int set length(int newLength) => _basicList.length = newLength;
+}
+
 $else
 class JsoNativeFieldWrapper extends NativeFieldWrapperClass2 {}
 

tools/dom/templates/html/impl/impl_MutationObserver.darttemplate

@@ -22,7 +22,7 @@ $if DARTIUM
   @DocsEditable()
   $if JSINTEROP
   static MutationObserver _create(callback) => wrap_jso(_blink.BlinkMutationObserver.instance.constructorCallback_1_((mutations, observer) {
-    callback(mutations, wrap_jso(observer));
+    callback(wrap_jso(mutations), wrap_jso(observer));
   }));
   $else
   static MutationObserver _create(callback) => _blink.BlinkMutationObserver.instance.constructorCallback_1_(callback);