System chrome overlay style fixes (#4627)

* System chrome overlay style fixes

1) Add a cache to avoid superfluous calls to the embedder
2) Coalesce calls to the embedder that are made in the same event loop
3) Move call site to material's AppBar from Title widget

#4461

packages/flutter/lib/src/material/app.dart

@@ -177,7 +177,6 @@ class _MaterialAppState extends State<MaterialApp> {
       child: new WidgetsApp(
         title: config.title,
         textStyle: _errorTextStyle,
-        brightness: theme.primaryColorBrightness,
         color: theme?.primaryColor ?? Colors.blue[500], // blue[500] is the primary color of the default theme
         navigatorObserver: _heroController,
         onGenerateRoute: _onGenerateRoute,

packages/flutter/lib/src/material/app_bar.dart

@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:flutter/services.dart';
 import 'package:flutter/widgets.dart';
+import 'package:sky_services/flutter/platform/system_chrome.mojom.dart' as mojom;
 
 import 'constants.dart';
 import 'icon_theme.dart';
@@ -54,6 +56,7 @@ class AppBar extends StatelessWidget {
     this.tabBar,
     this.elevation: 4,
     this.backgroundColor,
+    this.brightness,
     this.textTheme,
     this.padding: EdgeInsets.zero,
     double expandedHeight,
@@ -102,11 +105,18 @@ class AppBar extends StatelessWidget {
   /// The following elevations have defined shadows: 1, 2, 3, 4, 6, 8, 9, 12, 16, 24
   final int elevation;
 
-  /// The color to use for the the app bar's material.
+  /// The color to use for the app bar's material. This generally should be set
+  /// in tandem with [brightness].
   ///
   /// Defaults to [ThemeData.primaryColor].
   final Color backgroundColor;
 
+  /// The brightness of the app bar's material. This generally should be set in
+  /// tandem with [backgroundColor].
+  ///
+  /// Defaults to [ThemeData.brightness].
+  final Brightness brightness;
+
   /// The typographic style to use for text in the app bar.
   ///
   /// Defaults to [ThemeData.primaryTextTheme].
@@ -130,6 +140,7 @@ class AppBar extends StatelessWidget {
     Widget flexibleSpace,
     int elevation,
     Color backgroundColor,
+    Brightness brightness,
     TextTheme textTheme,
     EdgeInsets padding,
     double expandedHeight,
@@ -144,6 +155,7 @@ class AppBar extends StatelessWidget {
       tabBar: tabBar ?? this.tabBar,
       elevation: elevation ?? this.elevation,
       backgroundColor: backgroundColor ?? this.backgroundColor,
+      brightness: brightness ?? this.brightness,
       textTheme: textTheme ?? this.textTheme,
       padding: padding ?? this.padding,
       expandedHeight: expandedHeight ?? this._expandedHeight,
@@ -188,6 +200,11 @@ class AppBar extends StatelessWidget {
     TextStyle centerStyle = textTheme?.title ?? theme.primaryTextTheme.title;
     TextStyle sideStyle = textTheme?.body1 ?? theme.primaryTextTheme.body1;
 
+    Brightness brightness = this.brightness ?? theme.brightness;
+    SystemChrome.setSystemUIOverlayStyle(brightness == Brightness.dark
+      ? mojom.SystemUiOverlayStyle.light
+      : mojom.SystemUiOverlayStyle.dark);
+
     final double toolBarOpacity = _toolBarOpacity(size.height, statusBarHeight);
     if (toolBarOpacity != 1.0) {
       final double opacity = const Interval(0.25, 1.0, curve: Curves.ease).transform(toolBarOpacity);

packages/flutter/lib/src/services/system_chrome.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:meta/meta.dart';
 import 'package:sky_services/flutter/platform/system_chrome.mojom.dart' as mojom;
 import 'package:sky_services/flutter/platform/system_chrome.mojom.dart';
 
@@ -85,21 +86,77 @@ class SystemChrome {
   /// embedder (if any). The embedder may choose to ignore unsupported
   /// overlays.
   ///
-  /// Arguments:
-  ///
-  ///  * [style]: A [SystemUiOverlayStyle] enum value denoting the style to use
-  ///
-  /// Return Value:
-  ///
-  ///   boolean indicating if the preference was conveyed successfully to the
-  ///   embedder.
+  /// This method will schedule the embedder update to be run in a microtask.
+  /// Any subsequent calls to this method during the current event loop will
+  /// overwrite the pending value to be set on the embedder.
   ///
-  /// Platform Specific Notes:
-  ///
-  ///   If the overlay is unsupported on the platform, enabling or disabling
-  ///   that overlay is a no-op and always return true.
-  static Future<bool> setSystemUIOverlayStyle(SystemUiOverlayStyle style) async {
-    return (await _systemChromeProxy.setSystemUiOverlayStyle(
-      style)).success;
+  /// The return value indicates both the preference that was eventually
+  /// conveyed to the embedder, along with whether it was successfully
+  /// conveyed.
+  static Future<SystemUiOverlayStyleUpdate> setSystemUIOverlayStyle(SystemUiOverlayStyle style) {
+    assert(style != null);
+
+    if (_pendingStyleUpdate != null) {
+      _pendingStyleUpdate.style = style;
+      return _pendingStyleUpdate.future;
+    }
+
+    _pendingStyleUpdate = new _PendingStyleUpdate(style);
+    scheduleMicrotask(() {
+      assert(_pendingStyleUpdate != null);
+      if (_pendingStyleUpdate.style == _latestStyle) {
+        // No update needed; trivial success.
+        _pendingStyleUpdate.complete(success: true);
+        _pendingStyleUpdate = null;
+        return;
+      }
+
+      _PendingStyleUpdate update = _pendingStyleUpdate;
+      _systemChromeProxy.setSystemUiOverlayStyle(update.style)
+        .then((SystemChromeSetSystemUiOverlayStyleResponseParams value) {
+          update.complete(success: value.success);
+        }, onError: (_) {
+          update.complete(success: false);
+        });
+      _latestStyle = _pendingStyleUpdate.style;
+      _pendingStyleUpdate = null;
+    });
+
+    return _pendingStyleUpdate.future;
+  }
+
+  static _PendingStyleUpdate _pendingStyleUpdate;
+  static SystemUiOverlayStyle _latestStyle;
+}
+
+/// Struct that represents an attempted update to the system overlays that are
+/// visible on the embedder.
+class SystemUiOverlayStyleUpdate {
+  const SystemUiOverlayStyleUpdate._({
+    @required this.style,
+    @required this.success
+  });
+
+  /// The style that was passed to the embedder.
+  final SystemUiOverlayStyle style;
+
+  /// Whether the preference was successfully conveyed to the embedder.
+  final bool success;
+}
+
+class _PendingStyleUpdate {
+  _PendingStyleUpdate(this.style);
+
+  final Completer<SystemUiOverlayStyleUpdate> _completer =
+    new Completer<SystemUiOverlayStyleUpdate>();
+  SystemUiOverlayStyle style;
+
+  Future<SystemUiOverlayStyleUpdate> get future => _completer.future;
+
+  void complete({@required bool success}) {
+    _completer.complete(new SystemUiOverlayStyleUpdate._(
+      style: style,
+      success: success
+    ));
   }
 }

packages/flutter/lib/src/widgets/app.dart

@@ -39,7 +39,6 @@ class WidgetsApp extends StatefulWidget {
     Key key,
     @required this.onGenerateRoute,
     this.title,
-    this.brightness,
     this.textStyle,
     this.color,
     this.navigatorObserver,
@@ -56,9 +55,6 @@ class WidgetsApp extends StatefulWidget {
   /// A one-line description of this app for use in the window manager.
   final String title;
 
-  /// The overall brightness of the app, describing its contrast needs.
-  final Brightness brightness;
-
   /// The default text style for [Text] in the application.
   final TextStyle textStyle;
 
@@ -180,7 +176,6 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv
         data: _localeData,
         child: new Title(
           title: config.title,
-          brightness: config.brightness,
           color: config.color,
           child: new Navigator(
             key: _navigator,

packages/flutter/lib/src/widgets/title.dart

@@ -4,7 +4,6 @@
 
 import 'package:flutter/services.dart';
 import 'package:flutter/widgets.dart';
-import 'package:sky_services/flutter/platform/system_chrome.mojom.dart' as mojom;
 
 /// A widget that describes this app in the operating system.
 class Title extends StatelessWidget {
@@ -12,7 +11,6 @@ class Title extends StatelessWidget {
   Title({
     Key key,
     this.title,
-    this.brightness,
     this.color,
     this.child
   }) : super(key: key) {
@@ -22,9 +20,6 @@ class Title extends StatelessWidget {
   /// A one-line description of this app for use in the window manager.
   final String title;
 
-  /// The brightness against which the window manager should render system text.
-  final Brightness brightness;
-
   /// A color that the window manager should use to identify this app.
   final Color color;
 
@@ -33,22 +28,10 @@ class Title extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    _updateSystemChrome();
     updateTaskDescription(label: title, color: color);
     return child;
   }
 
-  /// Updates the system chrome settings based on this title's metadata.
-  void _updateSystemChrome() {
-    // TODO(tvolkert): This may result in a decent amount of unnecessary
-    // overhead in the embedder (on every build). Consider making Title
-    // a StatefulWidget that only calls into the SystemChrome service when
-    // it sees that a value has changed.
-    SystemChrome.setSystemUIOverlayStyle(brightness == Brightness.dark
-        ? mojom.SystemUiOverlayStyle.light
-        : mojom.SystemUiOverlayStyle.dark);
-  }
-
   @override
   void debugFillDescription(List<String> description) {
     super.debugFillDescription(description);