From d6d35ca7ccb6b5b23840380cae6c9e965d7654c0 Mon Sep 17 00:00:00 2001
From: Todd Volkert <tvolkert@users.noreply.github.com>
Date: Tue, 29 May 2018 14:45:40 -0700
Subject: [PATCH] Address post-review comments from #17977 (#18000)

---
 .../flutter/lib/src/foundation/debug.dart     | 45 +++++++------------
 .../flutter/test/foundation/debug_test.dart   | 26 +----------
 2 files changed, 18 insertions(+), 53 deletions(-)

diff --git a/packages/flutter/lib/src/foundation/debug.dart b/packages/flutter/lib/src/foundation/debug.dart
index eb13f69a6f..b5f585b7cf 100644
--- a/packages/flutter/lib/src/foundation/debug.dart
+++ b/packages/flutter/lib/src/foundation/debug.dart
@@ -42,38 +42,25 @@ bool debugInstrumentationEnabled = false;
 /// non-debug builds, or when [debugInstrumentationEnabled] is false, this will
 /// run [action] without any instrumentation.
 ///
-/// Returns the result of running [action], wrapped in a `Future` if the action
-/// was synchronous.
-Future<T> debugInstrumentAction<T>(String description, FutureOr<T> action()) {
-  if (!debugInstrumentationEnabled)
-    return new Future<T>.value(action());
-
-  Stopwatch stopwatch;
-  assert(() {
-    stopwatch = new Stopwatch()..start();
-    return true;
-  } ());
-  void stopStopwatchAndPrintElapsed() {
-    assert(() {
+/// Returns the result of running [action].
+///
+/// See also:
+///
+///   * [Timeline], which is used to record synchronous tracing events for
+///     visualization in Chrome's tracing format. This method does not
+///     implicitly add any timeline events.
+Future<T> debugInstrumentAction<T>(String description, Future<T> action()) {
+  bool instrument = false;
+  assert(() { instrument = debugInstrumentationEnabled; return true; }());
+  if (instrument) {
+    final Stopwatch stopwatch = new Stopwatch()..start();
+    return action().whenComplete(() {
       stopwatch.stop();
       debugPrint('Action "$description" took ${stopwatch.elapsed}');
-      return true;
-    }());
+    });
+  } else {
+    return action();
   }
-
-  Future<T> returnResult;
-  FutureOr<T> actionResult;
-  try {
-    actionResult = action();
-  } finally {
-    if (actionResult is Future<T>) {
-      returnResult = actionResult.whenComplete(stopStopwatchAndPrintElapsed);
-    } else {
-      stopStopwatchAndPrintElapsed();
-      returnResult = new Future<T>.value(actionResult);
-    }
-  }
-  return returnResult;
 }
 
 /// Arguments to whitelist [Timeline] events in order to be shown in the
diff --git a/packages/flutter/test/foundation/debug_test.dart b/packages/flutter/test/foundation/debug_test.dart
index 3be82e651c..2258cc8b1f 100644
--- a/packages/flutter/test/foundation/debug_test.dart
+++ b/packages/flutter/test/foundation/debug_test.dart
@@ -24,19 +24,7 @@ void main() {
       debugPrint = originalDebugPrintCallback;
     });
 
-    test('works with sync actions', () async {
-      final int result = await debugInstrumentAction<int>('no-op', () {
-        debugPrint('action()');
-        return 1;
-      });
-      expect(result, 1);
-      expect(
-        printBuffer.toString(),
-        matches(new RegExp('^action\\(\\)\nAction "no-op" took .+\$', multiLine: true)),
-      );
-    });
-
-    test('works with async actions', () async {
+    test('works with non-failing actions', () async {
       final int result = await debugInstrumentAction<int>('no-op', () async {
         debugPrint('action()');
         return 1;
@@ -48,17 +36,7 @@ void main() {
       );
     });
 
-    test('throws if sync action throws', () {
-      try {
-        debugInstrumentAction<void>('throws', () => throw 'Error');
-        fail('Error expected but not thrown');
-      } on String catch (error) {
-        expect(error, 'Error');
-        expect(printBuffer.toString(), matches(r'^Action "throws" took .+'));
-      }
-    });
-
-    test('returns failing future if async action throws', () async {
+    test('returns failing future if action throws', () async {
       try {
         await debugInstrumentAction<void>('throws', () async {
           await new Future<void>.delayed(Duration.zero);