From c7cac19ac29ddc884e6dde5e79c8c1973228c585 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 8 Apr 2020 10:34:28 -0700 Subject: [PATCH 1/7] Fix multi-reply on message channel. Add test --- .cirrus.yml | 2 +- .../lib/platform_messages_main.dart | 62 +++++++++++++++++++ .../test_driver/platform_messages_e2e.dart | 58 +++++++++++++++++ .../platform_messages_e2e_test.dart | 7 +++ lib/web_ui/lib/src/engine/clipboard.dart | 10 ++- .../src/engine/compositor/embedded_views.dart | 2 - lib/web_ui/lib/src/engine/platform_views.dart | 2 - 7 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 e2etests/web/regular_integration_tests/lib/platform_messages_main.dart create mode 100644 e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart create mode 100644 e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e_test.dart diff --git a/.cirrus.yml b/.cirrus.yml index b2942ded3b186..e04df18633dfc 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -168,7 +168,7 @@ task: - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/text_editing_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/treeshaking_e2e.dart -d web-server --profile --browser-name=chrome --local-engine=host_debug_unopt - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/image_loading_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt - + - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --no-headless --target=test_driver/platform_messages_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt - name: build_and_test_web_linux_firefox compile_host_script: | cd $ENGINE_PATH/src diff --git a/e2etests/web/regular_integration_tests/lib/platform_messages_main.dart b/e2etests/web/regular_integration_tests/lib/platform_messages_main.dart new file mode 100644 index 0000000000000..3598d31da686c --- /dev/null +++ b/e2etests/web/regular_integration_tests/lib/platform_messages_main.dart @@ -0,0 +1,62 @@ +// Copyright 2013 The Flutter 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/material.dart'; +import 'package:flutter/services.dart'; + +void main() => runApp(MyApp()); + +Future dataFuture; + +class MyApp extends StatelessWidget { + @override + Widget build(BuildContext context) { + return MaterialApp( + key: const Key('mainapp'), + title: 'Integration Test App For Platform Messages', + home: MyHomePage(title: 'Integration Test App For Platform Messages'), + ); + } +} + +class MyHomePage extends StatefulWidget { + MyHomePage({Key key, this.title}) : super(key: key); + + final String title; + + @override + _MyHomePageState createState() => _MyHomePageState(); +} + +class _MyHomePageState extends State { + final TextEditingController _controller = + TextEditingController(text: 'Text1'); + + @override + Widget build(BuildContext context) { + return Scaffold( + appBar: AppBar( + title: Text(widget.title), + ), + body: Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + const Text('Hello World', + ), + TextFormField( + key: const Key('input'), + enabled: true, + controller: _controller, + //initialValue: 'Text1', + decoration: const InputDecoration( + labelText: 'Text Input Field:', + ), + ), + ], + ), + ), + ); + } +} diff --git a/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart new file mode 100644 index 0000000000000..189bdca344826 --- /dev/null +++ b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart @@ -0,0 +1,58 @@ +// Copyright 2013 The Flutter 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 'dart:html' as html; +// ignore: undefined_shown_name +import 'dart:ui' as ui show platformViewRegistry; +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:regular_integration_tests/platform_messages_main.dart' as app; + +import 'package:e2e/e2e.dart'; + +void main() async { + E2EWidgetsFlutterBinding.ensureInitialized() as E2EWidgetsFlutterBinding; + + testWidgets('platform message for Clipboard.set/getData reply with future', + (WidgetTester tester) async { + app.main(); + await tester.pumpAndSettle(); + + // TODO(nurhan): https://github.com/flutter/flutter/issues/51885 + SystemChannels.textInput.setMockMethodCallHandler(null); + // Focus on a TextFormField. + final Finder finder = find.byKey(const Key('input')); + expect(finder, findsOneWidget); + await tester.tap(find.byKey(const Key('input'))); + // Focus in input, otherwise clipboard will fail with + // 'document is not focused' platform exception. + html.document.querySelector('input').focus(); + await Clipboard.setData(const ClipboardData(text: 'sample text')); + }); + + testWidgets('Should create and dispose view embedder', + (WidgetTester tester) async { + int viewInstanceCount = 0; + + final int currentViewId = platformViewsRegistry.getNextPlatformViewId(); + // ignore: undefined_prefixed_name + ui.platformViewRegistry.registerViewFactory('MyView', (int viewId) { + ++viewInstanceCount; + return html.DivElement(); + }); + + app.main(); + await tester.pumpAndSettle(); + final Map createArgs = { + 'id': '567', + 'viewType': 'MyView', + }; + await SystemChannels.platform_views.invokeMethod('create', createArgs); + final Map disposeArgs = { + 'id': '567', + }; + await SystemChannels.platform_views.invokeMethod('dispose', disposeArgs); + }); +} diff --git a/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e_test.dart b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e_test.dart new file mode 100644 index 0000000000000..a29203f7dcdd9 --- /dev/null +++ b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e_test.dart @@ -0,0 +1,7 @@ +// Copyright 2013 The Flutter 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:e2e/e2e_driver.dart' as e2e; + +Future main() async => e2e.main(); diff --git a/lib/web_ui/lib/src/engine/clipboard.dart b/lib/web_ui/lib/src/engine/clipboard.dart index de119a2842e73..cc5ed5532c96c 100644 --- a/lib/web_ui/lib/src/engine/clipboard.dart +++ b/lib/web_ui/lib/src/engine/clipboard.dart @@ -18,6 +18,7 @@ class ClipboardMessageHandler { void setDataMethodCall( MethodCall methodCall, ui.PlatformMessageResponseCallback callback) { const MethodCodec codec = JSONMethodCodec(); + bool errorEnvelopeEncoded = false; _copyToClipboardStrategy .setData(methodCall.arguments['text']) .then((bool success) { @@ -26,10 +27,15 @@ class ClipboardMessageHandler { } else { callback(codec.encodeErrorEnvelope( code: 'copy_fail', message: 'Clipboard.setData failed')); + errorEnvelopeEncoded = true; } }).catchError((dynamic _) { - callback(codec.encodeErrorEnvelope( - code: 'copy_fail', message: 'Clipboard.setData failed')); + // Don't encode a duplicate reply if we already failed and an error + // was already encoded. + if (!errorEnvelopeEncoded) { + callback(codec.encodeErrorEnvelope( + code: 'copy_fail', message: 'Clipboard.setData failed')); + } }); } diff --git a/lib/web_ui/lib/src/engine/compositor/embedded_views.dart b/lib/web_ui/lib/src/engine/compositor/embedded_views.dart index 4cb6e64b7a74a..60d218d0d0e47 100644 --- a/lib/web_ui/lib/src/engine/compositor/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/compositor/embedded_views.dart @@ -69,11 +69,9 @@ class HtmlViewEmbedder { switch (decoded.method) { case 'create': _create(decoded, callback); - window._replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); return; case 'dispose': _dispose(decoded, callback); - window._replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); return; } callback(null); diff --git a/lib/web_ui/lib/src/engine/platform_views.dart b/lib/web_ui/lib/src/engine/platform_views.dart index 2454cd44edb1d..d422f9a3ecf84 100644 --- a/lib/web_ui/lib/src/engine/platform_views.dart +++ b/lib/web_ui/lib/src/engine/platform_views.dart @@ -50,11 +50,9 @@ void handlePlatformViewCall( switch (decoded.method) { case 'create': _createPlatformView(decoded, callback); - window._replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); return; case 'dispose': _disposePlatformView(decoded, callback); - window._replyToPlatformMessage(callback, codec.encodeSuccessEnvelope(true)); return; } callback(null); From 3268207c148ad0b64d8df706e5cd3a5e18a1648f Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 8 Apr 2020 10:36:30 -0700 Subject: [PATCH 2/7] fix formatting in cirrus.yml --- .cirrus.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.cirrus.yml b/.cirrus.yml index e04df18633dfc..f3fed9aa18ea5 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -169,6 +169,7 @@ task: - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/treeshaking_e2e.dart -d web-server --profile --browser-name=chrome --local-engine=host_debug_unopt - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/image_loading_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --no-headless --target=test_driver/platform_messages_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt + - name: build_and_test_web_linux_firefox compile_host_script: | cd $ENGINE_PATH/src From edf2bf87afe7a2ac85fb3206bdab77c51996f0d7 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 8 Apr 2020 10:56:26 -0700 Subject: [PATCH 3/7] Add comment on HomePageState text field --- .../regular_integration_tests/lib/platform_messages_main.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/e2etests/web/regular_integration_tests/lib/platform_messages_main.dart b/e2etests/web/regular_integration_tests/lib/platform_messages_main.dart index 3598d31da686c..4201da2b47b77 100644 --- a/e2etests/web/regular_integration_tests/lib/platform_messages_main.dart +++ b/e2etests/web/regular_integration_tests/lib/platform_messages_main.dart @@ -45,6 +45,8 @@ class _MyHomePageState extends State { children: [ const Text('Hello World', ), + // Create a text form field since we can't test clipboard unless + // html document has focus. TextFormField( key: const Key('input'), enabled: true, From 8822398784145b4b3b39ea22e8f10d585587ea1b Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 8 Apr 2020 10:57:28 -0700 Subject: [PATCH 4/7] Fix test title --- .../test_driver/platform_messages_e2e.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart index 189bdca344826..c89a8005434ee 100644 --- a/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart +++ b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart @@ -15,7 +15,7 @@ import 'package:e2e/e2e.dart'; void main() async { E2EWidgetsFlutterBinding.ensureInitialized() as E2EWidgetsFlutterBinding; - testWidgets('platform message for Clipboard.set/getData reply with future', + testWidgets('platform message for Clipboard.setData reply with future', (WidgetTester tester) async { app.main(); await tester.pumpAndSettle(); From 31e4a90c49b257a4fa6d2769d1b4e3660f748f1e Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 8 Apr 2020 11:32:42 -0700 Subject: [PATCH 5/7] Add check for viewInstanceCount --- .../test_driver/platform_messages_e2e.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart index c89a8005434ee..0098dc64d4954 100644 --- a/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart +++ b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart @@ -54,5 +54,6 @@ void main() async { 'id': '567', }; await SystemChannels.platform_views.invokeMethod('dispose', disposeArgs); + expect(viewInstanceCount, 1); }); } From 5ed9251935052052bab70631e12d4d0a805118ed Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 8 Apr 2020 12:03:52 -0700 Subject: [PATCH 6/7] skip no-headless test --- .../test_driver/platform_messages_e2e.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart index 0098dc64d4954..cd6a816b67f67 100644 --- a/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart +++ b/e2etests/web/regular_integration_tests/test_driver/platform_messages_e2e.dart @@ -30,7 +30,7 @@ void main() async { // 'document is not focused' platform exception. html.document.querySelector('input').focus(); await Clipboard.setData(const ClipboardData(text: 'sample text')); - }); + }, skip: true); // https://github.com/flutter/flutter/issues/54296 testWidgets('Should create and dispose view embedder', (WidgetTester tester) async { From 8b0a070982141ade59dba7a79ff01b4f11c5a09a Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 8 Apr 2020 15:16:35 -0700 Subject: [PATCH 7/7] testCI fail --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index f3fed9aa18ea5..65370bc8b58c1 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -166,9 +166,9 @@ task: - $FRAMEWORK_PATH/flutter/bin/flutter config --local-engine=host_debug_unopt --no-analytics --enable-web - $FRAMEWORK_PATH/flutter/bin/flutter pub get --local-engine=host_debug_unopt - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/text_editing_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt + - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/platform_messages_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/treeshaking_e2e.dart -d web-server --profile --browser-name=chrome --local-engine=host_debug_unopt - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --target=test_driver/image_loading_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt - - $FRAMEWORK_PATH/flutter/bin/flutter drive -v --no-headless --target=test_driver/platform_messages_e2e.dart -d web-server --release --browser-name=chrome --local-engine=host_debug_unopt - name: build_and_test_web_linux_firefox compile_host_script: |