Simplify SystemChrome.setSystemUIOverlayStyle() (#4653)

* Only schedule overlay style update microtask if needed

* Simplify API

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

@@ -4,7 +4,6 @@
 
 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';
 
@@ -89,74 +88,31 @@ class SystemChrome {
   /// 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.
-  ///
-  /// 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) {
+  static void setSystemUIOverlayStyle(SystemUiOverlayStyle style) {
     assert(style != null);
 
-    if (_pendingStyleUpdate != null) {
-      _pendingStyleUpdate.style = style;
-      return _pendingStyleUpdate.future;
+    if (_pendingStyle != null) {
+      // The microtask has already been queued; just update the pending value.
+      _pendingStyle = style;
+      return;
     }
 
-    _pendingStyleUpdate = new _PendingStyleUpdate(style);
+    if (style == _latestStyle) {
+      // Trivial success; no need to queue a microtask.
+      return;
+    }
+
+    _pendingStyle = style;
     scheduleMicrotask(() {
-      assert(_pendingStyleUpdate != null);
-      if (_pendingStyleUpdate.style == _latestStyle) {
-        // No update needed; trivial success.
-        _pendingStyleUpdate.complete(success: true);
-        _pendingStyleUpdate = null;
-        return;
+      assert(_pendingStyle != null);
+      if (_pendingStyle != _latestStyle) {
+        _systemChromeProxy.setSystemUiOverlayStyle(_pendingStyle);
+        _latestStyle = _pendingStyle;
       }
-
-      _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;
+      _pendingStyle = null;
     });
-
-    return _pendingStyleUpdate.future;
   }
 
-  static _PendingStyleUpdate _pendingStyleUpdate;
+  static SystemUiOverlayStyle _pendingStyle;
   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/test/services/system_chrome_test.dart

@@ -0,0 +1,23 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// 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_test/flutter_test.dart';
+import 'package:sky_services/flutter/platform/system_chrome.mojom.dart' as mojom;
+
+void main() {
+  testWidgets('SystemChrome overlay style test', (WidgetTester tester) async {
+    // The first call is a cache miss and will queue a microtask
+    SystemChrome.setSystemUIOverlayStyle(mojom.SystemUiOverlayStyle.light);
+    expect(tester.binding.microtaskCount, equals(1));
+
+    // Flush all microtasks
+    await tester.idle();
+    expect(tester.binding.microtaskCount, equals(0));
+
+    // The second call with the same value should be a no-op
+    SystemChrome.setSystemUIOverlayStyle(mojom.SystemUiOverlayStyle.light);
+    expect(tester.binding.microtaskCount, equals(0));
+  });
+}

packages/flutter_test/lib/src/binding.dart

@@ -112,6 +112,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
   /// Whether there is currently a test executing.
   bool get inTest;
 
+  /// The number of outstanding microtasks in the queue.
+  int get microtaskCount;
+
   /// The default test timeout for tests when using this binding.
   test_package.Timeout get defaultTestTimeout;
 
@@ -410,6 +413,9 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
   @override
   bool get inTest => _fakeAsync != null;
 
+  @override
+  int get microtaskCount => _fakeAsync.microtaskCount;
+
   @override
   Future<Null> pump([ Duration duration, EnginePhase newPhase = EnginePhase.sendSemanticsTree ]) {
     return TestAsyncUtils.guard(() {
@@ -544,6 +550,14 @@ class LiveTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding {
   bool get inTest => _inTest;
   bool _inTest = false;
 
+  @override
+  int get microtaskCount {
+    // Unsupported until we have a wrapper around the real async API
+    // https://github.com/flutter/flutter/issues/4637
+    assert(false);
+    return -1;
+  }
+
   @override
   test_package.Timeout get defaultTestTimeout => test_package.Timeout.none;