Update _handlePushRouteInformation to Future<bool> to indicate whether any of the observer has handled the route or not #147901

hangyujin · 2024-05-06T23:56:31Z

follow up on comments on flutter/engine#52350

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

chunhtai · 2024-05-07T22:10:28Z

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

@@ -870,7 +871,8 @@ mixin WidgetsBinding on BindingBase, ServicesBinding, SchedulerBinding, GestureB
      // back gesture occurs but no predictive back route transition exists to
      // handle it. The back gesture should still cause normal pop even if it
      // doesn't cause a predictive transition.
-      return handlePopRoute();
+      await handlePopRoute();


is there a lint that suggests this? seems a bit weird...

yes _handleCommitBackGesture is a Future<void> and handlePopRoute is a Future<bool> here

ah I see. looks good then

chunhtai · 2024-05-07T22:12:47Z

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

@@ -896,33 +898,36 @@ mixin WidgetsBinding on BindingBase, ServicesBinding, SchedulerBinding, GestureB
  @protected
  @mustCallSuper
  @visibleForTesting
-  Future<void> handlePushRoute(String route) async {
+  Future<bool> handlePushRoute(String route) async {


Just a heads up. This may potentially be a breaking change.

chunhtai · 2024-05-07T22:20:55Z

packages/flutter/test/widgets/binding_test.dart

@@ -377,6 +384,49 @@ void main() {
    WidgetsBinding.instance.removeObserver(observer);
  });

+    testWidgets('pushRouteInformation not handled by observer returns false', (WidgetTester tester) async {


Do we have test for handled cases?

yes the existing tests on Line 275 and line 297 returns true for the result. they are handled cases.

oh i missed the diff above. Thanks

chunhtai · 2024-05-07T22:39:52Z

packages/flutter/test/widgets/binding_test.dart

@@ -377,6 +384,49 @@ void main() {
    WidgetsBinding.instance.removeObserver(observer);
  });

+    testWidgets('pushRouteInformation not handled by observer returns false', (WidgetTester tester) async {


oh i missed the diff above. Thanks

chunhtai · 2024-05-07T22:40:25Z

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

@@ -870,7 +871,8 @@ mixin WidgetsBinding on BindingBase, ServicesBinding, SchedulerBinding, GestureB
      // back gesture occurs but no predictive back route transition exists to
      // handle it. The back gesture should still cause normal pop even if it
      // doesn't cause a predictive transition.
-      return handlePopRoute();
+      await handlePopRoute();
+      return ;


additional space between return and semicolon

* master: (2580 commits) plugin_ffi template comment fix (flutter#148378) Roll Flutter Engine from 942d7c35de75 to 9e17588b330c (2 revisions) (flutter#148455) Reland fix TextField helper top padding on M3 (flutter#146754) Removing duplicate assert on `VisualDensity` constructor (flutter#148281) Roll Flutter Engine from 65ac4bf96ed7 to 942d7c35de75 (1 revision) (flutter#148450) Roll Flutter Engine from c11d64be5102 to 65ac4bf96ed7 (3 revisions) (flutter#148448) Roll Flutter Engine from f6195e9d4b4b to c11d64be5102 (1 revision) (flutter#148445) Roll Flutter Engine from cd150986ae63 to f6195e9d4b4b (2 revisions) (flutter#148441) Fix leaky tests. (flutter#148434) Roll Flutter Engine from 41b86b59f0ab to cd150986ae63 (2 revisions) (flutter#148430) Roll Flutter Engine from bf1c6da0dd31 to 41b86b59f0ab (15 revisions) (flutter#148428) Update _handlePushRouteInformation to Future<bool> to indicate whether any of the observer has handled the route or not (flutter#147901) Fix memory leaks in `_PopupMenuRoute` (flutter#148373) Add `clipBehavior` to `DrawerThemeData` (flutter#148061) Reland Native ios context menu (flutter#143002) (flutter#148238) (flutter#148265) Roll Packages from fd714bd7d516 to 87a02e393be0 (8 revisions) (flutter#148419) Stop running module_test_ios in devicelab and x64 Macs (flutter#148264) Roll Flutter Engine from d35a1a603c80 to bf1c6da0dd31 (1 revision) (flutter#148369) Roll Flutter Engine from 55c62ff82c7e to d35a1a603c80 (4 revisions) (flutter#148367) Roll Flutter Engine from a1d930a3a84d to 55c62ff82c7e (3 revisions) (flutter#148365) ...

…ate whether any of the observer has handled the route or not (flutter/flutter#147901)

flutter/flutter@39651e8...0d22d91 2024-05-16 102401667+Dimilkalathiya@users.noreply.github.com fixes `DialogRoute` memory leak (flutter/flutter#147816) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9bc449ee2e8b to 460df6caef0e (1 revision) (flutter/flutter#148476) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1ebad3a6179d to 9bc449ee2e8b (1 revision) (flutter/flutter#148469) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9e17588b330c to 1ebad3a6179d (1 revision) (flutter/flutter#148465) 2024-05-16 6655696+guidezpl@users.noreply.github.com ThemeData minor spring cleaning (flutter/flutter#148408) 2024-05-16 fluttergithubbot@gmail.com Marks Mac_pixel_7pro native_assets_android to be flaky (flutter/flutter#148403) 2024-05-16 dacoharkes@google.com plugin_ffi template comment fix (flutter/flutter#148378) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 942d7c35de75 to 9e17588b330c (2 revisions) (flutter/flutter#148455) 2024-05-16 bruno.leroux@gmail.com Reland fix TextField helper top padding on M3 (flutter/flutter#146754) 2024-05-16 52160996+FMorschel@users.noreply.github.com Removing duplicate assert on `VisualDensity` constructor (flutter/flutter#148281) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 65ac4bf96ed7 to 942d7c35de75 (1 revision) (flutter/flutter#148450) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from c11d64be5102 to 65ac4bf96ed7 (3 revisions) (flutter/flutter#148448) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from f6195e9d4b4b to c11d64be5102 (1 revision) (flutter/flutter#148445) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from cd150986ae63 to f6195e9d4b4b (2 revisions) (flutter/flutter#148441) 2024-05-15 polinach@google.com Fix leaky tests. (flutter/flutter#148434) 2024-05-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 41b86b59f0ab to cd150986ae63 (2 revisions) (flutter/flutter#148430) 2024-05-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from bf1c6da0dd31 to 41b86b59f0ab (15 revisions) (flutter/flutter#148428) 2024-05-15 jhy03261997@gmail.com Update _handlePushRouteInformation to Future<bool> to indicate whether any of the observer has handled the route or not (flutter/flutter#147901) 2024-05-15 32538273+ValentinVignal@users.noreply.github.com Fix memory leaks in `_PopupMenuRoute` (flutter/flutter#148373) 2024-05-15 32538273+ValentinVignal@users.noreply.github.com Add `clipBehavior` to `DrawerThemeData` (flutter/flutter#148061) 2024-05-15 jmccandless@google.com Reland Native ios context menu (#143002) (#148238) (flutter/flutter#148265) 2024-05-15 engine-flutter-autoroll@skia.org Roll Packages from fd714bd to 87a02e3 (8 revisions) (flutter/flutter#148419) 2024-05-15 magder@google.com Stop running module_test_ios in devicelab and x64 Macs (flutter/flutter#148264) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r#6748) flutter/flutter@39651e8...0d22d91 2024-05-16 102401667+Dimilkalathiya@users.noreply.github.com fixes `DialogRoute` memory leak (flutter/flutter#147816) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9bc449ee2e8b to 460df6caef0e (1 revision) (flutter/flutter#148476) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1ebad3a6179d to 9bc449ee2e8b (1 revision) (flutter/flutter#148469) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9e17588b330c to 1ebad3a6179d (1 revision) (flutter/flutter#148465) 2024-05-16 6655696+guidezpl@users.noreply.github.com ThemeData minor spring cleaning (flutter/flutter#148408) 2024-05-16 fluttergithubbot@gmail.com Marks Mac_pixel_7pro native_assets_android to be flaky (flutter/flutter#148403) 2024-05-16 dacoharkes@google.com plugin_ffi template comment fix (flutter/flutter#148378) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 942d7c35de75 to 9e17588b330c (2 revisions) (flutter/flutter#148455) 2024-05-16 bruno.leroux@gmail.com Reland fix TextField helper top padding on M3 (flutter/flutter#146754) 2024-05-16 52160996+FMorschel@users.noreply.github.com Removing duplicate assert on `VisualDensity` constructor (flutter/flutter#148281) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 65ac4bf96ed7 to 942d7c35de75 (1 revision) (flutter/flutter#148450) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from c11d64be5102 to 65ac4bf96ed7 (3 revisions) (flutter/flutter#148448) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from f6195e9d4b4b to c11d64be5102 (1 revision) (flutter/flutter#148445) 2024-05-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from cd150986ae63 to f6195e9d4b4b (2 revisions) (flutter/flutter#148441) 2024-05-15 polinach@google.com Fix leaky tests. (flutter/flutter#148434) 2024-05-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 41b86b59f0ab to cd150986ae63 (2 revisions) (flutter/flutter#148430) 2024-05-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from bf1c6da0dd31 to 41b86b59f0ab (15 revisions) (flutter/flutter#148428) 2024-05-15 jhy03261997@gmail.com Update _handlePushRouteInformation to Future<bool> to indicate whether any of the observer has handled the route or not (flutter/flutter#147901) 2024-05-15 32538273+ValentinVignal@users.noreply.github.com Fix memory leaks in `_PopupMenuRoute` (flutter/flutter#148373) 2024-05-15 32538273+ValentinVignal@users.noreply.github.com Add `clipBehavior` to `DrawerThemeData` (flutter/flutter#148061) 2024-05-15 jmccandless@google.com Reland Native ios context menu (#143002) (#148238) (flutter/flutter#148265) 2024-05-15 engine-flutter-autoroll@skia.org Roll Packages from fd714bd to 87a02e3 (8 revisions) (flutter/flutter#148419) 2024-05-15 magder@google.com Stop running module_test_ios in devicelab and x64 Macs (flutter/flutter#148264) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This comment was marked as outdated.

Sign in to view

github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label May 6, 2024

hangyujin requested a review from chunhtai May 7, 2024 00:37

chunhtai reviewed May 7, 2024

View reviewed changes

chunhtai approved these changes May 7, 2024

View reviewed changes

This was referenced May 8, 2024

[deep link][ios] Update openURL method to reflect the result from framework flutter/engine#52643

Open

Set deep linking flag to true by default flutter/engine#52350

Open

hangyujin added 5 commits May 14, 2024 16:48

Update binding.dart

68217d4

add tests

139d681

Update binding_test.dart

bd69740

Update binding_test.dart

a0df4f4

lint

c6275f7

hangyujin force-pushed the 2405pushRouteMessage branch from 143f304 to c6275f7 Compare May 15, 2024 00:47

hangyujin added the autosubmit Merge PR when tree becomes green via auto submit App label May 15, 2024

auto-submit bot merged commit 6826fc0 into flutter:master May 15, 2024
73 checks passed

engine-flutter-autoroll mentioned this pull request May 16, 2024

Roll Flutter from 39651e84ea12 to 82fb5db89306 (22 revisions) flutter/packages#6746

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 16, 2024

6826fc08f Update _handlePushRouteInformation to Future<bool> to indic…

6b83364

…ate whether any of the observer has handled the route or not (flutter/flutter#147901)

engine-flutter-autoroll mentioned this pull request May 16, 2024

Roll Flutter from 39651e84ea12 to 0d22d9101aac (23 revisions) flutter/packages#6748

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 16, 2024

6826fc08f Update _handlePushRouteInformation to Future<bool> to indic…

0105b9b

…ate whether any of the observer has handled the route or not (flutter/flutter#147901)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update _handlePushRouteInformation to Future<bool> to indicate whether any of the observer has handled the route or not #147901

Update _handlePushRouteInformation to Future<bool> to indicate whether any of the observer has handled the route or not #147901

hangyujin commented May 6, 2024 •

edited

This comment was marked as outdated.

chunhtai May 7, 2024

hangyujin May 7, 2024 •

edited

chunhtai May 7, 2024

chunhtai May 7, 2024

chunhtai May 7, 2024

hangyujin May 7, 2024

chunhtai May 7, 2024

chunhtai May 7, 2024

chunhtai May 7, 2024 •

edited

Update _handlePushRouteInformation to Future<bool> to indicate whether any of the observer has handled the route or not #147901

Update _handlePushRouteInformation to Future<bool> to indicate whether any of the observer has handled the route or not #147901

Conversation

hangyujin commented May 6, 2024 • edited

Pre-launch Checklist

This comment was marked as outdated.

chunhtai May 7, 2024

Choose a reason for hiding this comment

hangyujin May 7, 2024 • edited

Choose a reason for hiding this comment

chunhtai May 7, 2024

Choose a reason for hiding this comment

chunhtai May 7, 2024

Choose a reason for hiding this comment

chunhtai May 7, 2024

Choose a reason for hiding this comment

hangyujin May 7, 2024

Choose a reason for hiding this comment

chunhtai May 7, 2024

Choose a reason for hiding this comment

chunhtai May 7, 2024

Choose a reason for hiding this comment

chunhtai May 7, 2024 • edited

Choose a reason for hiding this comment

hangyujin commented May 6, 2024 •

edited

hangyujin May 7, 2024 •

edited

chunhtai May 7, 2024 •

edited