From e04252671ae57c019abd9ceaf935ba070c11c475 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Fri, 12 Oct 2018 19:04:23 +0200
Subject: [PATCH 01/26] Add Locale.fromComponents.

---
 lib/ui/window.dart            | 93 ++++++++++++++++++++++++++++++-----
 testing/dart/locale_test.dart | 24 +++++++++
 2 files changed, 106 insertions(+), 11 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 0b1318c997aaf..18ca26d7ee729 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -150,7 +150,37 @@ class Locale {
   /// Registry](https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry).
   /// Typically this means the primary language subtag should be lowercase and
   /// the region subtag should be uppercase.
-  const Locale(this._languageCode, [ this._countryCode ]) : assert(_languageCode != null);
+  const Locale(
+    this._languageCode, [
+    this._countryCode,
+  ]) : assert(_languageCode != null),
+       this.scriptCode = null,
+       this._variants = null;
+
+  /// Creates a new Locale object.
+  ///
+  /// The keyword arguments specify the subtags of the Locale. The subtag values
+  /// should match the values listed as valid in Unicode CLDR supplemental data:
+  /// [language](http://unicode.org/cldr/latest/common/validity/language.xml),
+  /// [script](http://unicode.org/cldr/latest/common/validity/script.xml),
+  /// [region](http://unicode.org/cldr/latest/common/validity/region.xml) and
+  /// [variant](http://unicode.org/cldr/latest/common/validity/variant.xml).
+  /// There may be multiple variants as long as they appear in sorted order and
+  /// are joined by hyphens.
+  const Locale.fromComponents({
+    String language,
+    String script,
+    String region,
+    String variants,
+  }) : assert(language == null || (language.length >= 2 && language.length <= 8
+                                   && language.length != 4)),
+       assert(script == null || script.length == 4),
+       assert(region == null || (region.length >= 2 && region.length <= 3)),
+       assert(variants == null || variants.length >= 4),
+       _languageCode = language ?? 'und',
+       scriptCode = script,
+       _countryCode = region,
+       _variants = variants;
 
   /// The primary language subtag for the locale.
   ///
@@ -166,10 +196,10 @@ class Locale {
   /// Locale('he')` and `const Locale('iw')` are equal, and both have the
   /// [languageCode] `he`, because `iw` is a deprecated language subtag that was
   /// replaced by the subtag `he`.
-  String get languageCode => _canonicalizeLanguageCode(_languageCode);
+  String get languageCode => _replaceDeprecatedLanguageSubtag(_languageCode);
   final String _languageCode;
 
-  static String _canonicalizeLanguageCode(String languageCode) {
+  static String _replaceDeprecatedLanguageSubtag(String languageCode) {
     // This switch statement is generated by //flutter/tools/gen_locale.dart
     // Mappings generated for language subtag registry as of 2018-08-08.
     switch (languageCode) {
@@ -255,6 +285,16 @@ class Locale {
     }
   }
 
+  /// The script subtag for the locale.
+  ///
+  /// This can be null.
+  ///
+  /// This is expected to be a valid Unicode Language Identifier script subtag
+  /// that is listed in Unicode CLDR supplemental data:
+  /// http://unicode.org/cldr/latest/common/validity/script.xml. Please see
+  /// constructor documentation.
+  final String scriptCode;
+
   /// The region subtag for the locale.
   ///
   /// This can be null.
@@ -269,10 +309,10 @@ class Locale {
   /// 'DE')` and `const Locale('de', 'DD')` are equal, and both have the
   /// [countryCode] `DE`, because `DD` is a deprecated language subtag that was
   /// replaced by the subtag `DE`.
-  String get countryCode => _canonicalizeRegionCode(_countryCode);
+  String get countryCode => _replaceDeprecatedRegionSubtag(_countryCode);
   final String _countryCode;
 
-  static String _canonicalizeRegionCode(String regionCode) {
+  static String _replaceDeprecatedRegionSubtag(String regionCode) {
     // This switch statement is generated by //flutter/tools/gen_locale.dart
     // Mappings generated for language subtag registry as of 2018-08-08.
     switch (regionCode) {
@@ -286,6 +326,29 @@ class Locale {
     }
   }
 
+  /// The variants subtags for the locale.
+  ///
+  /// Each subtag is expected to be a valid Unicode Language Identifier variant
+  /// subtag that is listed in Unicode CLDR supplemental data:
+  /// http://unicode.org/cldr/latest/common/validity/variants.xml. Please see
+  /// constructor documentation.
+  Iterable<String> get variants {
+    if (_variants == null)
+      return const <String>[];
+    return _variants.split('-');
+  }
+
+  /// Hyphen-separated variants subtags for the locale.
+  ///
+  /// This can be null.
+  ///
+  /// This is expected to be a hyphen-separated sorted list of valid Unicode
+  /// Language Identifier variant subtags that are listed in Unicode CLDR
+  /// supplemental data:
+  /// http://unicode.org/cldr/latest/common/validity/variants.xml. Please see
+  /// constructor documentation.
+  final String _variants;
+
   @override
   bool operator ==(dynamic other) {
     if (identical(this, other))
@@ -294,23 +357,31 @@ class Locale {
       return false;
     final Locale typedOther = other;
     return languageCode == typedOther.languageCode
-        && countryCode == typedOther.countryCode;
+        && scriptCode == typedOther.scriptCode
+        && countryCode == typedOther.countryCode
+        && _variants == typedOther._variants;
   }
 
   @override
   int get hashCode {
     int result = 373;
     result = 37 * result + languageCode.hashCode;
-    if (_countryCode != null)
-      result = 37 * result + countryCode.hashCode;
+    result = 37 * result + scriptCode.hashCode;
+    result = 37 * result + countryCode.hashCode;
+    result = 37 * result + _variants.hashCode;
     return result;
   }
 
   @override
   String toString() {
-    if (_countryCode == null)
-      return languageCode;
-    return '${languageCode}_$countryCode';
+    final StringBuffer out = StringBuffer(languageCode);
+    if (scriptCode != null)
+      out.write('_$scriptCode');
+    if (_countryCode != null)
+      out.write('_$countryCode');
+    if (_variants != null)
+      out.write('_${_variants.replaceAll('-', '_')}');
+    return out.toString();
   }
 }
 
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 60a7bf99fe2af..c5b85bc70aa32 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -19,4 +19,28 @@ void main() {
     expect(const Locale('iw', 'DD').toString(), 'he_DE');
     expect(const Locale('iw', 'DD'), const Locale('he', 'DE'));
   });
+
+  test('Locale.fromComponents', () {
+    expect(
+      const Locale.fromComponents(language: 'en').toString(),
+      'en',
+    );
+    expect(
+      const Locale.fromComponents(script: 'Latn').toString(),
+      'und_Latn',
+    );
+    expect(
+      const Locale.fromComponents(region: 'US').toString(),
+      'und_US',
+    );
+    expect(
+      const Locale.fromComponents(variants: 'fonipa').toString(),
+      'und_fonipa',
+    );
+    expect(
+      const Locale.fromComponents(language: 'zh', script: 'Hans', region: 'CN')
+          .toString(),
+      'zh_Hans_CN',
+    );
+  });
 }

From 401360044f8a85ebbe7e8d77a15009b1d2d4768b Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Fri, 12 Oct 2018 19:25:10 +0200
Subject: [PATCH 02/26] Change toString from underscores to dashes. Expand the
 unit tests.

---
 lib/ui/window.dart            |  6 ++---
 testing/dart/locale_test.dart | 48 ++++++++++++++++++-----------------
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 18ca26d7ee729..5840b01d0d482 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -376,11 +376,11 @@ class Locale {
   String toString() {
     final StringBuffer out = StringBuffer(languageCode);
     if (scriptCode != null)
-      out.write('_$scriptCode');
+      out.write('-$scriptCode');
     if (_countryCode != null)
-      out.write('_$countryCode');
+      out.write('-$countryCode');
     if (_variants != null)
-      out.write('_${_variants.replaceAll('-', '_')}');
+      out.write('-$_variants');
     return out.toString();
   }
 }
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index c5b85bc70aa32..7ae5770ec0995 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -14,33 +14,35 @@ void main() {
     expect(const Locale('en').hashCode, new Locale('en', $null).hashCode);
     expect(const Locale('en'), isNot(new Locale('en', '')));
     expect(const Locale('en').hashCode, isNot(new Locale('en', '').hashCode));
-    expect(const Locale('en', 'US').toString(), 'en_US');
+    expect(const Locale('en', 'US').toString(), 'en-US');
     expect(const Locale('iw').toString(), 'he');
-    expect(const Locale('iw', 'DD').toString(), 'he_DE');
+    expect(const Locale('iw', 'DD').toString(), 'he-DE');
     expect(const Locale('iw', 'DD'), const Locale('he', 'DE'));
   });
 
   test('Locale.fromComponents', () {
-    expect(
-      const Locale.fromComponents(language: 'en').toString(),
-      'en',
-    );
-    expect(
-      const Locale.fromComponents(script: 'Latn').toString(),
-      'und_Latn',
-    );
-    expect(
-      const Locale.fromComponents(region: 'US').toString(),
-      'und_US',
-    );
-    expect(
-      const Locale.fromComponents(variants: 'fonipa').toString(),
-      'und_fonipa',
-    );
-    expect(
-      const Locale.fromComponents(language: 'zh', script: 'Hans', region: 'CN')
-          .toString(),
-      'zh_Hans_CN',
-    );
+    expect(const Locale.fromComponents().languageCode, 'und');
+    expect(const Locale.fromComponents().scriptCode, null);
+    expect(const Locale.fromComponents().countryCode, null);
+    expect(const Locale.fromComponents().variants, orderedEquals([]));
+
+    expect(const Locale.fromComponents(language: 'en').toString(), 'en');
+    expect(const Locale.fromComponents(language: 'en').languageCode, 'en');
+    expect(const Locale.fromComponents(script: 'Latn').toString(), 'und-Latn');
+    expect(const Locale.fromComponents(script: 'Latn').scriptCode, 'Latn');
+    expect(const Locale.fromComponents(region: 'US').toString(), 'und-US');
+    expect(const Locale.fromComponents(region: 'US').countryCode, 'US');
+    expect(const Locale.fromComponents(variants: 'fonipa-scouse').variants,
+           orderedEquals(['fonipa', 'scouse']));
+
+    expect(Locale.fromComponents(language: 'es', region: '419').toString(), 'es-419');
+    expect(Locale.fromComponents(language: 'es', region: '419').languageCode, 'es');
+    expect(Locale.fromComponents(language: 'es', region: '419').countryCode, '419');
+    expect(Locale.fromComponents(script: 'Latn', variants: 'fonipa').toString(), 'und-Latn-fonipa');
+    expect(Locale.fromComponents(script: 'Latn', variants: 'fonipa').scriptCode, 'Latn');
+    expect(Locale.fromComponents(script: 'Latn', variants: 'fonipa').variants,
+           orderedEquals(['fonipa']));
+
+    expect(Locale.fromComponents(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh-Hans-CN');
   });
 }

From 813998dac2378d7d2446945246ea5b87f3d1f5c2 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Mon, 15 Oct 2018 12:47:26 +0200
Subject: [PATCH 03/26] Rename 'fromComponents' to 'create'. Change variants
 from String to List<String>.

---
 lib/ui/window.dart            | 20 +++++++++---------
 testing/dart/locale_test.dart | 38 +++++++++++++++++------------------
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 5840b01d0d482..82c08fc5e60e1 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -164,19 +164,19 @@ class Locale {
   /// [language](http://unicode.org/cldr/latest/common/validity/language.xml),
   /// [script](http://unicode.org/cldr/latest/common/validity/script.xml),
   /// [region](http://unicode.org/cldr/latest/common/validity/region.xml) and
-  /// [variant](http://unicode.org/cldr/latest/common/validity/variant.xml).
-  /// There may be multiple variants as long as they appear in sorted order and
-  /// are joined by hyphens.
-  const Locale.fromComponents({
+  /// [variant](http://unicode.org/cldr/latest/common/validity/variant.xml). If
+  /// there is more than one variant, they should be in sorted order. This list
+  /// will be used as-is, and should never again be modified.
+  const Locale.create({
     String language,
     String script,
     String region,
-    String variants,
+    List<String> variants,
   }) : assert(language == null || (language.length >= 2 && language.length <= 8
                                    && language.length != 4)),
        assert(script == null || script.length == 4),
        assert(region == null || (region.length >= 2 && region.length <= 3)),
-       assert(variants == null || variants.length >= 4),
+       assert(variants == null || variants[0].length >= 4),
        _languageCode = language ?? 'und',
        scriptCode = script,
        _countryCode = region,
@@ -335,10 +335,10 @@ class Locale {
   Iterable<String> get variants {
     if (_variants == null)
       return const <String>[];
-    return _variants.split('-');
+    return _variants;
   }
 
-  /// Hyphen-separated variants subtags for the locale.
+  /// Variants subtags for the locale.
   ///
   /// This can be null.
   ///
@@ -347,7 +347,7 @@ class Locale {
   /// supplemental data:
   /// http://unicode.org/cldr/latest/common/validity/variants.xml. Please see
   /// constructor documentation.
-  final String _variants;
+  final List<String> _variants;
 
   @override
   bool operator ==(dynamic other) {
@@ -380,7 +380,7 @@ class Locale {
     if (_countryCode != null)
       out.write('-$countryCode');
     if (_variants != null)
-      out.write('-$_variants');
+      out.write('-${_variants.join("-")}');
     return out.toString();
   }
 }
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 7ae5770ec0995..35382dca76391 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -20,29 +20,29 @@ void main() {
     expect(const Locale('iw', 'DD'), const Locale('he', 'DE'));
   });
 
-  test('Locale.fromComponents', () {
-    expect(const Locale.fromComponents().languageCode, 'und');
-    expect(const Locale.fromComponents().scriptCode, null);
-    expect(const Locale.fromComponents().countryCode, null);
-    expect(const Locale.fromComponents().variants, orderedEquals([]));
+  test('Locale.create', () {
+    expect(const Locale.create().languageCode, 'und');
+    expect(const Locale.create().scriptCode, null);
+    expect(const Locale.create().countryCode, null);
+    expect(const Locale.create().variants, orderedEquals([]));
 
-    expect(const Locale.fromComponents(language: 'en').toString(), 'en');
-    expect(const Locale.fromComponents(language: 'en').languageCode, 'en');
-    expect(const Locale.fromComponents(script: 'Latn').toString(), 'und-Latn');
-    expect(const Locale.fromComponents(script: 'Latn').scriptCode, 'Latn');
-    expect(const Locale.fromComponents(region: 'US').toString(), 'und-US');
-    expect(const Locale.fromComponents(region: 'US').countryCode, 'US');
-    expect(const Locale.fromComponents(variants: 'fonipa-scouse').variants,
+    expect(const Locale.create(language: 'en').toString(), 'en');
+    expect(const Locale.create(language: 'en').languageCode, 'en');
+    expect(const Locale.create(script: 'Latn').toString(), 'und-Latn');
+    expect(const Locale.create(script: 'Latn').scriptCode, 'Latn');
+    expect(const Locale.create(region: 'US').toString(), 'und-US');
+    expect(const Locale.create(region: 'US').countryCode, 'US');
+    expect(const Locale.create(variants: ['fonipa', 'scouse']).variants,
            orderedEquals(['fonipa', 'scouse']));
 
-    expect(Locale.fromComponents(language: 'es', region: '419').toString(), 'es-419');
-    expect(Locale.fromComponents(language: 'es', region: '419').languageCode, 'es');
-    expect(Locale.fromComponents(language: 'es', region: '419').countryCode, '419');
-    expect(Locale.fromComponents(script: 'Latn', variants: 'fonipa').toString(), 'und-Latn-fonipa');
-    expect(Locale.fromComponents(script: 'Latn', variants: 'fonipa').scriptCode, 'Latn');
-    expect(Locale.fromComponents(script: 'Latn', variants: 'fonipa').variants,
+    expect(Locale.create(language: 'es', region: '419').toString(), 'es-419');
+    expect(Locale.create(language: 'es', region: '419').languageCode, 'es');
+    expect(Locale.create(language: 'es', region: '419').countryCode, '419');
+    expect(Locale.create(script: 'Latn', variants: ['fonipa']).toString(), 'und-Latn-fonipa');
+    expect(Locale.create(script: 'Latn', variants: ['fonipa']).scriptCode, 'Latn');
+    expect(Locale.create(script: 'Latn', variants: ['fonipa']).variants,
            orderedEquals(['fonipa']));
 
-    expect(Locale.fromComponents(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh-Hans-CN');
+    expect(Locale.create(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh-Hans-CN');
   });
 }

From fe5e1104b41ac520da3f74b454de19495f7e8a3e Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Mon, 15 Oct 2018 14:57:50 +0200
Subject: [PATCH 04/26] Use default for language parameter. Use
 hashCode/hashList.

---
 lib/ui/window.dart | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 82c08fc5e60e1..1ce37224bcb7e 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -168,7 +168,7 @@ class Locale {
   /// there is more than one variant, they should be in sorted order. This list
   /// will be used as-is, and should never again be modified.
   const Locale.create({
-    String language,
+    String language = 'und',
     String script,
     String region,
     List<String> variants,
@@ -177,7 +177,7 @@ class Locale {
        assert(script == null || script.length == 4),
        assert(region == null || (region.length >= 2 && region.length <= 3)),
        assert(variants == null || variants[0].length >= 4),
-       _languageCode = language ?? 'und',
+       _languageCode = language,
        scriptCode = script,
        _countryCode = region,
        _variants = variants;
@@ -363,14 +363,7 @@ class Locale {
   }
 
   @override
-  int get hashCode {
-    int result = 373;
-    result = 37 * result + languageCode.hashCode;
-    result = 37 * result + scriptCode.hashCode;
-    result = 37 * result + countryCode.hashCode;
-    result = 37 * result + _variants.hashCode;
-    return result;
-  }
+  int get hashCode => hashValues(languageCode, scriptCode, countryCode, hashList(_variants));
 
   @override
   String toString() {

From fb72be0fd7492f1aaa6f6ace2b7059a83328152e Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Mon, 15 Oct 2018 15:14:30 +0200
Subject: [PATCH 05/26] Have toString() stick with old (underscore) behaviour.

---
 lib/ui/window.dart            |  6 +++---
 testing/dart/locale_test.dart | 14 +++++++-------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 1ce37224bcb7e..e42139d02fe0b 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -369,11 +369,11 @@ class Locale {
   String toString() {
     final StringBuffer out = StringBuffer(languageCode);
     if (scriptCode != null)
-      out.write('-$scriptCode');
+      out.write('_$scriptCode');
     if (_countryCode != null)
-      out.write('-$countryCode');
+      out.write('_$countryCode');
     if (_variants != null)
-      out.write('-${_variants.join("-")}');
+      out.write('_${_variants.join("_")}');
     return out.toString();
   }
 }
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 35382dca76391..c5a64056140ec 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -14,9 +14,9 @@ void main() {
     expect(const Locale('en').hashCode, new Locale('en', $null).hashCode);
     expect(const Locale('en'), isNot(new Locale('en', '')));
     expect(const Locale('en').hashCode, isNot(new Locale('en', '').hashCode));
-    expect(const Locale('en', 'US').toString(), 'en-US');
+    expect(const Locale('en', 'US').toString(), 'en_US');
     expect(const Locale('iw').toString(), 'he');
-    expect(const Locale('iw', 'DD').toString(), 'he-DE');
+    expect(const Locale('iw', 'DD').toString(), 'he_DE');
     expect(const Locale('iw', 'DD'), const Locale('he', 'DE'));
   });
 
@@ -28,21 +28,21 @@ void main() {
 
     expect(const Locale.create(language: 'en').toString(), 'en');
     expect(const Locale.create(language: 'en').languageCode, 'en');
-    expect(const Locale.create(script: 'Latn').toString(), 'und-Latn');
+    expect(const Locale.create(script: 'Latn').toString(), 'und_Latn');
     expect(const Locale.create(script: 'Latn').scriptCode, 'Latn');
-    expect(const Locale.create(region: 'US').toString(), 'und-US');
+    expect(const Locale.create(region: 'US').toString(), 'und_US');
     expect(const Locale.create(region: 'US').countryCode, 'US');
     expect(const Locale.create(variants: ['fonipa', 'scouse']).variants,
            orderedEquals(['fonipa', 'scouse']));
 
-    expect(Locale.create(language: 'es', region: '419').toString(), 'es-419');
+    expect(Locale.create(language: 'es', region: '419').toString(), 'es_419');
     expect(Locale.create(language: 'es', region: '419').languageCode, 'es');
     expect(Locale.create(language: 'es', region: '419').countryCode, '419');
-    expect(Locale.create(script: 'Latn', variants: ['fonipa']).toString(), 'und-Latn-fonipa');
+    expect(Locale.create(script: 'Latn', variants: ['fonipa']).toString(), 'und_Latn_fonipa');
     expect(Locale.create(script: 'Latn', variants: ['fonipa']).scriptCode, 'Latn');
     expect(Locale.create(script: 'Latn', variants: ['fonipa']).variants,
            orderedEquals(['fonipa']));
 
-    expect(Locale.create(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh-Hans-CN');
+    expect(Locale.create(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh_Hans_CN');
   });
 }

From c6a8b96680e73ba90a7923ae0b24f855cf304ce8 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Mon, 15 Oct 2018 15:44:24 +0200
Subject: [PATCH 06/26] Demonstrate empty-list bug in assert code.

---
 testing/dart/locale_test.dart | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index c5a64056140ec..f0fd6fba4179e 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -32,6 +32,8 @@ void main() {
     expect(const Locale.create(script: 'Latn').scriptCode, 'Latn');
     expect(const Locale.create(region: 'US').toString(), 'und_US');
     expect(const Locale.create(region: 'US').countryCode, 'US');
+    expect(const Locale.create(variants: []).variants,
+           orderedEquals([]));
     expect(const Locale.create(variants: ['fonipa', 'scouse']).variants,
            orderedEquals(['fonipa', 'scouse']));
 

From af40b723aa0bee761620bda1ba0bcbaae74ab2c8 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Mon, 15 Oct 2018 15:45:09 +0200
Subject: [PATCH 07/26] Fix empty-list assert bug.

---
 lib/ui/window.dart | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index e42139d02fe0b..04fa06d1488bc 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -176,7 +176,7 @@ class Locale {
                                    && language.length != 4)),
        assert(script == null || script.length == 4),
        assert(region == null || (region.length >= 2 && region.length <= 3)),
-       assert(variants == null || variants[0].length >= 4),
+       assert(variants == null || variants.length == 0 || variants[0].length >= 4),
        _languageCode = language,
        scriptCode = script,
        _countryCode = region,

From 488cf31e88b364080fc6037fb6fb8c505d5d5ed8 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Mon, 15 Oct 2018 20:10:09 +0200
Subject: [PATCH 08/26] Add ignores for lint issues. Unsure about 71340 though.

---
 lib/ui/window.dart | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 04fa06d1488bc..42069f3cfafd6 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -176,9 +176,9 @@ class Locale {
                                    && language.length != 4)),
        assert(script == null || script.length == 4),
        assert(region == null || (region.length >= 2 && region.length <= 3)),
-       assert(variants == null || variants.length == 0 || variants[0].length >= 4),
+       assert(variants == null || variants.length == 0 || variants[0].length >= 4), // ignore: prefer_is_empty, const_eval_type_bool_num_string, https://github.com/dart-lang/sdk/issues/34798, https://dart-review.googlesource.com/c/sdk/+/71340
        _languageCode = language,
-       scriptCode = script,
+       scriptCode = script,  // ignore: prefer_initializing_formals, https://github.com/dart-lang/sdk/issues/34797
        _countryCode = region,
        _variants = variants;
 

From 829af355b4dfa6b5a85373e2577d693ebe6b8c1e Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Tue, 16 Oct 2018 10:20:23 +0200
Subject: [PATCH 09/26] Fix operator== via _listEquals.

---
 lib/ui/window.dart            | 14 +++++++++++++-
 testing/dart/locale_test.dart | 11 +++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 42069f3cfafd6..d7973ddbeb6bd 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -359,7 +359,19 @@ class Locale {
     return languageCode == typedOther.languageCode
         && scriptCode == typedOther.scriptCode
         && countryCode == typedOther.countryCode
-        && _variants == typedOther._variants;
+        && _listEquals<String>(_variants, typedOther._variants);
+  }
+
+  bool _listEquals<T>(List<T> a, List<T> b) {
+    if (a == null)
+      return b == null;
+    if (b == null || a.length != b.length)
+      return false;
+    for (int index = 0; index < a.length; index += 1) {
+      if (a[index] != b[index])
+        return false;
+    }
+    return true;
   }
 
   @override
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index f0fd6fba4179e..6c1e7f4a8bd53 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -47,4 +47,15 @@ void main() {
 
     expect(Locale.create(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh_Hans_CN');
   });
+
+  test('Locale equality', () {
+    expect(Locale.create(language: 'en', variants: ['fonipa']),
+           Locale.create(language: 'en', variants: ['fonipa']));
+    expect(Locale.create(language: 'en', variants: ['fonipa']).hashCode,
+           Locale.create(language: 'en', variants: ['fonipa']).hashCode);
+    expect(Locale.create(language: 'en'),
+           isNot(Locale.create(language: 'en', script: 'Latn')));
+    expect(Locale.create(language: 'en').hashCode,
+           isNot(Locale.create(language: 'en', script: 'Latn').hashCode));
+  });
 }

From c749663dbb1d82ac3e4fb1c1277ac626ebb859de Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Tue, 16 Oct 2018 10:21:56 +0200
Subject: [PATCH 10/26] Remove length-checking asserts: we're anyway not
 checking characters in fields.

---
 lib/ui/window.dart | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index d7973ddbeb6bd..031200e812258 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -172,12 +172,7 @@ class Locale {
     String script,
     String region,
     List<String> variants,
-  }) : assert(language == null || (language.length >= 2 && language.length <= 8
-                                   && language.length != 4)),
-       assert(script == null || script.length == 4),
-       assert(region == null || (region.length >= 2 && region.length <= 3)),
-       assert(variants == null || variants.length == 0 || variants[0].length >= 4), // ignore: prefer_is_empty, const_eval_type_bool_num_string, https://github.com/dart-lang/sdk/issues/34798, https://dart-review.googlesource.com/c/sdk/+/71340
-       _languageCode = language,
+  }) : _languageCode = language,
        scriptCode = script,  // ignore: prefer_initializing_formals, https://github.com/dart-lang/sdk/issues/34797
        _countryCode = region,
        _variants = variants;

From 04c52e95858a4d28e007b7dbc554170cd25f1aeb Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Tue, 16 Oct 2018 11:07:58 +0200
Subject: [PATCH 11/26] Documentation update.

---
 lib/ui/window.dart | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 031200e812258..973aff5eac1ab 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -116,9 +116,9 @@ class WindowPadding {
   }
 }
 
-/// An identifier used to select a user's language and formatting preferences,
-/// consisting of a language and a country. This is a subset of locale
-/// identifiers as defined by BCP 47.
+/// An identifier used to select a user's language and formatting preferences.
+/// This represents a [Unicode Language
+/// Identifier](https://www.unicode.org/reports/tr35/#Unicode_language_identifier).
 ///
 /// Locales are canonicalized according to the "preferred value" entries in the
 /// [IANA Language Subtag
@@ -145,11 +145,20 @@ class Locale {
   /// The primary language subtag must not be null. The region subtag is
   /// optional.
   ///
-  /// The values are _case sensitive_, and should match the case of the relevant
-  /// subtags in the [IANA Language Subtag
-  /// Registry](https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry).
-  /// Typically this means the primary language subtag should be lowercase and
-  /// the region subtag should be uppercase.
+  /// The subtag values are _case sensitive_ and should be one of the valid
+  /// subtags according to CLDR supplemental data:
+  /// [language](http://unicode.org/cldr/latest/common/validity/language.xml),
+  /// [region](http://unicode.org/cldr/latest/common/validity/region.xml). The
+  /// primary language subtag should be two to three lowercase letters. The
+  /// region region subtag should be two uppercase letters or three digits. See
+  /// the [Unicode Language
+  /// Identifier](https://www.unicode.org/reports/tr35/#Unicode_language_identifier)
+  /// specification.
+  ///
+  /// Validity is not checked by default, but some methods may throw away
+  /// invalid data.
+  ///
+  /// See also: [Locale.create].
   const Locale(
     this._languageCode, [
     this._countryCode,
@@ -159,14 +168,19 @@ class Locale {
 
   /// Creates a new Locale object.
   ///
-  /// The keyword arguments specify the subtags of the Locale. The subtag values
-  /// should match the values listed as valid in Unicode CLDR supplemental data:
+  /// The keyword arguments specify the subtags of the Locale.
+  ///
+  /// The subtag values are _case sensitive_ and should be valid subtags
+  /// according to CLDR supplemental data:
   /// [language](http://unicode.org/cldr/latest/common/validity/language.xml),
   /// [script](http://unicode.org/cldr/latest/common/validity/script.xml),
   /// [region](http://unicode.org/cldr/latest/common/validity/region.xml) and
   /// [variant](http://unicode.org/cldr/latest/common/validity/variant.xml). If
   /// there is more than one variant, they should be in sorted order. This list
   /// will be used as-is, and should never again be modified.
+  ///
+  /// Validity is not checked by default, but some methods may throw away
+  /// invalid data.
   const Locale.create({
     String language = 'und',
     String script,
@@ -337,9 +351,8 @@ class Locale {
   ///
   /// This can be null.
   ///
-  /// This is expected to be a hyphen-separated sorted list of valid Unicode
-  /// Language Identifier variant subtags that are listed in Unicode CLDR
-  /// supplemental data:
+  /// This is expected to be a sorted list of valid Unicode Language Identifier
+  /// variant subtags that are listed in Unicode CLDR supplemental data:
   /// http://unicode.org/cldr/latest/common/validity/variants.xml. Please see
   /// constructor documentation.
   final List<String> _variants;

From 58bfdc6e75338cef4a1666b5074b89f755ffc606 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Tue, 16 Oct 2018 18:02:18 +0200
Subject: [PATCH 12/26] Change reasoning for
 ignore:prefer_initializing_formals.

---
 lib/ui/window.dart | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 973aff5eac1ab..067bc19b2fce9 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -187,7 +187,7 @@ class Locale {
     String region,
     List<String> variants,
   }) : _languageCode = language,
-       scriptCode = script,  // ignore: prefer_initializing_formals, https://github.com/dart-lang/sdk/issues/34797
+       scriptCode = script,  // ignore: prefer_initializing_formals, parameter name and member name are different.
        _countryCode = region,
        _variants = variants;
 

From bc4cc070e72426a1a27a36f8467ebb92bac01b0f Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Wed, 17 Oct 2018 12:45:33 +0200
Subject: [PATCH 13/26] Try 'fromSubtags' as new constructor name.

---
 lib/ui/window.dart            |  2 +-
 testing/dart/locale_test.dart | 58 +++++++++++++++++------------------
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 067bc19b2fce9..b86a319f8dd14 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -181,7 +181,7 @@ class Locale {
   ///
   /// Validity is not checked by default, but some methods may throw away
   /// invalid data.
-  const Locale.create({
+  const Locale.fromSubtags({
     String language = 'und',
     String script,
     String region,
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 6c1e7f4a8bd53..03b08a2bf9f72 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -20,42 +20,42 @@ void main() {
     expect(const Locale('iw', 'DD'), const Locale('he', 'DE'));
   });
 
-  test('Locale.create', () {
-    expect(const Locale.create().languageCode, 'und');
-    expect(const Locale.create().scriptCode, null);
-    expect(const Locale.create().countryCode, null);
-    expect(const Locale.create().variants, orderedEquals([]));
-
-    expect(const Locale.create(language: 'en').toString(), 'en');
-    expect(const Locale.create(language: 'en').languageCode, 'en');
-    expect(const Locale.create(script: 'Latn').toString(), 'und_Latn');
-    expect(const Locale.create(script: 'Latn').scriptCode, 'Latn');
-    expect(const Locale.create(region: 'US').toString(), 'und_US');
-    expect(const Locale.create(region: 'US').countryCode, 'US');
-    expect(const Locale.create(variants: []).variants,
+  test('Locale.fromSubtags', () {
+    expect(const Locale.fromSubtags().languageCode, 'und');
+    expect(const Locale.fromSubtags().scriptCode, null);
+    expect(const Locale.fromSubtags().countryCode, null);
+    expect(const Locale.fromSubtags().variants, orderedEquals([]));
+
+    expect(const Locale.fromSubtags(language: 'en').toString(), 'en');
+    expect(const Locale.fromSubtags(language: 'en').languageCode, 'en');
+    expect(const Locale.fromSubtags(script: 'Latn').toString(), 'und_Latn');
+    expect(const Locale.fromSubtags(script: 'Latn').scriptCode, 'Latn');
+    expect(const Locale.fromSubtags(region: 'US').toString(), 'und_US');
+    expect(const Locale.fromSubtags(region: 'US').countryCode, 'US');
+    expect(const Locale.fromSubtags(variants: []).variants,
            orderedEquals([]));
-    expect(const Locale.create(variants: ['fonipa', 'scouse']).variants,
+    expect(const Locale.fromSubtags(variants: ['fonipa', 'scouse']).variants,
            orderedEquals(['fonipa', 'scouse']));
 
-    expect(Locale.create(language: 'es', region: '419').toString(), 'es_419');
-    expect(Locale.create(language: 'es', region: '419').languageCode, 'es');
-    expect(Locale.create(language: 'es', region: '419').countryCode, '419');
-    expect(Locale.create(script: 'Latn', variants: ['fonipa']).toString(), 'und_Latn_fonipa');
-    expect(Locale.create(script: 'Latn', variants: ['fonipa']).scriptCode, 'Latn');
-    expect(Locale.create(script: 'Latn', variants: ['fonipa']).variants,
+    expect(Locale.fromSubtags(language: 'es', region: '419').toString(), 'es_419');
+    expect(Locale.fromSubtags(language: 'es', region: '419').languageCode, 'es');
+    expect(Locale.fromSubtags(language: 'es', region: '419').countryCode, '419');
+    expect(Locale.fromSubtags(script: 'Latn', variants: ['fonipa']).toString(), 'und_Latn_fonipa');
+    expect(Locale.fromSubtags(script: 'Latn', variants: ['fonipa']).scriptCode, 'Latn');
+    expect(Locale.fromSubtags(script: 'Latn', variants: ['fonipa']).variants,
            orderedEquals(['fonipa']));
 
-    expect(Locale.create(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh_Hans_CN');
+    expect(Locale.fromSubtags(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh_Hans_CN');
   });
 
   test('Locale equality', () {
-    expect(Locale.create(language: 'en', variants: ['fonipa']),
-           Locale.create(language: 'en', variants: ['fonipa']));
-    expect(Locale.create(language: 'en', variants: ['fonipa']).hashCode,
-           Locale.create(language: 'en', variants: ['fonipa']).hashCode);
-    expect(Locale.create(language: 'en'),
-           isNot(Locale.create(language: 'en', script: 'Latn')));
-    expect(Locale.create(language: 'en').hashCode,
-           isNot(Locale.create(language: 'en', script: 'Latn').hashCode));
+    expect(Locale.fromSubtags(language: 'en', variants: ['fonipa']),
+           Locale.fromSubtags(language: 'en', variants: ['fonipa']));
+    expect(Locale.fromSubtags(language: 'en', variants: ['fonipa']).hashCode,
+           Locale.fromSubtags(language: 'en', variants: ['fonipa']).hashCode);
+    expect(Locale.fromSubtags(language: 'en'),
+           isNot(Locale.fromSubtags(language: 'en', script: 'Latn')));
+    expect(Locale.fromSubtags(language: 'en').hashCode,
+           isNot(Locale.fromSubtags(language: 'en', script: 'Latn').hashCode));
   });
 }

From a8b27975b81b17106cd57f34a9ad69cee36ec200 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Wed, 17 Oct 2018 13:54:47 +0200
Subject: [PATCH 14/26] Documentation improvements based on Pull Request
 review.

---
 lib/ui/window.dart | 44 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index b86a319f8dd14..a87042394b62b 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -205,6 +205,15 @@ class Locale {
   /// Locale('he')` and `const Locale('iw')` are equal, and both have the
   /// [languageCode] `he`, because `iw` is a deprecated language subtag that was
   /// replaced by the subtag `he`.
+  ///
+  /// This should be a valid Unicode Language subtag as listed in [Unicode CLDR
+  /// supplemental
+  /// data](http://unicode.org/cldr/latest/common/validity/language.xml).
+  ///
+  /// See also:
+  ///
+  ///  * [Locale.fromSubtags], which describes the conventions for creating
+  ///    [Locale] objects.
   String get languageCode => _replaceDeprecatedLanguageSubtag(_languageCode);
   final String _languageCode;
 
@@ -296,17 +305,21 @@ class Locale {
 
   /// The script subtag for the locale.
   ///
-  /// This can be null.
+  /// This may be null, indicating that there is no specified script subtag.
   ///
-  /// This is expected to be a valid Unicode Language Identifier script subtag
-  /// that is listed in Unicode CLDR supplemental data:
-  /// http://unicode.org/cldr/latest/common/validity/script.xml. Please see
-  /// constructor documentation.
+  /// This should be a valid Unicode Language Identifier script subtag as listed
+  /// in [Unicode CLDR supplemental
+  /// data](http://unicode.org/cldr/latest/common/validity/script.xml).
+  ///
+  /// See also:
+  ///
+  ///  * [Locale.fromSubtags], which describes the conventions for creating
+  ///    [Locale] objects.
   final String scriptCode;
 
   /// The region subtag for the locale.
   ///
-  /// This can be null.
+  /// This may be null, indicating that there is no specified region subtag.
   ///
   /// This is expected to be string registered in the [IANA Language Subtag
   /// Registry](https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry)
@@ -318,6 +331,11 @@ class Locale {
   /// 'DE')` and `const Locale('de', 'DD')` are equal, and both have the
   /// [countryCode] `DE`, because `DD` is a deprecated language subtag that was
   /// replaced by the subtag `DE`.
+  ///
+  /// See also:
+  ///
+  ///  * [Locale.fromSubtags], which describes the conventions for creating
+  ///    [Locale] objects.
   String get countryCode => _replaceDeprecatedRegionSubtag(_countryCode);
   final String _countryCode;
 
@@ -341,20 +359,30 @@ class Locale {
   /// subtag that is listed in Unicode CLDR supplemental data:
   /// http://unicode.org/cldr/latest/common/validity/variants.xml. Please see
   /// constructor documentation.
+  ///
+  /// See also:
+  ///
+  ///  * [Locale.fromSubtags], which describes the conventions for creating
+  ///    [Locale] objects.
   Iterable<String> get variants {
     if (_variants == null)
       return const <String>[];
     return _variants;
   }
 
-  /// Variants subtags for the locale.
+  /// The variants subtags for the locale.
   ///
-  /// This can be null.
+  /// This may be null, indicating that there is no specified variants subtag.
   ///
   /// This is expected to be a sorted list of valid Unicode Language Identifier
   /// variant subtags that are listed in Unicode CLDR supplemental data:
   /// http://unicode.org/cldr/latest/common/validity/variants.xml. Please see
   /// constructor documentation.
+  ///
+  /// See also:
+  ///
+  ///  * [Locale.fromSubtags], which describes the conventions for creating
+  ///    [Locale] objects.
   final List<String> _variants;
 
   @override

From d6f06f5e7b3537d60000c47641580475ef16abbe Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Thu, 18 Oct 2018 15:37:06 +0200
Subject: [PATCH 15/26] Assert-fail for invalid-length subtags and drop bad
 subtags in production code.

---
 lib/ui/window.dart            | 27 ++++++++++++++++++++-------
 testing/dart/locale_test.dart | 17 +++++++++++++++--
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index a87042394b62b..ad5809c677aa4 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -179,17 +179,30 @@ class Locale {
   /// there is more than one variant, they should be in sorted order. This list
   /// will be used as-is, and should never again be modified.
   ///
-  /// Validity is not checked by default, but some methods may throw away
-  /// invalid data.
+  /// Well-formedness is not fully checked by this constructor: language, script
+  /// and region subtags with bad length are discarded, however characters are
+  /// not checked. Other methods may throw away subtags with syntactically
+  /// invalid contents.
   const Locale.fromSubtags({
     String language = 'und',
     String script,
     String region,
     List<String> variants,
-  }) : _languageCode = language,
-       scriptCode = script,  // ignore: prefer_initializing_formals, parameter name and member name are different.
-       _countryCode = region,
-       _variants = variants;
+  }) : _languageCode = (language == null || language.length == 4 ||
+                        language.length < 2 || language.length > 8)
+                       ? 'und' : language,
+       // Essentially we'd like to catch errors in debug mode:
+       // assert(_languageCode == language):
+       assert(language == null ||
+              (language.length != 4 && language.length >= 2 && language.length <= 8)),
+       scriptCode = (script == null || script.length != 4) ? null : script,
+       assert(script == null || script.length == 4),
+       _countryCode = (region == null || region.length < 2 || region.length > 3) ? null : region,
+       assert(region == null || (region.length >= 2 && region.length <= 3)),
+       // We can check "variants[0].length >= 4" in a const expression, but
+       // there's not much we can do in response except drop all variants.
+       _variants = (variants == null || variants.length == 0) ? null : variants, // ignore: prefer_is_empty, const_eval_type_bool_num_string
+       assert(variants == null || variants.length > 0); // ignore: prefer_is_empty, const_eval_type_bool_num_string, https://github.com/dart-lang/sdk/issues/34798, https://dart-review.googlesource.com/c/sdk/+/71340
 
   /// The primary language subtag for the locale.
   ///
@@ -421,7 +434,7 @@ class Locale {
     if (_countryCode != null)
       out.write('_$countryCode');
     if (_variants != null)
-      out.write('_${_variants.join("_")}');
+      out.write('_${variants.join("_")}');
     return out.toString();
   }
 }
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 03b08a2bf9f72..65f27811f1be1 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -32,11 +32,24 @@ void main() {
     expect(const Locale.fromSubtags(script: 'Latn').scriptCode, 'Latn');
     expect(const Locale.fromSubtags(region: 'US').toString(), 'und_US');
     expect(const Locale.fromSubtags(region: 'US').countryCode, 'US');
-    expect(const Locale.fromSubtags(variants: []).variants,
-           orderedEquals([]));
+    expect(const Locale.fromSubtags(variants: ['fonipa', 'scouse']).toString(),
+           'und_fonipa_scouse');
     expect(const Locale.fromSubtags(variants: ['fonipa', 'scouse']).variants,
            orderedEquals(['fonipa', 'scouse']));
 
+    // Cannot test these here, because this would be the case in production
+    // mode, whereas in debug mode they cause assertion failures:
+    //
+    // expect(const Locale.fromSubtags(language: 'a').languageCode, 'und',
+    //        reason: 'Valid language subtags are 2 to 8 characters long.');
+    // expect(const Locale.fromSubtags(language: 'Latn').languageCode, 'und',
+    //        reason: 'Valid language subtags are not 4 characters long, '
+    //                'to not be confused with script subtags.');
+    // expect(const Locale.fromSubtags(script: 'en').scriptCode, null,
+    //        reason: 'Valid script subtags are 4 characters.');
+    // expect(const Locale.fromSubtags(region: 'Germany').countryCode, null,
+    //        reason: 'Valid region subtags are 2 characters or 3 digits.');
+
     expect(Locale.fromSubtags(language: 'es', region: '419').toString(), 'es_419');
     expect(Locale.fromSubtags(language: 'es', region: '419').languageCode, 'es');
     expect(Locale.fromSubtags(language: 'es', region: '419').countryCode, '419');

From 8948502843cbab5c5abc40c4a8dfdba72c4f8154 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Fri, 19 Oct 2018 12:56:05 +0200
Subject: [PATCH 16/26] Revert "Assert-fail for invalid-length subtags and drop
 bad subtags in production code."

This reverts commit d6f06f5e7b3537d60000c47641580475ef16abbe.
---
 lib/ui/window.dart            | 27 +++++++--------------------
 testing/dart/locale_test.dart | 17 ++---------------
 2 files changed, 9 insertions(+), 35 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index ad5809c677aa4..a87042394b62b 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -179,30 +179,17 @@ class Locale {
   /// there is more than one variant, they should be in sorted order. This list
   /// will be used as-is, and should never again be modified.
   ///
-  /// Well-formedness is not fully checked by this constructor: language, script
-  /// and region subtags with bad length are discarded, however characters are
-  /// not checked. Other methods may throw away subtags with syntactically
-  /// invalid contents.
+  /// Validity is not checked by default, but some methods may throw away
+  /// invalid data.
   const Locale.fromSubtags({
     String language = 'und',
     String script,
     String region,
     List<String> variants,
-  }) : _languageCode = (language == null || language.length == 4 ||
-                        language.length < 2 || language.length > 8)
-                       ? 'und' : language,
-       // Essentially we'd like to catch errors in debug mode:
-       // assert(_languageCode == language):
-       assert(language == null ||
-              (language.length != 4 && language.length >= 2 && language.length <= 8)),
-       scriptCode = (script == null || script.length != 4) ? null : script,
-       assert(script == null || script.length == 4),
-       _countryCode = (region == null || region.length < 2 || region.length > 3) ? null : region,
-       assert(region == null || (region.length >= 2 && region.length <= 3)),
-       // We can check "variants[0].length >= 4" in a const expression, but
-       // there's not much we can do in response except drop all variants.
-       _variants = (variants == null || variants.length == 0) ? null : variants, // ignore: prefer_is_empty, const_eval_type_bool_num_string
-       assert(variants == null || variants.length > 0); // ignore: prefer_is_empty, const_eval_type_bool_num_string, https://github.com/dart-lang/sdk/issues/34798, https://dart-review.googlesource.com/c/sdk/+/71340
+  }) : _languageCode = language,
+       scriptCode = script,  // ignore: prefer_initializing_formals, parameter name and member name are different.
+       _countryCode = region,
+       _variants = variants;
 
   /// The primary language subtag for the locale.
   ///
@@ -434,7 +421,7 @@ class Locale {
     if (_countryCode != null)
       out.write('_$countryCode');
     if (_variants != null)
-      out.write('_${variants.join("_")}');
+      out.write('_${_variants.join("_")}');
     return out.toString();
   }
 }
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 65f27811f1be1..03b08a2bf9f72 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -32,24 +32,11 @@ void main() {
     expect(const Locale.fromSubtags(script: 'Latn').scriptCode, 'Latn');
     expect(const Locale.fromSubtags(region: 'US').toString(), 'und_US');
     expect(const Locale.fromSubtags(region: 'US').countryCode, 'US');
-    expect(const Locale.fromSubtags(variants: ['fonipa', 'scouse']).toString(),
-           'und_fonipa_scouse');
+    expect(const Locale.fromSubtags(variants: []).variants,
+           orderedEquals([]));
     expect(const Locale.fromSubtags(variants: ['fonipa', 'scouse']).variants,
            orderedEquals(['fonipa', 'scouse']));
 
-    // Cannot test these here, because this would be the case in production
-    // mode, whereas in debug mode they cause assertion failures:
-    //
-    // expect(const Locale.fromSubtags(language: 'a').languageCode, 'und',
-    //        reason: 'Valid language subtags are 2 to 8 characters long.');
-    // expect(const Locale.fromSubtags(language: 'Latn').languageCode, 'und',
-    //        reason: 'Valid language subtags are not 4 characters long, '
-    //                'to not be confused with script subtags.');
-    // expect(const Locale.fromSubtags(script: 'en').scriptCode, null,
-    //        reason: 'Valid script subtags are 4 characters.');
-    // expect(const Locale.fromSubtags(region: 'Germany').countryCode, null,
-    //        reason: 'Valid region subtags are 2 characters or 3 digits.');
-
     expect(Locale.fromSubtags(language: 'es', region: '419').toString(), 'es_419');
     expect(Locale.fromSubtags(language: 'es', region: '419').languageCode, 'es');
     expect(Locale.fromSubtags(language: 'es', region: '419').countryCode, '419');

From 20e8bd715990abd19ee8117a9ef48122c5d8e0a1 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Fri, 19 Oct 2018 15:03:08 +0200
Subject: [PATCH 17/26] Re-fix Locale.toString() for variants=[].

---
 lib/ui/window.dart            | 2 +-
 testing/dart/locale_test.dart | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index a87042394b62b..05205d48b3e9d 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -420,7 +420,7 @@ class Locale {
       out.write('_$scriptCode');
     if (_countryCode != null)
       out.write('_$countryCode');
-    if (_variants != null)
+    if (_variants != null && _variants.isNotEmpty)
       out.write('_${_variants.join("_")}');
     return out.toString();
   }
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 03b08a2bf9f72..6aebaef74f5f4 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -32,6 +32,7 @@ void main() {
     expect(const Locale.fromSubtags(script: 'Latn').scriptCode, 'Latn');
     expect(const Locale.fromSubtags(region: 'US').toString(), 'und_US');
     expect(const Locale.fromSubtags(region: 'US').countryCode, 'US');
+    expect(const Locale.fromSubtags(variants: []).toString(), 'und');
     expect(const Locale.fromSubtags(variants: []).variants,
            orderedEquals([]));
     expect(const Locale.fromSubtags(variants: ['fonipa', 'scouse']).variants,

From 9911dfd6bb32956038cfef2049067173cbf26f72 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Fri, 19 Oct 2018 16:12:35 +0200
Subject: [PATCH 18/26] Tear out variants, in case we want to have one fewer
 pointer in the future.

---
 lib/ui/window.dart            | 54 +++++------------------------------
 testing/dart/locale_test.dart | 14 ---------
 2 files changed, 7 insertions(+), 61 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 05205d48b3e9d..e926406b625c5 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -118,7 +118,8 @@ class WindowPadding {
 
 /// An identifier used to select a user's language and formatting preferences.
 /// This represents a [Unicode Language
-/// Identifier](https://www.unicode.org/reports/tr35/#Unicode_language_identifier).
+/// Identifier](https://www.unicode.org/reports/tr35/#Unicode_language_identifier),
+/// except variants have not been implemented yet..
 ///
 /// Locales are canonicalized according to the "preferred value" entries in the
 /// [IANA Language Subtag
@@ -163,8 +164,7 @@ class Locale {
     this._languageCode, [
     this._countryCode,
   ]) : assert(_languageCode != null),
-       this.scriptCode = null,
-       this._variants = null;
+       this.scriptCode = null;
 
   /// Creates a new Locale object.
   ///
@@ -174,10 +174,7 @@ class Locale {
   /// according to CLDR supplemental data:
   /// [language](http://unicode.org/cldr/latest/common/validity/language.xml),
   /// [script](http://unicode.org/cldr/latest/common/validity/script.xml),
-  /// [region](http://unicode.org/cldr/latest/common/validity/region.xml) and
-  /// [variant](http://unicode.org/cldr/latest/common/validity/variant.xml). If
-  /// there is more than one variant, they should be in sorted order. This list
-  /// will be used as-is, and should never again be modified.
+  /// [region](http://unicode.org/cldr/latest/common/validity/region.xml).
   ///
   /// Validity is not checked by default, but some methods may throw away
   /// invalid data.
@@ -185,11 +182,9 @@ class Locale {
     String language = 'und',
     String script,
     String region,
-    List<String> variants,
   }) : _languageCode = language,
        scriptCode = script,  // ignore: prefer_initializing_formals, parameter name and member name are different.
-       _countryCode = region,
-       _variants = variants;
+       _countryCode = region;
 
   /// The primary language subtag for the locale.
   ///
@@ -353,38 +348,6 @@ class Locale {
     }
   }
 
-  /// The variants subtags for the locale.
-  ///
-  /// Each subtag is expected to be a valid Unicode Language Identifier variant
-  /// subtag that is listed in Unicode CLDR supplemental data:
-  /// http://unicode.org/cldr/latest/common/validity/variants.xml. Please see
-  /// constructor documentation.
-  ///
-  /// See also:
-  ///
-  ///  * [Locale.fromSubtags], which describes the conventions for creating
-  ///    [Locale] objects.
-  Iterable<String> get variants {
-    if (_variants == null)
-      return const <String>[];
-    return _variants;
-  }
-
-  /// The variants subtags for the locale.
-  ///
-  /// This may be null, indicating that there is no specified variants subtag.
-  ///
-  /// This is expected to be a sorted list of valid Unicode Language Identifier
-  /// variant subtags that are listed in Unicode CLDR supplemental data:
-  /// http://unicode.org/cldr/latest/common/validity/variants.xml. Please see
-  /// constructor documentation.
-  ///
-  /// See also:
-  ///
-  ///  * [Locale.fromSubtags], which describes the conventions for creating
-  ///    [Locale] objects.
-  final List<String> _variants;
-
   @override
   bool operator ==(dynamic other) {
     if (identical(this, other))
@@ -394,8 +357,7 @@ class Locale {
     final Locale typedOther = other;
     return languageCode == typedOther.languageCode
         && scriptCode == typedOther.scriptCode
-        && countryCode == typedOther.countryCode
-        && _listEquals<String>(_variants, typedOther._variants);
+        && countryCode == typedOther.countryCode;
   }
 
   bool _listEquals<T>(List<T> a, List<T> b) {
@@ -411,7 +373,7 @@ class Locale {
   }
 
   @override
-  int get hashCode => hashValues(languageCode, scriptCode, countryCode, hashList(_variants));
+  int get hashCode => hashValues(languageCode, scriptCode, countryCode);
 
   @override
   String toString() {
@@ -420,8 +382,6 @@ class Locale {
       out.write('_$scriptCode');
     if (_countryCode != null)
       out.write('_$countryCode');
-    if (_variants != null && _variants.isNotEmpty)
-      out.write('_${_variants.join("_")}');
     return out.toString();
   }
 }
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 6aebaef74f5f4..e9c63e1fb9a10 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -24,7 +24,6 @@ void main() {
     expect(const Locale.fromSubtags().languageCode, 'und');
     expect(const Locale.fromSubtags().scriptCode, null);
     expect(const Locale.fromSubtags().countryCode, null);
-    expect(const Locale.fromSubtags().variants, orderedEquals([]));
 
     expect(const Locale.fromSubtags(language: 'en').toString(), 'en');
     expect(const Locale.fromSubtags(language: 'en').languageCode, 'en');
@@ -32,28 +31,15 @@ void main() {
     expect(const Locale.fromSubtags(script: 'Latn').scriptCode, 'Latn');
     expect(const Locale.fromSubtags(region: 'US').toString(), 'und_US');
     expect(const Locale.fromSubtags(region: 'US').countryCode, 'US');
-    expect(const Locale.fromSubtags(variants: []).toString(), 'und');
-    expect(const Locale.fromSubtags(variants: []).variants,
-           orderedEquals([]));
-    expect(const Locale.fromSubtags(variants: ['fonipa', 'scouse']).variants,
-           orderedEquals(['fonipa', 'scouse']));
 
     expect(Locale.fromSubtags(language: 'es', region: '419').toString(), 'es_419');
     expect(Locale.fromSubtags(language: 'es', region: '419').languageCode, 'es');
     expect(Locale.fromSubtags(language: 'es', region: '419').countryCode, '419');
-    expect(Locale.fromSubtags(script: 'Latn', variants: ['fonipa']).toString(), 'und_Latn_fonipa');
-    expect(Locale.fromSubtags(script: 'Latn', variants: ['fonipa']).scriptCode, 'Latn');
-    expect(Locale.fromSubtags(script: 'Latn', variants: ['fonipa']).variants,
-           orderedEquals(['fonipa']));
 
     expect(Locale.fromSubtags(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh_Hans_CN');
   });
 
   test('Locale equality', () {
-    expect(Locale.fromSubtags(language: 'en', variants: ['fonipa']),
-           Locale.fromSubtags(language: 'en', variants: ['fonipa']));
-    expect(Locale.fromSubtags(language: 'en', variants: ['fonipa']).hashCode,
-           Locale.fromSubtags(language: 'en', variants: ['fonipa']).hashCode);
     expect(Locale.fromSubtags(language: 'en'),
            isNot(Locale.fromSubtags(language: 'en', script: 'Latn')));
     expect(Locale.fromSubtags(language: 'en').hashCode,

From cb81b55fb811315bbaa045029b46833016df6945 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Fri, 19 Oct 2018 17:21:30 +0200
Subject: [PATCH 19/26] Make named parameters' names consistent with member
 names.

---
 lib/ui/window.dart            | 12 ++++++------
 testing/dart/locale_test.dart | 32 ++++++++++++++++----------------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index e926406b625c5..7bceb360e4c26 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -175,16 +175,16 @@ class Locale {
   /// [language](http://unicode.org/cldr/latest/common/validity/language.xml),
   /// [script](http://unicode.org/cldr/latest/common/validity/script.xml),
   /// [region](http://unicode.org/cldr/latest/common/validity/region.xml).
+  /// for each of languageCode, scriptCode and countryCode respectively.
   ///
   /// Validity is not checked by default, but some methods may throw away
   /// invalid data.
   const Locale.fromSubtags({
-    String language = 'und',
-    String script,
-    String region,
-  }) : _languageCode = language,
-       scriptCode = script,  // ignore: prefer_initializing_formals, parameter name and member name are different.
-       _countryCode = region;
+    String languageCode = 'und',
+    this.scriptCode,
+    String countryCode,
+  }) : _languageCode = languageCode,
+       _countryCode = countryCode;
 
   /// The primary language subtag for the locale.
   ///
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index e9c63e1fb9a10..66fd537a72444 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -25,24 +25,24 @@ void main() {
     expect(const Locale.fromSubtags().scriptCode, null);
     expect(const Locale.fromSubtags().countryCode, null);
 
-    expect(const Locale.fromSubtags(language: 'en').toString(), 'en');
-    expect(const Locale.fromSubtags(language: 'en').languageCode, 'en');
-    expect(const Locale.fromSubtags(script: 'Latn').toString(), 'und_Latn');
-    expect(const Locale.fromSubtags(script: 'Latn').scriptCode, 'Latn');
-    expect(const Locale.fromSubtags(region: 'US').toString(), 'und_US');
-    expect(const Locale.fromSubtags(region: 'US').countryCode, 'US');
-
-    expect(Locale.fromSubtags(language: 'es', region: '419').toString(), 'es_419');
-    expect(Locale.fromSubtags(language: 'es', region: '419').languageCode, 'es');
-    expect(Locale.fromSubtags(language: 'es', region: '419').countryCode, '419');
-
-    expect(Locale.fromSubtags(language: 'zh', script: 'Hans', region: 'CN').toString(), 'zh_Hans_CN');
+    expect(const Locale.fromSubtags(languageCode: 'en').toString(), 'en');
+    expect(const Locale.fromSubtags(languageCode: 'en').languageCode, 'en');
+    expect(const Locale.fromSubtags(scriptCode: 'Latn').toString(), 'und_Latn');
+    expect(const Locale.fromSubtags(scriptCode: 'Latn').scriptCode, 'Latn');
+    expect(const Locale.fromSubtags(countryCode: 'US').toString(), 'und_US');
+    expect(const Locale.fromSubtags(countryCode: 'US').countryCode, 'US');
+
+    expect(Locale.fromSubtags(languageCode: 'es', countryCode: '419').toString(), 'es_419');
+    expect(Locale.fromSubtags(languageCode: 'es', countryCode: '419').languageCode, 'es');
+    expect(Locale.fromSubtags(languageCode: 'es', countryCode: '419').countryCode, '419');
+
+    expect(Locale.fromSubtags(languageCode: 'zh', scriptCode: 'Hans', countryCode: 'CN').toString(), 'zh_Hans_CN');
   });
 
   test('Locale equality', () {
-    expect(Locale.fromSubtags(language: 'en'),
-           isNot(Locale.fromSubtags(language: 'en', script: 'Latn')));
-    expect(Locale.fromSubtags(language: 'en').hashCode,
-           isNot(Locale.fromSubtags(language: 'en', script: 'Latn').hashCode));
+    expect(Locale.fromSubtags(languageCode: 'en'),
+           isNot(Locale.fromSubtags(languageCode: 'en', scriptCode: 'Latn')));
+    expect(Locale.fromSubtags(languageCode: 'en').hashCode,
+           isNot(Locale.fromSubtags(languageCode: 'en', scriptCode: 'Latn').hashCode));
   });
 }

From 5e0def86ec7ca74fce7e89720bd7be3153fd9408 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Mon, 22 Oct 2018 14:46:55 +0200
Subject: [PATCH 20/26] Also remove _listEquals: no longer in use.

---
 lib/ui/window.dart | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 7bceb360e4c26..a5af1033940f6 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -360,18 +360,6 @@ class Locale {
         && countryCode == typedOther.countryCode;
   }
 
-  bool _listEquals<T>(List<T> a, List<T> b) {
-    if (a == null)
-      return b == null;
-    if (b == null || a.length != b.length)
-      return false;
-    for (int index = 0; index < a.length; index += 1) {
-      if (a[index] != b[index])
-        return false;
-    }
-    return true;
-  }
-
   @override
   int get hashCode => hashValues(languageCode, scriptCode, countryCode);
 

From 97ab4fea36f5eed0bbd3cf1446b91324e25b6a2f Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Tue, 23 Oct 2018 16:22:30 +0200
Subject: [PATCH 21/26] Lint fix.

---
 lib/ui/window.dart | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index c2bccc0d8f5a3..d614e41272579 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -165,7 +165,7 @@ class Locale {
     this._countryCode,
   ]) : assert(_languageCode != null),
        assert(_languageCode != ''),
-       this.scriptCode = null;
+       scriptCode = null;
 
   /// Creates a new Locale object.
   ///

From 5f625ae0f9dbd5afb63079578164aa43edae5bf7 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Wed, 24 Oct 2018 14:56:09 +0200
Subject: [PATCH 22/26] Fix code review nits.

---
 lib/ui/window.dart | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index d614e41272579..65be3a15a836f 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -117,9 +117,10 @@ class WindowPadding {
 }
 
 /// An identifier used to select a user's language and formatting preferences.
+///
 /// This represents a [Unicode Language
-/// Identifier](https://www.unicode.org/reports/tr35/#Unicode_language_identifier),
-/// except variants have not been implemented yet..
+/// Identifier](https://www.unicode.org/reports/tr35/#Unicode_language_identifier)
+/// (i.e. without Locale extensions), except variants are not supported.
 ///
 /// Locales are canonicalized according to the "preferred value" entries in the
 /// [IANA Language Subtag
@@ -159,7 +160,9 @@ class Locale {
   /// Validity is not checked by default, but some methods may throw away
   /// invalid data.
   ///
-  /// See also: [Locale.create].
+  /// See also:
+  ///
+  ///  * [new Locale.fromSubtags].
   const Locale(
     this._languageCode, [
     this._countryCode,
@@ -184,12 +187,14 @@ class Locale {
     String languageCode = 'und',
     this.scriptCode,
     String countryCode,
-  }) : _languageCode = languageCode,
+  }) : assert(_languageCode != null),
+       assert(_languageCode != ''),
+       _languageCode = languageCode,
        _countryCode = countryCode;
 
   /// The primary language subtag for the locale.
   ///
-  /// This must not be null.
+  /// This must not be null. It may be 'und' representing 'undefined'.
   ///
   /// This is expected to be string registered in the [IANA Language Subtag
   /// Registry](https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry)
@@ -208,7 +213,7 @@ class Locale {
   ///
   /// See also:
   ///
-  ///  * [Locale.fromSubtags], which describes the conventions for creating
+  ///  * [new Locale.fromSubtags], which describes the conventions for creating
   ///    [Locale] objects.
   String get languageCode => _replaceDeprecatedLanguageSubtag(_languageCode);
   final String _languageCode;
@@ -309,7 +314,7 @@ class Locale {
   ///
   /// See also:
   ///
-  ///  * [Locale.fromSubtags], which describes the conventions for creating
+  ///  * [new Locale.fromSubtags], which describes the conventions for creating
   ///    [Locale] objects.
   final String scriptCode;
 
@@ -330,7 +335,7 @@ class Locale {
   ///
   /// See also:
   ///
-  ///  * [Locale.fromSubtags], which describes the conventions for creating
+  ///  * [new Locale.fromSubtags], which describes the conventions for creating
   ///    [Locale] objects.
   String get countryCode => _replaceDeprecatedRegionSubtag(_countryCode);
   final String _countryCode;

From 3b870683cbd57770e44d7d15844611fddacc8ca9 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Wed, 24 Oct 2018 15:03:08 +0200
Subject: [PATCH 23/26] Lint fix for assert, and a couple more
 not-zero-length-string asserts.

---
 lib/ui/window.dart | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 65be3a15a836f..668d6ac2b8e2f 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -187,9 +187,11 @@ class Locale {
     String languageCode = 'und',
     this.scriptCode,
     String countryCode,
-  }) : assert(_languageCode != null),
-       assert(_languageCode != ''),
+  }) : assert(languageCode != null),
+       assert(languageCode != ''),
        _languageCode = languageCode,
+       assert(scriptCode != ''),
+       assert(countryCode != ''),
        _countryCode = countryCode;
 
   /// The primary language subtag for the locale.

From df46762f4bb8626e1bc38f06314cde611432f5d3 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Fri, 26 Oct 2018 11:44:26 +0200
Subject: [PATCH 24/26] Code Review: two of three nits addressed...

---
 lib/ui/window.dart | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 668d6ac2b8e2f..fd5fe8d9631e2 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -162,7 +162,8 @@ class Locale {
   ///
   /// See also:
   ///
-  ///  * [new Locale.fromSubtags].
+  ///  * [new Locale.fromSubtags], which also allows a [scriptCode] to be
+  ///    specified.
   const Locale(
     this._languageCode, [
     this._countryCode,
@@ -196,7 +197,7 @@ class Locale {
 
   /// The primary language subtag for the locale.
   ///
-  /// This must not be null. It may be 'und' representing 'undefined'.
+  /// This must not be null. It may be 'und', representing 'undefined'.
   ///
   /// This is expected to be string registered in the [IANA Language Subtag
   /// Registry](https://www.iana.org/assignments/language-subtag-registry/language-subtag-registry)

From 57b068e4e0fd79bbeb82e6a193d1addf8d0b4ceb Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Fri, 26 Oct 2018 16:42:21 +0200
Subject: [PATCH 25/26] Review fix: change 'should' to 'must' for subtag
 prescriptions.

---
 lib/ui/window.dart | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index fd5fe8d9631e2..57b40bf1cdffa 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -147,13 +147,13 @@ class Locale {
   /// The primary language subtag must not be null. The region subtag is
   /// optional.
   ///
-  /// The subtag values are _case sensitive_ and should be one of the valid
+  /// The subtag values are _case sensitive_ and must be one of the valid
   /// subtags according to CLDR supplemental data:
   /// [language](http://unicode.org/cldr/latest/common/validity/language.xml),
   /// [region](http://unicode.org/cldr/latest/common/validity/region.xml). The
-  /// primary language subtag should be two to three lowercase letters. The
-  /// region region subtag should be two uppercase letters or three digits. See
-  /// the [Unicode Language
+  /// primary language subtag must be at least two and at most eight lowercase
+  /// letters, but not four letters. The region region subtag must be two
+  /// uppercase letters or three digits. See the [Unicode Language
   /// Identifier](https://www.unicode.org/reports/tr35/#Unicode_language_identifier)
   /// specification.
   ///
@@ -175,12 +175,12 @@ class Locale {
   ///
   /// The keyword arguments specify the subtags of the Locale.
   ///
-  /// The subtag values are _case sensitive_ and should be valid subtags
-  /// according to CLDR supplemental data:
+  /// The subtag values are _case sensitive_ and must be valid subtags according
+  /// to CLDR supplemental data:
   /// [language](http://unicode.org/cldr/latest/common/validity/language.xml),
-  /// [script](http://unicode.org/cldr/latest/common/validity/script.xml),
-  /// [region](http://unicode.org/cldr/latest/common/validity/region.xml).
-  /// for each of languageCode, scriptCode and countryCode respectively.
+  /// [script](http://unicode.org/cldr/latest/common/validity/script.xml) and
+  /// [region](http://unicode.org/cldr/latest/common/validity/region.xml) for
+  /// each of languageCode, scriptCode and countryCode respectively.
   ///
   /// Validity is not checked by default, but some methods may throw away
   /// invalid data.
@@ -210,7 +210,7 @@ class Locale {
   /// [languageCode] `he`, because `iw` is a deprecated language subtag that was
   /// replaced by the subtag `he`.
   ///
-  /// This should be a valid Unicode Language subtag as listed in [Unicode CLDR
+  /// This must be a valid Unicode Language subtag as listed in [Unicode CLDR
   /// supplemental
   /// data](http://unicode.org/cldr/latest/common/validity/language.xml).
   ///
@@ -311,7 +311,7 @@ class Locale {
   ///
   /// This may be null, indicating that there is no specified script subtag.
   ///
-  /// This should be a valid Unicode Language Identifier script subtag as listed
+  /// This must be a valid Unicode Language Identifier script subtag as listed
   /// in [Unicode CLDR supplemental
   /// data](http://unicode.org/cldr/latest/common/validity/script.xml).
   ///

From 6f07e8d413abe98e7bf28a34c766269bafdd20f9 Mon Sep 17 00:00:00 2001
From: Hugo van der Merwe <hugovdm@gmail.com>
Date: Mon, 29 Oct 2018 14:27:22 +0100
Subject: [PATCH 26/26] Assert-check that countryCode is never ''.

---
 lib/ui/window.dart            | 3 ++-
 testing/dart/locale_test.dart | 2 --
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/ui/window.dart b/lib/ui/window.dart
index 57b40bf1cdffa..fff5eb46206bc 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -169,7 +169,8 @@ class Locale {
     this._countryCode,
   ]) : assert(_languageCode != null),
        assert(_languageCode != ''),
-       scriptCode = null;
+       scriptCode = null,
+       assert(_countryCode != '');
 
   /// Creates a new Locale object.
   ///
diff --git a/testing/dart/locale_test.dart b/testing/dart/locale_test.dart
index 66fd537a72444..4613a707510c4 100644
--- a/testing/dart/locale_test.dart
+++ b/testing/dart/locale_test.dart
@@ -12,8 +12,6 @@ void main() {
     expect(const Locale('en').toString(), 'en');
     expect(const Locale('en'), new Locale('en', $null));
     expect(const Locale('en').hashCode, new Locale('en', $null).hashCode);
-    expect(const Locale('en'), isNot(new Locale('en', '')));
-    expect(const Locale('en').hashCode, isNot(new Locale('en', '').hashCode));
     expect(const Locale('en', 'US').toString(), 'en_US');
     expect(const Locale('iw').toString(), 'he');
     expect(const Locale('iw', 'DD').toString(), 'he_DE');