From cc3f5767f482c21c1b3f385bee2733c514839ff2 Mon Sep 17 00:00:00 2001 From: Yegor Date: Wed, 4 Oct 2017 21:50:32 -0700 Subject: [PATCH] ThemeData: optimize by removing polymorphism and caching; fix merging (#12249) * optimize ThemeData: make it monomorphic, memoize result * address comments * RLU cache; fix text theme merging * use FIFO cache for ThemeData; use HashMap to store inherited widgets * address comments --- .../flutter/lib/src/material/theme_data.dart | 374 +++++------------- .../flutter/lib/src/material/typography.dart | 88 ++--- .../flutter/lib/src/widgets/framework.dart | 4 +- .../flutter/test/material/theme_test.dart | 58 ++- 4 files changed, 210 insertions(+), 314 deletions(-) diff --git a/packages/flutter/lib/src/material/theme_data.dart b/packages/flutter/lib/src/material/theme_data.dart index 34280c05c0..15110fc5f9 100644 --- a/packages/flutter/lib/src/material/theme_data.dart +++ b/packages/flutter/lib/src/material/theme_data.dart @@ -48,6 +48,7 @@ const Color _kDarkThemeSplashColor = const Color(0x40CCCCCC); /// Use this class to configure a [Theme] widget. /// /// To obtain the current theme, use [Theme.of]. +@immutable class ThemeData { /// Create a ThemeData given a set of preferred values. /// @@ -418,50 +419,80 @@ class ThemeData { IconThemeData accentIconTheme, TargetPlatform platform, }) { - return _copyThemeDataWith( - this, - brightness: brightness, - primaryColor: primaryColor, - primaryColorBrightness: primaryColorBrightness, - accentColor: accentColor, - accentColorBrightness: accentColorBrightness, - canvasColor: canvasColor, - scaffoldBackgroundColor: scaffoldBackgroundColor, - cardColor: cardColor, - dividerColor: dividerColor, - highlightColor: highlightColor, - splashColor: splashColor, - selectedRowColor: selectedRowColor, - unselectedWidgetColor: unselectedWidgetColor, - disabledColor: disabledColor, - buttonColor: buttonColor, - secondaryHeaderColor: secondaryHeaderColor, - textSelectionColor: textSelectionColor, - textSelectionHandleColor: textSelectionHandleColor, - backgroundColor: backgroundColor, - dialogBackgroundColor: dialogBackgroundColor, - indicatorColor: indicatorColor, - hintColor: hintColor, - errorColor: errorColor, - textTheme: textTheme, - primaryTextTheme: primaryTextTheme, - accentTextTheme: accentTextTheme, - iconTheme: iconTheme, - primaryIconTheme: primaryIconTheme, - accentIconTheme: accentIconTheme, - platform: platform, + return new ThemeData.raw( + brightness: brightness ?? this.brightness, + primaryColor: primaryColor ?? this.primaryColor, + primaryColorBrightness: primaryColorBrightness ?? this.primaryColorBrightness, + accentColor: accentColor ?? this.accentColor, + accentColorBrightness: accentColorBrightness ?? this.accentColorBrightness, + canvasColor: canvasColor ?? this.canvasColor, + scaffoldBackgroundColor: scaffoldBackgroundColor ?? this.scaffoldBackgroundColor, + cardColor: cardColor ?? this.cardColor, + dividerColor: dividerColor ?? this.dividerColor, + highlightColor: highlightColor ?? this.highlightColor, + splashColor: splashColor ?? this.splashColor, + selectedRowColor: selectedRowColor ?? this.selectedRowColor, + unselectedWidgetColor: unselectedWidgetColor ?? this.unselectedWidgetColor, + disabledColor: disabledColor ?? this.disabledColor, + buttonColor: buttonColor ?? this.buttonColor, + secondaryHeaderColor: secondaryHeaderColor ?? this.secondaryHeaderColor, + textSelectionColor: textSelectionColor ?? this.textSelectionColor, + textSelectionHandleColor: textSelectionHandleColor ?? this.textSelectionHandleColor, + backgroundColor: backgroundColor ?? this.backgroundColor, + dialogBackgroundColor: dialogBackgroundColor ?? this.dialogBackgroundColor, + indicatorColor: indicatorColor ?? this.indicatorColor, + hintColor: hintColor ?? this.hintColor, + errorColor: errorColor ?? this.errorColor, + textTheme: textTheme ?? this.textTheme, + primaryTextTheme: primaryTextTheme ?? this.primaryTextTheme, + accentTextTheme: accentTextTheme ?? this.accentTextTheme, + iconTheme: iconTheme ?? this.iconTheme, + primaryIconTheme: primaryIconTheme ?? this.primaryIconTheme, + accentIconTheme: accentIconTheme ?? this.accentIconTheme, + platform: platform ?? this.platform, ); } + // The number 5 was chosen without any real science or research behind it. It + // just seemed like a number that's not too big (we should be able to fit 5 + // copies of ThemeData in memory comfortably) and not too small (most apps + // shouldn't have more than 5 theme/localization pairs). + static const int _localizedThemeDataCacheSize = 5; + + /// Caches localized themes to speed up the [localize] method. + static final _FifoCache<_IdentityThemeDataCacheKey, ThemeData> _localizedThemeDataCache = new _FifoCache<_IdentityThemeDataCacheKey, ThemeData>(_localizedThemeDataCacheSize); + /// Returns a new theme built by merging [baseTheme] into the text geometry /// provided by the [localTextGeometry]. /// /// The [TextStyle.inherit] field in the text styles provided by /// [localTextGeometry] must be set to true. static ThemeData localize(ThemeData baseTheme, TextTheme localTextGeometry) { + // WARNING: this method memoizes the result in a cache based on the + // previously seen baseTheme and localTextGeometry. Memoization is safe + // because all inputs and outputs of this function are deeply immutable, and + // the computations are referentially transparent. It only short-circuits + // the computation if the new inputs are identical() to the previous ones. + // It does not use the == operator, which performs a costly deep comparison. + // + // When changing this method, make sure the memoization logic is correct. + // Remember: + // + // There are only two hard things in Computer Science: cache invalidation + // and naming things. -- Phil Karlton assert(baseTheme != null); assert(localTextGeometry != null); - return new _LocalizedThemeData(baseTheme, localTextGeometry); + + return _localizedThemeDataCache.putIfAbsent( + new _IdentityThemeDataCacheKey(baseTheme, localTextGeometry), + () { + return baseTheme.copyWith( + primaryTextTheme: localTextGeometry.merge(baseTheme.primaryTextTheme), + accentTextTheme: localTextGeometry.merge(baseTheme.accentTextTheme), + textTheme: localTextGeometry.merge(baseTheme.textTheme), + ); + }, + ); } // See @@ -614,245 +645,58 @@ class ThemeData { String toString() => '$runtimeType(${ platform != defaultTargetPlatform ? "$platform " : ''}$brightness $primaryColor etc...)'; } -/// A lazily evaluated theme that provides the properties of the given -/// [delegate] theme localized using the properties of the given -/// [localTextGeometry]. -/// -/// The localization is done by merging of the [TextTheme] fields of the -/// [delegate] into the [localTextGeometry] and caching the results. -class _LocalizedThemeData implements ThemeData { - _LocalizedThemeData(this.delegate, this.localTextGeometry); +class _IdentityThemeDataCacheKey { + _IdentityThemeDataCacheKey(this.baseTheme, this.localTextGeometry); - final ThemeData delegate; + final ThemeData baseTheme; final TextTheme localTextGeometry; + // Using XOR to make the hash function as fast as possible (e.g. Jenkins is + // noticeably slower). @override - Color get accentColor => delegate.accentColor; + int get hashCode => identityHashCode(baseTheme) ^ identityHashCode(localTextGeometry); @override - Brightness get accentColorBrightness => delegate.accentColorBrightness; - - @override - IconThemeData get accentIconTheme => delegate.accentIconTheme; - - @override - Color get backgroundColor => delegate.backgroundColor; - - @override - Brightness get brightness => delegate.brightness; - - @override - Color get buttonColor => delegate.buttonColor; - - @override - Color get canvasColor => delegate.canvasColor; - - @override - Color get cardColor => delegate.cardColor; - - @override - Color get dialogBackgroundColor => delegate.dialogBackgroundColor; - - @override - Color get disabledColor => delegate.disabledColor; - - @override - Color get dividerColor => delegate.dividerColor; - - @override - Color get errorColor => delegate.errorColor; - - @override - Color get highlightColor => delegate.highlightColor; - - @override - Color get hintColor => delegate.hintColor; - - @override - IconThemeData get iconTheme => delegate.iconTheme; - - @override - Color get indicatorColor => delegate.indicatorColor; - - @override - TargetPlatform get platform => delegate.platform; - - @override - Color get primaryColor => delegate.primaryColor; - - @override - Brightness get primaryColorBrightness => delegate.primaryColorBrightness; - - @override - IconThemeData get primaryIconTheme => delegate.primaryIconTheme; - - @override - Color get scaffoldBackgroundColor => delegate.scaffoldBackgroundColor; - - @override - Color get secondaryHeaderColor => delegate.secondaryHeaderColor; - - @override - Color get selectedRowColor => delegate.selectedRowColor; - - @override - Color get splashColor => delegate.splashColor; - - @override - Color get textSelectionColor => delegate.textSelectionColor; - - @override - Color get textSelectionHandleColor => delegate.textSelectionHandleColor; - - @override - Color get unselectedWidgetColor => delegate.unselectedWidgetColor; - - @override - TextTheme get primaryTextTheme => _primaryTextTheme ??= delegate.primaryTextTheme.merge(localTextGeometry); - TextTheme _primaryTextTheme; - - @override - TextTheme get accentTextTheme => _accentTextTheme ??= delegate.accentTextTheme.merge(localTextGeometry); - TextTheme _accentTextTheme; - - @override - TextTheme get textTheme => _textTheme ??= delegate.textTheme.merge(localTextGeometry); - TextTheme _textTheme; - - /// This should be identical to [ThemeData.copyWith]. - @override - ThemeData copyWith({ - Brightness brightness, - Color primaryColor, - Brightness primaryColorBrightness, - Color accentColor, - Brightness accentColorBrightness, - Color canvasColor, - Color scaffoldBackgroundColor, - Color cardColor, - Color dividerColor, - Color highlightColor, - Color splashColor, - Color selectedRowColor, - Color unselectedWidgetColor, - Color disabledColor, - Color buttonColor, - Color secondaryHeaderColor, - Color textSelectionColor, - Color textSelectionHandleColor, - Color backgroundColor, - Color dialogBackgroundColor, - Color indicatorColor, - Color hintColor, - Color errorColor, - TextTheme textTheme, - TextTheme primaryTextTheme, - TextTheme accentTextTheme, - IconThemeData iconTheme, - IconThemeData primaryIconTheme, - IconThemeData accentIconTheme, - TargetPlatform platform, - }) { - return _copyThemeDataWith( - this, - brightness: brightness, - primaryColor: primaryColor, - primaryColorBrightness: primaryColorBrightness, - accentColor: accentColor, - accentColorBrightness: accentColorBrightness, - canvasColor: canvasColor, - scaffoldBackgroundColor: scaffoldBackgroundColor, - cardColor: cardColor, - dividerColor: dividerColor, - highlightColor: highlightColor, - splashColor: splashColor, - selectedRowColor: selectedRowColor, - unselectedWidgetColor: unselectedWidgetColor, - disabledColor: disabledColor, - buttonColor: buttonColor, - secondaryHeaderColor: secondaryHeaderColor, - textSelectionColor: textSelectionColor, - textSelectionHandleColor: textSelectionHandleColor, - backgroundColor: backgroundColor, - dialogBackgroundColor: dialogBackgroundColor, - indicatorColor: indicatorColor, - hintColor: hintColor, - errorColor: errorColor, - textTheme: textTheme, - primaryTextTheme: primaryTextTheme, - accentTextTheme: accentTextTheme, - iconTheme: iconTheme, - primaryIconTheme: primaryIconTheme, - accentIconTheme: accentIconTheme, - platform: platform, - ); + bool operator ==(Object other) { + // We are explicitly ignoring the possibility that the types might not + // match in the interests of speed. + final _IdentityThemeDataCacheKey otherKey = other; + return identical(baseTheme, otherKey.baseTheme) && identical(localTextGeometry, otherKey.localTextGeometry); } } -/// Implementation of [ThemeData.copyWith], shared with [_LocalizedThemeData.copyWith]. -ThemeData _copyThemeDataWith( - ThemeData base, { - @required Brightness brightness, - @required Color primaryColor, - @required Brightness primaryColorBrightness, - @required Color accentColor, - @required Brightness accentColorBrightness, - @required Color canvasColor, - @required Color scaffoldBackgroundColor, - @required Color cardColor, - @required Color dividerColor, - @required Color highlightColor, - @required Color splashColor, - @required Color selectedRowColor, - @required Color unselectedWidgetColor, - @required Color disabledColor, - @required Color buttonColor, - @required Color secondaryHeaderColor, - @required Color textSelectionColor, - @required Color textSelectionHandleColor, - @required Color backgroundColor, - @required Color dialogBackgroundColor, - @required Color indicatorColor, - @required Color hintColor, - @required Color errorColor, - @required TextTheme textTheme, - @required TextTheme primaryTextTheme, - @required TextTheme accentTextTheme, - @required IconThemeData iconTheme, - @required IconThemeData primaryIconTheme, - @required IconThemeData accentIconTheme, - @required TargetPlatform platform, -}) { - return new ThemeData.raw( - brightness: brightness ?? base.brightness, - primaryColor: primaryColor ?? base.primaryColor, - primaryColorBrightness: primaryColorBrightness ?? base.primaryColorBrightness, - accentColor: accentColor ?? base.accentColor, - accentColorBrightness: accentColorBrightness ?? base.accentColorBrightness, - canvasColor: canvasColor ?? base.canvasColor, - scaffoldBackgroundColor: scaffoldBackgroundColor ?? base.scaffoldBackgroundColor, - cardColor: cardColor ?? base.cardColor, - dividerColor: dividerColor ?? base.dividerColor, - highlightColor: highlightColor ?? base.highlightColor, - splashColor: splashColor ?? base.splashColor, - selectedRowColor: selectedRowColor ?? base.selectedRowColor, - unselectedWidgetColor: unselectedWidgetColor ?? base.unselectedWidgetColor, - disabledColor: disabledColor ?? base.disabledColor, - buttonColor: buttonColor ?? base.buttonColor, - secondaryHeaderColor: secondaryHeaderColor ?? base.secondaryHeaderColor, - textSelectionColor: textSelectionColor ?? base.textSelectionColor, - textSelectionHandleColor: textSelectionHandleColor ?? base.textSelectionHandleColor, - backgroundColor: backgroundColor ?? base.backgroundColor, - dialogBackgroundColor: dialogBackgroundColor ?? base.dialogBackgroundColor, - indicatorColor: indicatorColor ?? base.indicatorColor, - hintColor: hintColor ?? base.hintColor, - errorColor: errorColor ?? base.errorColor, - textTheme: textTheme ?? base.textTheme, - primaryTextTheme: primaryTextTheme ?? base.primaryTextTheme, - accentTextTheme: accentTextTheme ?? base.accentTextTheme, - iconTheme: iconTheme ?? base.iconTheme, - primaryIconTheme: primaryIconTheme ?? base.primaryIconTheme, - accentIconTheme: accentIconTheme ?? base.accentIconTheme, - platform: platform ?? base.platform, - ); +/// Cache of objects of limited size that uses the first in first out eviction +/// strategy (a.k.a least recently inserted). +/// +/// The key that was inserted before all other keys is evicted first, i.e. the +/// one inserted least recently. +class _FifoCache { + _FifoCache(this._maximumSize) + : assert(_maximumSize != null && _maximumSize > 0); + + /// In Dart the map literal uses a linked hash-map implementation, whose keys + /// are stored such that [Map.keys] returns them in the order they were + /// inserted. + final Map _cache = {}; + + /// Maximum number of entries to store in the cache. + /// + /// Once this many entries have been cached, the entry inserted least recently + /// is evicted when adding a new entry. + final int _maximumSize; + + /// Returns the previously cached value for the given key, if available; + /// if not, calls the given callback to obtain it first. + /// + /// The arguments must not be null. + V putIfAbsent(K key, V loader()) { + assert(key != null); + assert(loader != null); + final V result = _cache[key]; + if (result != null) + return result; + if (_cache.length == _maximumSize) + _cache.remove(_cache.keys.first); + return _cache[key] = loader(); + } } diff --git a/packages/flutter/lib/src/material/typography.dart b/packages/flutter/lib/src/material/typography.dart index dd3a6e498a..8e16c31572 100644 --- a/packages/flutter/lib/src/material/typography.dart +++ b/packages/flutter/lib/src/material/typography.dart @@ -353,59 +353,59 @@ class Typography { // TODO(yjbanov): implement font fallback (see "Font stack" at https://material.io/guidelines/style/typography.html) class _MaterialTextColorThemes { static const TextTheme blackMountainView = const TextTheme( - display4: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black54), - display3: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black54), - display2: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black54), - display1: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black54), - headline: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black87), - title : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black87), - subhead : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black87), - body2 : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black87), - body1 : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black87), - caption : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black54), - button : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.black87), + display4: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black54), + display3: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black54), + display2: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black54), + display1: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black54), + headline: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black87), + title : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black87), + subhead : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black87), + body2 : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black87), + body1 : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black87), + caption : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black54), + button : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.black87), ); static const TextTheme whiteMountainView = const TextTheme( - display4: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white70), - display3: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white70), - display2: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white70), - display1: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white70), - headline: const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white), - title : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white), - subhead : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white), - body2 : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white), - body1 : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white), - caption : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white70), - button : const TextStyle(fontFamily: 'Roboto', inherit: false, color: Colors.white), + display4: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white70), + display3: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white70), + display2: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white70), + display1: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white70), + headline: const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white), + title : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white), + subhead : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white), + body2 : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white), + body1 : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white), + caption : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white70), + button : const TextStyle(fontFamily: 'Roboto', inherit: true, color: Colors.white), ); static const TextTheme blackCupertino = const TextTheme( - display4: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.black54), - display3: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.black54), - display2: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.black54), - display1: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.black54), - headline: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.black87), - title : const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.black87), - subhead : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.black87), - body2 : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.black87), - body1 : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.black87), - caption : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.black54), - button : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.black87), + display4: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.black54), + display3: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.black54), + display2: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.black54), + display1: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.black54), + headline: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.black87), + title : const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.black87), + subhead : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.black87), + body2 : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.black87), + body1 : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.black87), + caption : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.black54), + button : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.black87), ); static const TextTheme whiteCupertino = const TextTheme( - display4: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.white70), - display3: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.white70), - display2: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.white70), - display1: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.white70), - headline: const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.white), - title : const TextStyle(fontFamily: '.SF UI Display', inherit: false, color: Colors.white), - subhead : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.white), - body2 : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.white), - body1 : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.white), - caption : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.white70), - button : const TextStyle(fontFamily: '.SF UI Text', inherit: false, color: Colors.white), + display4: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.white70), + display3: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.white70), + display2: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.white70), + display1: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.white70), + headline: const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.white), + title : const TextStyle(fontFamily: '.SF UI Display', inherit: true, color: Colors.white), + subhead : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.white), + body2 : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.white), + body1 : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.white), + caption : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.white70), + button : const TextStyle(fontFamily: '.SF UI Text', inherit: true, color: Colors.white), ); } diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index f04545f50e..909a01fc72 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -3911,9 +3911,9 @@ class InheritedElement extends ProxyElement { assert(_active); final Map incomingWidgets = _parent?._inheritedWidgets; if (incomingWidgets != null) - _inheritedWidgets = new Map.from(incomingWidgets); + _inheritedWidgets = new HashMap.from(incomingWidgets); else - _inheritedWidgets = {}; + _inheritedWidgets = new HashMap(); _inheritedWidgets[widget.runtimeType] = this; } diff --git a/packages/flutter/test/material/theme_test.dart b/packages/flutter/test/material/theme_test.dart index 51cedbc1a3..a993da227a 100644 --- a/packages/flutter/test/material/theme_test.dart +++ b/packages/flutter/test/material/theme_test.dart @@ -53,12 +53,33 @@ void main() { ) ); - final dynamic localizedTheme = Theme.of(capturedContext); - expect('${localizedTheme.runtimeType}', '_LocalizedThemeData'); - expect(localizedTheme.delegate, equals(new ThemeData.fallback())); + expect(Theme.of(capturedContext), equals(ThemeData.localize(new ThemeData.fallback(), MaterialTextGeometry.englishLike))); expect(Theme.of(capturedContext, shadowThemeOnly: true), isNull); }); + testWidgets('ThemeData.localize memoizes the result', (WidgetTester tester) async { + final ThemeData light = new ThemeData.light(); + final ThemeData dark = new ThemeData.dark(); + + // Same input, same output. + expect( + ThemeData.localize(light, MaterialTextGeometry.englishLike), + same(ThemeData.localize(light, MaterialTextGeometry.englishLike)), + ); + + // Different text geometry, different output. + expect( + ThemeData.localize(light, MaterialTextGeometry.englishLike), + isNot(same(ThemeData.localize(light, MaterialTextGeometry.tall))), + ); + + // Different base theme, different output. + expect( + ThemeData.localize(light, MaterialTextGeometry.englishLike), + isNot(same(ThemeData.localize(dark, MaterialTextGeometry.englishLike))), + ); + }); + testWidgets('PopupMenu inherits shadowed app theme', (WidgetTester tester) async { // Regression test for https://github.com/flutter/flutter/issues/5572 final Key popupMenuButtonKey = new UniqueKey(); @@ -315,6 +336,37 @@ void main() { expect(testBuildCalled, 2); }, ); + + testWidgets('Text geometry set in Theme has higher precedence than that of Localizations', (WidgetTester tester) async { + const double _kMagicFontSize = 4321.0; + final ThemeData fallback = new ThemeData.fallback(); + final ThemeData customTheme = fallback.copyWith( + primaryTextTheme: fallback.primaryTextTheme.copyWith( + body1: fallback.primaryTextTheme.body1.copyWith( + fontSize: _kMagicFontSize, + ) + ), + ); + expect(customTheme.primaryTextTheme.body1.fontSize, _kMagicFontSize); + + double actualFontSize; + await tester.pumpWidget(new Directionality( + textDirection: TextDirection.ltr, + child: new Theme( + data: customTheme, + child: new Builder(builder: (BuildContext context) { + final ThemeData theme = Theme.of(context); + actualFontSize = theme.primaryTextTheme.body1.fontSize; + return new Text( + 'A', + style: theme.primaryTextTheme.body1, + ); + }), + ), + )); + + expect(actualFontSize, _kMagicFontSize); + }); } int testBuildCalled;