From 6a34f3f5f84e01f28d0c1cba545b37a8f6b9669e Mon Sep 17 00:00:00 2001 From: Paul Blasi Date: Thu, 16 Feb 2023 11:09:39 -0700 Subject: [PATCH 1/3] Match PlatformConfiguration properties to PlatformDispatcher ones * Updated PlatformDispatcher to use PlatformConfiguration to back `nativeSpellCheckServiceDefined` and `brieflyShowPassword` * Updated web PlatformDispatcher/Configuration to match new API --- lib/ui/platform_dispatcher.dart | 38 +++++++++++++++++-------- lib/web_ui/lib/platform_dispatcher.dart | 12 ++++++-- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 609ce71f80381..5b182af67b7a5 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -981,8 +981,7 @@ class PlatformDispatcher { /// This option is used by [EditableTextState] to define its /// [SpellCheckConfiguration] when a default spell check service /// is requested. - bool get nativeSpellCheckServiceDefined => _nativeSpellCheckServiceDefined; - bool _nativeSpellCheckServiceDefined = false; + bool get nativeSpellCheckServiceDefined => configuration.nativeSpellCheckServiceDefined; /// Whether briefly displaying the characters as you type in obscured text /// fields is enabled in system settings. @@ -991,8 +990,7 @@ class PlatformDispatcher { /// /// * [EditableText.obscureText], which when set to true hides the text in /// the text field. - bool get brieflyShowPassword => _brieflyShowPassword; - bool _brieflyShowPassword = true; + bool get brieflyShowPassword => configuration.brieflyShowPassword; /// The setting indicating the current brightness mode of the host platform. /// If the platform has no preference, [platformBrightness] defaults to @@ -1046,16 +1044,8 @@ class PlatformDispatcher { final double textScaleFactor = (data['textScaleFactor']! as num).toDouble(); final bool alwaysUse24HourFormat = data['alwaysUse24HourFormat']! as bool; final bool? nativeSpellCheckServiceDefined = data['nativeSpellCheckServiceDefined'] as bool?; - if (nativeSpellCheckServiceDefined != null) { - _nativeSpellCheckServiceDefined = nativeSpellCheckServiceDefined; - } else { - _nativeSpellCheckServiceDefined = false; - } // This field is optional. final bool? brieflyShowPassword = data['brieflyShowPassword'] as bool?; - if (brieflyShowPassword != null) { - _brieflyShowPassword = brieflyShowPassword; - } final Brightness platformBrightness = data['platformBrightness']! as String == 'dark' ? Brightness.dark : Brightness.light; final String? systemFontFamily = data['systemFontFamily'] as String?; @@ -1072,6 +1062,8 @@ class PlatformDispatcher { _configuration = previousConfiguration.copyWith( textScaleFactor: textScaleFactor, alwaysUse24HourFormat: alwaysUse24HourFormat, + nativeSpellCheckServiceDefined: nativeSpellCheckServiceDefined ?? false, + brieflyShowPassword: brieflyShowPassword, platformBrightness: platformBrightness, systemFontFamily: systemFontFamily, ); @@ -1259,6 +1251,8 @@ class PlatformConfiguration { this.semanticsEnabled = false, this.platformBrightness = Brightness.light, this.textScaleFactor = 1.0, + this.nativeSpellCheckServiceDefined = false, + this.brieflyShowPassword = true, this.locales = const [], this.defaultRouteName, this.systemFontFamily, @@ -1271,6 +1265,8 @@ class PlatformConfiguration { bool? semanticsEnabled, Brightness? platformBrightness, double? textScaleFactor, + bool? nativeSpellCheckServiceDefined, + bool? brieflyShowPassword, List? locales, String? defaultRouteName, String? systemFontFamily, @@ -1281,6 +1277,8 @@ class PlatformConfiguration { semanticsEnabled: semanticsEnabled ?? this.semanticsEnabled, platformBrightness: platformBrightness ?? this.platformBrightness, textScaleFactor: textScaleFactor ?? this.textScaleFactor, + nativeSpellCheckServiceDefined: nativeSpellCheckServiceDefined ?? this.nativeSpellCheckServiceDefined, + brieflyShowPassword: brieflyShowPassword ?? this.brieflyShowPassword, locales: locales ?? this.locales, defaultRouteName: defaultRouteName ?? this.defaultRouteName, systemFontFamily: systemFontFamily ?? this.systemFontFamily, @@ -1306,6 +1304,22 @@ class PlatformConfiguration { /// The system-reported text scale. final double textScaleFactor; + /// Whether the spell check service is supported on the current platform. + /// + /// This option is used by [EditableTextState] to define its + /// [SpellCheckConfiguration] when a default spell check service + /// is requested. + final bool nativeSpellCheckServiceDefined; + + /// Whether briefly displaying the characters as you type in obscured text + /// fields is enabled in system settings. + /// + /// See also: + /// + /// * [EditableText.obscureText], which when set to true hides the text in + /// the text field. + final bool brieflyShowPassword; + /// The full system-reported supported locales of the device. final List locales; diff --git a/lib/web_ui/lib/platform_dispatcher.dart b/lib/web_ui/lib/platform_dispatcher.dart index 16887b86df744..fe71c4eebf7a3 100644 --- a/lib/web_ui/lib/platform_dispatcher.dart +++ b/lib/web_ui/lib/platform_dispatcher.dart @@ -110,9 +110,9 @@ abstract class PlatformDispatcher { double get textScaleFactor => configuration.textScaleFactor; - bool get nativeSpellCheckServiceDefined => false; + bool get nativeSpellCheckServiceDefined => configuration.nativeSpellCheckServiceDefined; - bool get brieflyShowPassword => true; + bool get brieflyShowPassword => configuration.brieflyShowPassword; VoidCallback? get onTextScaleFactorChanged; set onTextScaleFactorChanged(VoidCallback? callback); @@ -153,6 +153,8 @@ class PlatformConfiguration { this.semanticsEnabled = false, this.platformBrightness = Brightness.light, this.textScaleFactor = 1.0, + this.nativeSpellCheckServiceDefined = false, + this.brieflyShowPassword = true, this.locales = const [], this.defaultRouteName = '/', this.systemFontFamily, @@ -164,6 +166,8 @@ class PlatformConfiguration { bool? semanticsEnabled, Brightness? platformBrightness, double? textScaleFactor, + bool? nativeSpellCheckServiceDefined, + bool? brieflyShowPassword, List? locales, String? defaultRouteName, String? systemFontFamily, @@ -174,6 +178,8 @@ class PlatformConfiguration { semanticsEnabled: semanticsEnabled ?? this.semanticsEnabled, platformBrightness: platformBrightness ?? this.platformBrightness, textScaleFactor: textScaleFactor ?? this.textScaleFactor, + nativeSpellCheckServiceDefined: nativeSpellCheckServiceDefined ?? this.nativeSpellCheckServiceDefined, + brieflyShowPassword: brieflyShowPassword ?? this.brieflyShowPassword, locales: locales ?? this.locales, defaultRouteName: defaultRouteName ?? this.defaultRouteName, systemFontFamily: systemFontFamily ?? this.systemFontFamily, @@ -185,6 +191,8 @@ class PlatformConfiguration { final bool semanticsEnabled; final Brightness platformBrightness; final double textScaleFactor; + final bool nativeSpellCheckServiceDefined; + final bool brieflyShowPassword; final List locales; final String defaultRouteName; final String? systemFontFamily; From 8ea008346306c41fc3e7df45728cef3655c9001d Mon Sep 17 00:00:00 2001 From: Paul Blasi Date: Wed, 22 Feb 2023 12:59:19 -0700 Subject: [PATCH 2/3] Added a test to ensure `PlatformDispatcher` and `PlatformConfig` have the same getters with exceptions. --- testing/dart/platform_dispatcher_test.dart | 61 ++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/testing/dart/platform_dispatcher_test.dart b/testing/dart/platform_dispatcher_test.dart index dc19fba3fe57f..1cdd4e7449e8f 100644 --- a/testing/dart/platform_dispatcher_test.dart +++ b/testing/dart/platform_dispatcher_test.dart @@ -2,8 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:io'; import 'dart:ui'; +import 'package:analyzer/dart/analysis/features.dart'; +import 'package:analyzer/dart/analysis/results.dart'; +import 'package:analyzer/dart/analysis/utilities.dart'; +import 'package:analyzer/dart/ast/ast.dart'; import 'package:litetest/litetest.dart'; void main() { @@ -39,4 +44,60 @@ void main() { return ViewConfiguration(view: view)..copyWith(view: view, window: view); }); }); + + test("PlatformDispatcher and PlatformConfiguration have matching getters", () { + final List exclusionCriteria = [ + RegExp(r'^_.*'), // exclude any private members + RegExp(r'^on[A-Z].*'), // exclude any callbacks + RegExp(r'locale$'), // locale is a convenience getter for interacting with `locales` which _is_ backed by the config + 'instance', // exclude the static instance of PlatformDispatcher + 'configuration', // the configuration should not reference itself + 'views', // views are backed by their own config and don't need to be referenced in the platform config + 'initialLifecycleState', // default route is stored/retrieved from the platform, not the engine + 'frameData', // framed data updates too often and would kick off too many config changes + ]; + + final String flutterDir = Platform.environment['FLUTTER_DIR']!; + final List classes = parseFile( + path: '$flutterDir/lib/ui/platform_dispatcher.dart', + featureSet: FeatureSet.latestLanguageVersion(), + throwIfDiagnostics: false, + ).unit.declarations.whereType().toList(); + final ClassDeclaration dispatcherClass = classes.singleWhere((ClassDeclaration c) => c.name.toString() == (PlatformDispatcher).toString()); + final ClassDeclaration configurationClass = classes.singleWhere((ClassDeclaration c) => c.name.toString() == (PlatformConfiguration).toString()); + + Iterable getNameOfClassMember(ClassMember member) { + if (member is MethodDeclaration) { + return [member.name.toString()]; + }else if (member is FieldDeclaration) { + return member.fields.variables.map((VariableDeclaration variable) => variable.name.toString()); + } else { + return []; + } + } + + final Set dispatcherMembers = dispatcherClass.members + .where((ClassMember member) => + member is FieldDeclaration || + member is MethodDeclaration && member.isGetter) + .expand(getNameOfClassMember) + .where((String name) => + exclusionCriteria.every((criteria) => + criteria.allMatches(name).isEmpty)) + .toSet(); + final Set configurationMembers = configurationClass.members + .where((ClassMember member) => + member is FieldDeclaration || + member is MethodDeclaration && member.isGetter) + .expand(getNameOfClassMember) + .toSet(); + + final Set missingMembers = configurationMembers.difference(dispatcherMembers); + final Set extraMembers = dispatcherMembers.difference(configurationMembers); + expect(missingMembers.isEmpty && extraMembers.isEmpty, isTrue, + reason: + 'PlatformDispatcher is out of sync with PlatformConfiguration.\n' + ' Missing Members: $missingMembers\n' + ' Members not backed by PlatformConfiguration: $extraMembers'); + }); } From 019c72cf8d95422949b2010ff81b46bb80c252f1 Mon Sep 17 00:00:00 2001 From: Paul Blasi Date: Thu, 23 Feb 2023 11:20:25 -0700 Subject: [PATCH 3/3] Removed test that required adding `analyzer` as a dependency. --- testing/dart/platform_dispatcher_test.dart | 61 ---------------------- 1 file changed, 61 deletions(-) diff --git a/testing/dart/platform_dispatcher_test.dart b/testing/dart/platform_dispatcher_test.dart index 1cdd4e7449e8f..dc19fba3fe57f 100644 --- a/testing/dart/platform_dispatcher_test.dart +++ b/testing/dart/platform_dispatcher_test.dart @@ -2,13 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:io'; import 'dart:ui'; -import 'package:analyzer/dart/analysis/features.dart'; -import 'package:analyzer/dart/analysis/results.dart'; -import 'package:analyzer/dart/analysis/utilities.dart'; -import 'package:analyzer/dart/ast/ast.dart'; import 'package:litetest/litetest.dart'; void main() { @@ -44,60 +39,4 @@ void main() { return ViewConfiguration(view: view)..copyWith(view: view, window: view); }); }); - - test("PlatformDispatcher and PlatformConfiguration have matching getters", () { - final List exclusionCriteria = [ - RegExp(r'^_.*'), // exclude any private members - RegExp(r'^on[A-Z].*'), // exclude any callbacks - RegExp(r'locale$'), // locale is a convenience getter for interacting with `locales` which _is_ backed by the config - 'instance', // exclude the static instance of PlatformDispatcher - 'configuration', // the configuration should not reference itself - 'views', // views are backed by their own config and don't need to be referenced in the platform config - 'initialLifecycleState', // default route is stored/retrieved from the platform, not the engine - 'frameData', // framed data updates too often and would kick off too many config changes - ]; - - final String flutterDir = Platform.environment['FLUTTER_DIR']!; - final List classes = parseFile( - path: '$flutterDir/lib/ui/platform_dispatcher.dart', - featureSet: FeatureSet.latestLanguageVersion(), - throwIfDiagnostics: false, - ).unit.declarations.whereType().toList(); - final ClassDeclaration dispatcherClass = classes.singleWhere((ClassDeclaration c) => c.name.toString() == (PlatformDispatcher).toString()); - final ClassDeclaration configurationClass = classes.singleWhere((ClassDeclaration c) => c.name.toString() == (PlatformConfiguration).toString()); - - Iterable getNameOfClassMember(ClassMember member) { - if (member is MethodDeclaration) { - return [member.name.toString()]; - }else if (member is FieldDeclaration) { - return member.fields.variables.map((VariableDeclaration variable) => variable.name.toString()); - } else { - return []; - } - } - - final Set dispatcherMembers = dispatcherClass.members - .where((ClassMember member) => - member is FieldDeclaration || - member is MethodDeclaration && member.isGetter) - .expand(getNameOfClassMember) - .where((String name) => - exclusionCriteria.every((criteria) => - criteria.allMatches(name).isEmpty)) - .toSet(); - final Set configurationMembers = configurationClass.members - .where((ClassMember member) => - member is FieldDeclaration || - member is MethodDeclaration && member.isGetter) - .expand(getNameOfClassMember) - .toSet(); - - final Set missingMembers = configurationMembers.difference(dispatcherMembers); - final Set extraMembers = dispatcherMembers.difference(configurationMembers); - expect(missingMembers.isEmpty && extraMembers.isEmpty, isTrue, - reason: - 'PlatformDispatcher is out of sync with PlatformConfiguration.\n' - ' Missing Members: $missingMembers\n' - ' Members not backed by PlatformConfiguration: $extraMembers'); - }); }