[dart2js] size_estimator: take care with partially finalized names

ModularNames can be 'finalized' to Namer Names that are not
finalized. This matters only when using the size-estimator to inform
partitioning, which scans the JS ASTs before finalizing names related
to the structure of deferred-loaded parts (i.e. 'holders').

A better fix would be to have a clearer distinction between shallow
finalization (perhaps 'resolution') and deep finalization, but this
fix is less risky if we decide to cherry-pick.

Bug: #64150
Change-Id: Ib1f9be92f90764389c36b0a7ccf14295bf062490
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/543120
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Stephen Adams <sra@google.com>

pkg/compiler/lib/src/js/size_estimator.dart

diff --git a/pkg/compiler/lib/src/js/size_estimator.dart b/pkg/compiler/lib/src/js/size_estimator.dart
index edabb93..422faea 100644
--- a/pkg/compiler/lib/src/js/size_estimator.dart
+++ b/pkg/compiler/lib/src/js/size_estimator.dart
@@ -46,11 +46,6 @@
         node is VariableDeclaration ||
         node is VariableUse) {
       return nameSizeEstimate;
-    } else if (node is LiteralString) {
-      assert(!node.isFinalized);
-      // We assume all non-final literal strings are minified names, and thus
-      // use the nameSizeEstimate.
-      return nameSizeEstimate;
     } else if (node is BoundMetadataEntry) {
       // Value is an int.
       return '####';
@@ -75,11 +70,13 @@
   }
 
   String literalStringToString(LiteralString node) {
-    if (node.isFinalized) {
-      return node.value;
-    } else {
-      return sizeEstimate(node);
+    if (node is LiteralStringFromName) {
+      // The name may be finalized to a compound name with unfinalized parts.
+      // See https://dartbug.com/64150.
+      return nameSizeEstimate;
     }
+    assert(node.isFinalized); // Must be a plain LiteralString.
+    return node.value;
   }
 
   /// Always emit a newline, even under `enableMinification`.

pkg/compiler/test/js/js_size_estimator_test.dart

diff --git a/pkg/compiler/test/js/js_size_estimator_test.dart b/pkg/compiler/test/js/js_size_estimator_test.dart
index 512067e..85a5cce 100644
--- a/pkg/compiler/test/js/js_size_estimator_test.dart
+++ b/pkg/compiler/test/js/js_size_estimator_test.dart
@@ -6,6 +6,8 @@
 import 'dart:io';
 
 import 'package:expect/expect.dart';
+import 'package:compiler/src/common/codegen.dart'
+    show ModularName, ModularNameKind;
 import 'package:compiler/src/js/js.dart';
 import 'package:compiler/src/js/size_estimator.dart';
 
@@ -123,5 +125,14 @@
     ).writeAsStringSync(json.JsonEncoder.withIndent('  ').convert(newGoldens));
   } else {
     testGoldens(currentGoldens, testSuites);
+    testLiteralStringFromName();
   }
 }
+
+void testLiteralStringFromName() {
+  // Verifies that unfinalized LiteralStringFromName nodes (such as those
+  // wrapping ModularName) do not crash during size estimation (Issue 64150).
+  var name = ModularName(ModularNameKind.asName, data: 'unfinalizedName');
+  var literal = LiteralStringFromName(name);
+  Expect.equals(5, estimateSize(literal)); // '"' + '###' + '"'
+}