Revert "Let flutter attach find the service port by looking through old logs again (#53153)" (#53282)

This reverts commit 0c5ffdc9efc4e04ed637645014465b656afc14a5.

dev/devicelab/bin/tasks/flutter_attach_test_android.dart

@@ -57,14 +57,10 @@ Future<void> testReload(Process process, { Future<void> Function() onListening }
   }
 
   await eventOrExit(listening.future);
-  await eventOrExit(ready.future).timeout(const Duration(seconds: 5), onTimeout: () {
+  await eventOrExit(ready.future);
-    // If it can't attach in 5 seconds, it's not capable of finding the
-    // observatory URL in the logs.
-    throw TaskResult.failure('Failed to attach to running Flutter process');
-  });
 
   if (exitCode != null)
-    throw TaskResult.failure('Failed to attach to test app; command unexpected exited, with exit code $exitCode.');
+    throw 'Failed to attach to test app; command unexpected exited, with exit code $exitCode.';
 
   process.stdin.write('r');
   process.stdin.flush();
@@ -79,10 +75,10 @@ Future<void> testReload(Process process, { Future<void> Function() onListening }
   await process.exitCode;
 
   if (stderr.isNotEmpty)
-    throw TaskResult.failure('flutter attach had output on standard error.');
+    throw 'flutter attach had output on standard error.';
 
   if (exitCode != 0)
-    throw TaskResult.failure('exit code was not 0');
+    throw 'exit code was not 0';
 }
 
 void main() {
@@ -124,7 +120,7 @@ void main() {
         // After the delay, force-stopping it shouldn't do anything, but doesn't hurt.
         await device.shellExec('am', <String>['force-stop', kAppId]);
 
-        String currentTime = (await device.shellEval('date', <String>['"+%F %R:%S.000"'])).trim();
+        final String currentTime = (await device.shellEval('date', <String>['"+%F %R:%S.000"'])).trim();
         print('Start time on device: $currentTime');
         section('Relaunching application');
         await device.shellExec('am', <String>['start', '-n', kActivityId]);
@@ -144,26 +140,6 @@ void main() {
         );
         await testReload(attachProcess);
 
-        // Give the device the time to really shut down the app.
-        await Future<void>.delayed(const Duration(milliseconds: 200));
-        // After the delay, force-stopping it shouldn't do anything, but doesn't hurt.
-        await device.shellExec('am', <String>['force-stop', kAppId]);
-
-        section('Attaching after relaunching application');
-        await device.shellExec('am', <String>['start', '-n', kActivityId]);
-
-        // Let the application launch. Sync to the next time an observatory is ready.
-        currentTime = (await device.shellEval('date', <String>['"+%F %R:%S.000"'])).trim();
-        await device.adb(<String>['logcat', '-e', 'Observatory listening on http:', '-m', '1', '-T', currentTime]);
-
-        // Attach again now that the VM is already running.
-        attachProcess = await startProcess(
-          path.join(flutterDirectory.path, 'bin', 'flutter'),
-          <String>['--suppress-analytics', 'attach', '-d', device.deviceId],
-          isBot: false, // we just want to test the output, not have any debugging info
-        );
-        // Verify that it can discover the observatory port from past logs.
-        await testReload(attachProcess);
       } finally {
         section('Uninstalling');
         await device.adb(<String>['uninstall', kAppId]);

packages/flutter_tools/doc/attach.md

@@ -9,17 +9,19 @@ without `flutter run` and provides a HotRunner (enabling hot reload/restart).
 
 There are four ways for the attach command to discover a running app:
 
+1. If the app is already running and the observatory port is known, it can be
+explicitly provided to attach via the command-line, e.g. `$ flutter attach
+--debug-port 12345`
+1. If the app is already running and the platform is iOS, attach can use mDNS
+to lookup the observatory port via the application ID, with just `$ flutter
+attach`
 1. If the platform is Fuchsia the module name must be provided, e.g. `$
 flutter attach --module=mod_name`. This can be called either before or after
 the application is started, attach will poll the device if it cannot
 immediately discover the port
-1. On Android and iOS, just running `flutter attach` suffices. Flutter tools
+1. On other platforms (i.e. Android), if the app is not yet running attach
-will search for an already running Flutter app or module if available.
+will listen and wait for the app to be (manually) started with the default
-Otherwise, the tool will wait for the next Flutter app or module to launch
+command: `$ flutter attach`
-before attaching.
-1. If the app or module is already running and the specific observatory port is
-known, it can be explicitly provided to attach via the command-line, e.g.
-`$ flutter attach --debug-port 12345`
 
 ## Source
 

packages/flutter_tools/lib/src/android/android_device.dart

@@ -213,7 +213,6 @@ class AndroidDevice extends Device {
   Future<String> get apiVersion => _getProperty('ro.build.version.sdk');
 
   AdbLogReader _logReader;
-  AdbLogReader _pastLogReader;
   _AndroidDevicePortForwarder _portForwarder;
 
   List<String> adbCommandForDevice(List<String> args) {
@@ -674,23 +673,9 @@ class AndroidDevice extends Device {
   }
 
   @override
-  FutureOr<DeviceLogReader> getLogReader({
+  FutureOr<DeviceLogReader> getLogReader({ AndroidApk app }) async {
-    AndroidApk app,
+    // The Android log reader isn't app-specific.
-    bool includePastLogs = false,
+    return _logReader ??= await AdbLogReader.createLogReader(this, globals.processManager);
-  }) async {
-    // The Android log reader isn't app-specific. The `app` parameter isn't used.
-    if (includePastLogs) {
-      return _pastLogReader ??= await AdbLogReader.createLogReader(
-        this,
-        globals.processManager,
-        includePastLogs: true,
-      );
-    } else {
-      return _logReader ??= await AdbLogReader.createLogReader(
-        this,
-        globals.processManager,
-      );
-    }
   }
 
   @override
@@ -739,7 +724,6 @@ class AndroidDevice extends Device {
   @override
   Future<void> dispose() async {
     _logReader?._stop();
-    _pastLogReader?._stop();
     await _portForwarder?.dispose();
   }
 }
@@ -917,9 +901,6 @@ class AdbLogReader extends DeviceLogReader {
   static Future<AdbLogReader> createLogReader(
     AndroidDevice device,
     ProcessManager processManager,
-    {
-      bool includePastLogs = false,
-    }
   ) async {
     // logcat -T is not supported on Android releases before Lollipop.
     const int kLollipopVersionCode = 21;
@@ -934,12 +915,7 @@ class AdbLogReader extends DeviceLogReader {
       'logcat',
       '-v',
       'time',
-      // If we include logs from the past, filter for 'flutter' logs only.
+      if (apiVersion != null && apiVersion >= kLollipopVersionCode) ...<String>[
-      if (includePastLogs) ...<String>[
-        '-s',
-        'flutter',
-      ] else if (apiVersion != null && apiVersion >= kLollipopVersionCode) ...<String>[
-        // Otherwise, filter for logs appearing past the present.
         // Empty `-T` means the timestamp of the logcat command invocation.
         '-T',
         device.lastLogcatTimestamp ?? '',

packages/flutter_tools/lib/src/commands/attach.dart

@@ -6,7 +6,6 @@ import 'dart:async';
 
 import 'package:meta/meta.dart';
 
-import '../android/android_device.dart';
 import '../artifacts.dart';
 import '../base/common.dart';
 import '../base/context.dart';
@@ -246,9 +245,7 @@ class AttachCommand extends FlutterCommand {
       if (observatoryUri == null) {
         final ProtocolDiscovery observatoryDiscovery =
           ProtocolDiscovery.observatory(
-            // If it's an Android device, attaching relies on past log searching
+            await device.getLogReader(),
-            // to find the service protocol.
-            await device.getLogReader(includePastLogs: device is AndroidDevice),
             portForwarder: device.portForwarder,
             ipv6: ipv6,
             devicePort: deviceVmservicePort,

packages/flutter_tools/lib/src/desktop_device.dart

@@ -63,11 +63,7 @@ abstract class DesktopDevice extends Device {
   Future<String> get sdkNameAndVersion async => globals.os.name;
 
   @override
-  DeviceLogReader getLogReader({
+  DeviceLogReader getLogReader({ ApplicationPackage app }) {
-    ApplicationPackage app,
-    bool includePastLogs = false,
-  }) {
-    assert(!includePastLogs, 'Past log reading not supported on desktop.');
     return _deviceLogReader;
   }
 

packages/flutter_tools/lib/src/device.dart

@@ -415,17 +415,9 @@ abstract class Device {
   Future<String> get sdkNameAndVersion;
 
   /// Get a log reader for this device.
-  ///
+  /// If [app] is specified, this will return a log reader specific to that
-  /// If `app` is specified, this will return a log reader specific to that
   /// application. Otherwise, a global log reader will be returned.
-  ///
+  FutureOr<DeviceLogReader> getLogReader({ covariant ApplicationPackage app });
-  /// If `includePastLogs` is true and the device type supports it, the log
-  /// reader will also include log messages from before the invocation time.
-  /// Defaults to false.
-  FutureOr<DeviceLogReader> getLogReader({
-    covariant ApplicationPackage app,
-    bool includePastLogs = false,
-  });
 
   /// Get the port forwarder for this device.
   DevicePortForwarder get portForwarder;

packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart

@@ -508,13 +508,8 @@ class FuchsiaDevice extends Device {
   }
 
   @override
-  DeviceLogReader getLogReader({
+  DeviceLogReader getLogReader({ApplicationPackage app}) =>
-    ApplicationPackage app,
+      _logReader ??= _FuchsiaLogReader(this, app);
-    bool includePastLogs = false,
-  }) {
-    assert(!includePastLogs, 'Past log reading not supported on Fuchsia.');
-    return _logReader ??= _FuchsiaLogReader(this, app);
-  }
   _FuchsiaLogReader _logReader;
 
   @override

packages/flutter_tools/lib/src/ios/devices.dart

@@ -372,11 +372,7 @@ class IOSDevice extends Device {
   Future<String> get sdkNameAndVersion async => 'iOS $_sdkVersion';
 
   @override
-  DeviceLogReader getLogReader({
+  DeviceLogReader getLogReader({ IOSApp app }) {
-    IOSApp app,
-    bool includePastLogs = false,
-  }) {
-    assert(!includePastLogs, 'Past log reading not supported on iOS devices.');
     _logReaders ??= <IOSApp, DeviceLogReader>{};
     return _logReaders.putIfAbsent(app, () => IOSDeviceLogReader.create(
       device: this,

packages/flutter_tools/lib/src/ios/simulators.dart

@@ -519,12 +519,8 @@ class IOSSimulator extends Device {
   }
 
   @override
-  DeviceLogReader getLogReader({
+  DeviceLogReader getLogReader({ covariant IOSApp app }) {
-    covariant IOSApp app,
-    bool includePastLogs = false,
-  }) {
     assert(app is IOSApp);
-    assert(!includePastLogs, 'Past log reading not supported on iOS simulators.');
     _logReaders ??= <ApplicationPackage, _IOSSimulatorLogReader>{};
     return _logReaders.putIfAbsent(app, () => _IOSSimulatorLogReader(this, app));
   }

packages/flutter_tools/lib/src/tester/flutter_tester.dart

@@ -79,12 +79,7 @@ class FlutterTesterDevice extends Device {
       _FlutterTesterDeviceLogReader();
 
   @override
-  DeviceLogReader getLogReader({
+  DeviceLogReader getLogReader({ ApplicationPackage app }) => _logReader;
-    ApplicationPackage app,
-    bool includePastLogs = false,
-  }) {
-    return _logReader;
-  }
 
   @override
   Future<bool> installApp(ApplicationPackage app) async => true;

packages/flutter_tools/lib/src/web/web_device.dart

@@ -60,10 +60,7 @@ class ChromeDevice extends Device {
   DeviceLogReader _logReader;
 
   @override
-  DeviceLogReader getLogReader({
+  DeviceLogReader getLogReader({ApplicationPackage app}) {
-    ApplicationPackage app,
-    bool includePastLogs = false,
-  }) {
     return _logReader ??= NoOpDeviceLogReader(app?.name);
   }
 
@@ -224,10 +221,7 @@ class WebServerDevice extends Device {
   DeviceLogReader _logReader;
 
   @override
-  DeviceLogReader getLogReader({
+  DeviceLogReader getLogReader({ApplicationPackage app}) {
-    ApplicationPackage app,
-    bool includePastLogs = false,
-  }) {
     return _logReader ??= NoOpDeviceLogReader(app?.name);
   }
 

packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart

@@ -97,8 +97,7 @@ void main() {
       });
 
       testUsingContext('finds observatory port and forwards', () async {
-        when(device.getLogReader(includePastLogs: anyNamed('includePastLogs')))
+        when(device.getLogReader()).thenAnswer((_) {
-          .thenAnswer((_) {
           // Now that the reader is used, start writing messages to it.
           mockLogReader.addLine('Foo');
           mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort');
@@ -135,9 +134,7 @@ void main() {
           ..writeAsStringSync('{}');
 
         when(device.name).thenReturn('MockAndroidDevice');
-        when(device.getLogReader(includePastLogs: anyNamed('includePastLogs')))
+        when(device.getLogReader()).thenReturn(mockLogReader);
-
-          .thenReturn(mockLogReader);
 
         final Process dartProcess = MockProcess();
         final StreamController<List<int>> compilerStdoutController = StreamController<List<int>>();
@@ -235,8 +232,7 @@ void main() {
       });
 
       testUsingContext('Fails with tool exit on bad Observatory uri', () async {
-        when(device.getLogReader(includePastLogs: anyNamed('includePastLogs')))
+        when(device.getLogReader()).thenAnswer((_) {
-          .thenAnswer((_) {
           // Now that the reader is used, start writing messages to it.
           mockLogReader.addLine('Foo');
           mockLogReader.addLine('Observatory listening on http:/:/127.0.0.1:$devicePort');
@@ -253,8 +249,7 @@ void main() {
       });
 
       testUsingContext('accepts filesystem parameters', () async {
-        when(device.getLogReader(includePastLogs: anyNamed('includePastLogs')))
+        when(device.getLogReader()).thenAnswer((_) {
-          .thenAnswer((_) {
           // Now that the reader is used, start writing messages to it.
           mockLogReader.addLine('Foo');
           mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort');
@@ -333,8 +328,7 @@ void main() {
       });
 
       testUsingContext('exits when ipv6 is specified and debug-port is not', () async {
-        when(device.getLogReader(includePastLogs: anyNamed('includePastLogs')))
+        when(device.getLogReader()).thenAnswer((_) {
-          .thenAnswer((_) {
           // Now that the reader is used, start writing messages to it.
           mockLogReader.addLine('Foo');
           mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort');
@@ -356,8 +350,7 @@ void main() {
       },);
 
       testUsingContext('exits when observatory-port is specified and debug-port is not', () async {
-        when(device.getLogReader(includePastLogs: anyNamed('includePastLogs')))
+        when(device.getLogReader()).thenAnswer((_) {
-          .thenAnswer((_) {
           // Now that the reader is used, start writing messages to it.
           mockLogReader.addLine('Foo');
           mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort');
@@ -409,7 +402,7 @@ void main() {
       when(mockHotRunner.isWaitingForObservatory).thenReturn(false);
 
       testDeviceManager.addDevice(device);
-      when(device.getLogReader(includePastLogs: anyNamed('includePastLogs')))
+      when(device.getLogReader())
         .thenAnswer((_) {
           // Now that the reader is used, start writing messages to it.
           mockLogReader.addLine('Foo');
@@ -448,8 +441,7 @@ void main() {
       final MockHotRunner mockHotRunner = MockHotRunner();
       final MockHotRunnerFactory mockHotRunnerFactory = MockHotRunnerFactory();
       when(device.portForwarder).thenReturn(portForwarder);
-      when(device.getLogReader(includePastLogs: anyNamed('includePastLogs')))
+      when(device.getLogReader()).thenAnswer((_) => mockLogReader);
-        .thenAnswer((_) => mockLogReader);
       when(portForwarder.forward(devicePort, hostPort: anyNamed('hostPort')))
         .thenAnswer((_) async => hostPort);
       when(portForwarder.forwardedPorts)

packages/flutter_tools/test/commands.shard/hermetic/run_test.dart

@@ -633,10 +633,7 @@ class FakeDevice extends Fake implements Device {
   Future<String> get sdkNameAndVersion => Future<String>.value('');
 
   @override
-  DeviceLogReader getLogReader({
+  DeviceLogReader getLogReader({ ApplicationPackage app }) {
-    ApplicationPackage app,
-    bool includePastLogs = false,
-  }) {
     return FakeDeviceLogReader();
   }
 

packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart

@@ -79,30 +79,6 @@ void main() {
     expect(processManager.hasRemainingExpectations, false);
   });
 
-  testWithoutContext('AdbLogReader calls adb logcat with expected flags when requesting past logs', () async {
-    final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
-      const FakeCommand(
-        command: <String>[
-          'adb',
-          '-s',
-          '1234',
-          'logcat',
-          '-v',
-          'time',
-          '-s',
-          'flutter',
-        ],
-      )
-    ]);
-    await AdbLogReader.createLogReader(
-      createMockDevice(null),
-      processManager,
-      includePastLogs: true,
-    );
-
-    expect(processManager.hasRemainingExpectations, false);
-  });
-
   testWithoutContext('AdbLogReader handles process early exit', () async {
     final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
       FakeCommand(

packages/flutter_tools/test/general.shard/android/android_device_test.dart

@@ -468,48 +468,17 @@ flutter:
     });
   });
 
-  group('logcat', () {
+  group('lastLogcatTimestamp', () {
     final ProcessManager mockProcessManager = MockProcessManager();
     final AndroidDevice device = AndroidDevice('1234');
 
-    testUsingContext('lastLogcatTimestamp returns null if shell command failed', () async {
+    testUsingContext('returns null if shell command failed', () async {
       when(mockProcessManager.runSync(argThat(contains('logcat'))))
           .thenReturn(ProcessResult(0, 1, '', ''));
       expect(device.lastLogcatTimestamp, isNull);
     }, overrides: <Type, Generator>{
       ProcessManager: () => mockProcessManager,
     });
-
-    testUsingContext('AdbLogReaders for past+future and future logs are not the same', () async {
-      when(mockProcessManager.run(
-        argThat(contains('getprop')),
-        stderrEncoding: anyNamed('stderrEncoding'),
-        stdoutEncoding: anyNamed('stdoutEncoding'),
-      )).thenAnswer((_) {
-        final StringBuffer buf = StringBuffer()
-          ..writeln('[ro.build.version.sdk]: [23]');
-        final ProcessResult result = ProcessResult(1, exitCode, buf.toString(), '');
-        return Future<ProcessResult>.value(result);
-      });
-      when(mockProcessManager.run(
-        argThat(contains('shell')),
-        stderrEncoding: anyNamed('stderrEncoding'),
-        stdoutEncoding: anyNamed('stdoutEncoding'),
-      )).thenAnswer((_) {
-        final StringBuffer buf = StringBuffer()
-          ..writeln('11-27 15:39:04.506');
-        final ProcessResult result = ProcessResult(1, exitCode, buf.toString(), '');
-        return Future<ProcessResult>.value(result);
-      });
-      final DeviceLogReader pastLogReader = await device.getLogReader(includePastLogs: true);
-      final DeviceLogReader defaultLogReader = await device.getLogReader();
-      expect(pastLogReader, isNot(equals(defaultLogReader)));
-      // Getting again is cached.
-      expect(pastLogReader, equals(await device.getLogReader(includePastLogs: true)));
-      expect(defaultLogReader, equals(await device.getLogReader()));
-    }, overrides: <Type, Generator>{
-      ProcessManager: () => mockProcessManager,
-    });
   });
 
   test('Can parse adb shell dumpsys info', () {