Fix ddc debugger_test

This CL:
- Fixes #43987 (an exception in custom formatting code)
- Restores (most of) debugger_test.dart to passing (which tests above)
- Fixes a type caching error also exposed by the above test

Note, this skips the golden file comparison.  That appears to be very broken
(see comment).

Change-Id: I283b66a710f17765faed47aa099b0b8570e6cac1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170022
Reviewed-by: Gary Roumanis <[email protected]>
Reviewed-by: Nicholas Shahan <[email protected]>
Commit-Queue: Vijay Menon <[email protected]>

Author: vsmenon

pkg/dev_compiler/lib/src/kernel/compiler.dart

@@ -581,11 +581,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
     var finishGenericTypeTest = _emitClassTypeTests(c, className, body);
 
-    // Attach caches on all canonicalized types not in our runtime.
-    // Types in the runtime will have caches attached in their constructors.
-    if (!isSdkInternalRuntime(_currentLibrary)) {
-      body.add(runtimeStatement('addTypeCaches(#)', [className]));
-    }
+    // Attach caches on all canonicalized types.
+    body.add(runtimeStatement('addTypeCaches(#)', [className]));
 
     _emitVirtualFieldSymbols(c, body);
     _emitClassSignature(c, className, body);

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/classes.dart

@@ -107,7 +107,11 @@ final mixinOn = JS('', 'Symbol("mixinOn")');
 @JSExportName('implements')
 final implements_ = JS('', 'Symbol("implements")');
 
-List? Function() getImplements(clazz) => JS(
+/// Either `null` if `clazz` doesn't directly implement any interfaces or a
+/// list of type objects if it does.  Note, indirectly (e.g., via superclass)
+/// implemented interfaces aren't included here.
+/// See compiler.dart for when/how it is emitted.
+List Function()? getImplements(clazz) => JS(
     '',
     'Object.hasOwnProperty.call(#, #) ? #[#] : null',
     clazz,

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart

@@ -2045,7 +2045,7 @@ Object? _getMatchingSupertype(Object? subtype, Object supertype) {
   // Check interfaces.
   var getInterfaces = getImplements(subtype);
   if (getInterfaces != null) {
-    for (var iface in getInterfaces()!) {
+    for (var iface in getInterfaces()) {
       result = _getMatchingSupertype(iface, supertype);
       if (result != null) return result;
     }

sdk/lib/_internal/js_dev_runtime/private/debugger.dart

@@ -264,7 +264,7 @@ class HeritageClause {
   final List types;
 }
 
-Object safeGetProperty(Object protoChain, Object name) {
+Object? safeGetProperty(Object protoChain, Object name) {
   try {
     return JSNative.getProperty(protoChain, name);
   } catch (e) {
@@ -609,7 +609,7 @@ class LibraryModuleFormatter implements Formatter {
     for (var name in getOwnPropertyNames(object)) {
       var value = safeGetProperty(object, name);
       children.add(NameValuePair(
-          name: name, value: Library(name, value), hideName: true));
+          name: name, value: Library(name, value!), hideName: true));
     }
     return children.toList();
   }
@@ -907,10 +907,10 @@ class ClassFormatter implements Formatter {
   bool accept(object, config) => config == JsonMLConfig.asClass;
 
   String preview(type) {
-    var implements = dart.getImplements(type)();
+    var implements = dart.getImplements(type);
     var typeName = getTypeName(type);
     if (implements != null) {
-      var typeNames = implements.map(getTypeName);
+      var typeNames = implements().map(getTypeName);
       return '${typeName} implements ${typeNames.join(", ")}';
     } else {
       return typeName;

tests/dartdevc/debugger/debugger_test.dart

@@ -81,7 +81,7 @@ replacer(String key, value) {
   return value;
 }
 
-String format(value) {
+String? format(value) {
   // Avoid double-escaping strings.
   if (value is String) return value;
   return stringify(value, allowInterop(replacer), 4);
@@ -90,8 +90,8 @@ String format(value) {
 class FormattedObject {
   FormattedObject(this.object, this.config);
 
-  Object object;
-  Object config;
+  Object? object;
+  Object? config;
 }
 
 /// Extract all object tags from a json ml expression to enable
@@ -130,14 +130,14 @@ main() async {
   var goldenUrl = '/root_dart/tests/dartdevc_2/debugger/'
       'debugger_test_golden.txt?cacheBlock=$cacheBlocker';
 
-  String golden;
+  String? golden;
   try {
     golden = (await HttpRequest.getString(goldenUrl)).trim();
   } catch (e) {
     print("Warning: couldn't load golden file from $goldenUrl");
   }
 
-  document.body.append(new ScriptElement()
+  document.body!.append(new ScriptElement()
     ..type = 'text/javascript'
     ..innerHtml = r"""
 window.ExampleJSClass = function ExampleJSClass(x) {
@@ -333,16 +333,16 @@ window.ExampleJSClass = function ExampleJSClass(x) {
       print(helpMessage);
       // Copy text to clipboard on page click. We can't copy to the clipboard
       // without a click due to Chrome security.
-      TextAreaElement textField = new Element.tag('textarea');
+      var textField = new Element.tag('textarea') as TextAreaElement;
       textField.maxLength = 100000000;
       textField.text = actualStr;
       textField.style
         ..width = '800px'
         ..height = '400px';
-      document.body.append(new Element.tag('h3')
+      document.body!.append(new Element.tag('h3')
         ..innerHtml = helpMessage.replaceAll('\n', '<br>'));
-      document.body.append(textField);
-      document.body.onClick.listen((_) {
+      document.body!.append(textField);
+      document.body!.onClick.listen((_) {
         textField.select();
         var result = document.execCommand('copy');
         if (result) {
@@ -352,6 +352,12 @@ window.ExampleJSClass = function ExampleJSClass(x) {
         }
       });
     }
-    expect(actualStr == golden, isTrue);
+    // TODO(vsm): This comparison appears to be badly broken for several
+    // reasons:
+    //   (1) The golden file isn't properly read on the bots or locally (see try
+    //       / catch above).
+    //   (2) The actual string appears to vary locally vs on the bots.
+    //   (3) Because of (2), visualizing the diff is difficult.
+    // expect(actualStr == golden, isTrue);
   });
 }

tests/dartdevc_2/debugger/debugger_test.dart

@@ -354,6 +354,12 @@ window.ExampleJSClass = function ExampleJSClass(x) {
         }
       });
     }
-    expect(actualStr == golden, isTrue);
+    // TODO(vsm): This comparison appears to be badly broken for several
+    // reasons:
+    //   (1) The golden file isn't properly read on the bots or locally (see try
+    //       / catch above).
+    //   (2) The actual string appears to vary locally vs on the bots.
+    //   (3) Because of (2), visualizing the diff is difficult.
+    // expect(actualStr == golden, isTrue);
   });
 }