Fix for issue 24243 (allow objects of dart:core and dart:collections to be passed in for now, we ne…

Fix for issue 24243 (allow objects of dart:core and dart:collections to be passed in for now, we need a whitelist of core types against which we need to check)

BUG=24243
R=koda@google.com

Committed: https://github.com/dart-lang/sdk/commit/807eacae309e5aadf23a6f7ca9ae5c605bb4b011

[siva, 2015-09-16 17:10:41]

asiva@google.com changed reviewers:
+ koda@google.com

[koda, 2015-09-16 17:41:26]

Thanks for addressing this issue.

https://codereview.chromium.org/1343213002/diff/40001/runtime/vm/snapshot.cc
File runtime/vm/snapshot.cc (right):

https://codereview.chromium.org/1343213002/diff/40001/runtime/vm/snapshot.cc#...
runtime/vm/snapshot.cc:2649: return (library ==
object_store()->collection_library() ||
I don't think the whitelisting of dart.collection is being exercised by your
tests, since both Map and List are in the dart.core library. You could simply
add a test with an explicit LinkedHashMap type argument.

https://codereview.chromium.org/1343213002/diff/40001/tests/isolate/issue_242...
File tests/isolate/issue_24243_child1_isolate.dart (right):

https://codereview.chromium.org/1343213002/diff/40001/tests/isolate/issue_242...
tests/isolate/issue_24243_child1_isolate.dart:14: var l = new
List<List<int>>.from([l0, l1, l2]);
Please avoid "l" as a name; it looks too much like "1" in some fonts.

https://codereview.chromium.org/1343213002/diff/40001/tests/isolate/issue_242...
File tests/isolate/issue_24243_parent_isolate_test.dart (right):

https://codereview.chromium.org/1343213002/diff/40001/tests/isolate/issue_242...
tests/isolate/issue_24243_parent_isolate_test.dart:12: import 'dart:async';
Sort imports.

[siva, 2015-09-16 18:24:45]

PTAL

https://codereview.chromium.org/1343213002/diff/40001/runtime/vm/snapshot.cc
File runtime/vm/snapshot.cc (right):

https://codereview.chromium.org/1343213002/diff/40001/runtime/vm/snapshot.cc#...
runtime/vm/snapshot.cc:2649: return (library ==
object_store()->collection_library() ||
On 2015/09/16 17:41:26, koda wrote:
> I don't think the whitelisting of dart.collection is being exercised by your
> tests, since both Map and List are in the dart.core library. You could simply
> add a test with an explicit LinkedHashMap type argument.

Added a new test with the type being expicitly set to a LinkedHashMap

https://codereview.chromium.org/1343213002/diff/40001/tests/isolate/issue_242...
File tests/isolate/issue_24243_child1_isolate.dart (right):

https://codereview.chromium.org/1343213002/diff/40001/tests/isolate/issue_242...
tests/isolate/issue_24243_child1_isolate.dart:14: var l = new
List<List<int>>.from([l0, l1, l2]);
On 2015/09/16 17:41:26, koda wrote:
> Please avoid "l" as a name; it looks too much like "1" in some fonts.

Done.

https://codereview.chromium.org/1343213002/diff/40001/tests/isolate/issue_242...
File tests/isolate/issue_24243_parent_isolate_test.dart (right):

https://codereview.chromium.org/1343213002/diff/40001/tests/isolate/issue_242...
tests/isolate/issue_24243_parent_isolate_test.dart:12: import 'dart:async';
On 2015/09/16 17:41:26, koda wrote:
> Sort imports.

Removed dart:async as it is not needed.

[koda, 2015-09-16 18:31:12]

lgtm

[siva, 2015-09-16 19:08:50]

Committed patchset #5 (id:80001) manually as
807eacae309e5aadf23a6f7ca9ae5c605bb4b011 (presubmit successful).