Revert "Update `FocusManager` platform check to include iOS" #148978

Renzo-Olivares · 2024-05-23T17:32:50Z

This reverts commit 8d955cd.

Renzo-Olivares · 2024-05-23T17:34:43Z

https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20web_skwasm_tests_7_last/715/overview

00:55 +295 ~9: test/widgets/focus_manager_test.dart: FocusNode FocusManager ignores app lifecycle changes on Android and iOS.
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: true
  Actual: <false>

When the exception was thrown, this was the stack:
at Error._throwWithCurrentStackTrace (http://localhost:38659/widgets/main.dart.wasm:wasm-function[987]:0x40852a)
    at fail (http://localhost:38659/widgets/main.dart.wasm:wasm-function[8576]:0x49cb50)
    at _expect (http://localhost:38659/widgets/main.dart.wasm:wasm-function[8555]:0x49c799)
    at expect (http://localhost:38659/widgets/main.dart.wasm:wasm-function[8554]:0x49c450)
    at expect (http://localhost:38659/widgets/main.dart.wasm:wasm-function[8517]:0x49b937)
    at main closure at org-dartlang-app:///widgets/focus_manager_test.dart:357:83 inner (http://localhost:38659/widgets/main.dart.wasm:wasm-function[58284]:0x9e3d3d)
    at _awaitHelper closure at org-dartlang-sdk:///dart-sdk/lib/_internal/wasm/lib/async_patch.dart:83:16 (http://localhost:38659/widgets/main.dart.wasm:wasm-function[6044]:0x46b8a4)
    at closure wrapper at org-dartlang-sdk:///dart-sdk/lib/_internal/wasm/lib/async_patch.dart:83:16 trampoline (http://localhost:38659/widgets/main.dart.wasm:wasm-function[6050]:0x46b9a2)

The test description was:
  FocusManager ignores app lifecycle changes on Android and iOS.
════════════════════════════════════════════════════════════════════════════════════════════════════
00:55 +295 ~9 -1: test/widgets/focus_manager_test.dart: FocusNode FocusManager ignores app lifecycle changes on Android and iOS. [E]
  Test failed. See exception logs above.
  The test description was: FocusManager ignores app lifecycle changes on Android and iOS.

LongCatIsLooong · 2024-05-23T17:48:37Z

cc @nate-thegrate could you take a look? The same test is failing in canvaskit tests too: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8747130618901677809/+/u/run_test.dart_for_web_canvaskit_tests_shard_and_subshard_2/stdout

Also it's strange that the tests are only failing during post submit.

nate-thegrate · 2024-05-23T18:08:07Z

I believe this problem comes from ~~the removal of kIsWeb~~ it turned out to be a different method that was added amidst the other changes:

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

Lines 1890 to 1902 in 84fe3b6

    
             @visibleForTesting 
        
             void listenToApplicationLifecycleChangesIfSupported() { 
        
               if (_appLifecycleListener == null && (kIsWeb || defaultTargetPlatform != TargetPlatform.android)) { 
        
                 // It appears that some Android keyboard implementations can cause 
        
                 // app lifecycle state changes: adding this listener would cause the 
        
                 // text field to unfocus as the user is trying to type. 
        
                 // 
        
                 // Until this is resolved, we won't be adding the listener to Android apps. 
        
                 // https://github.com/flutter/flutter/pull/142930#issuecomment-1981750069 
        
                 _appLifecycleListener = _AppLifecycleListener(_appLifecycleChange); 
        
                 WidgetsBinding.instance.addObserver(_appLifecycleListener!); 
        
               } 
        
             }

It looks like removing `kIsWeb` from the `FocusManager._appLifecycleListener` platform check is causing [memory leaks](#148985) and test failures. This pull request fixes #148475 and prevents the test failures shown in #148978.

…lutter/flutter#148978)

flutter/flutter@8d955cd...8dd0831 2024-05-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from c6041cd9049c to 4ab442475223 (1 revision) (flutter/flutter#149039) 2024-05-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8d896f4feb76 to c6041cd9049c (1 revision) (flutter/flutter#149035) 2024-05-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from fe3b26294d8f to 8d896f4feb76 (1 revision) (flutter/flutter#149030) 2024-05-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from b4fb08f21757 to fe3b26294d8f (1 revision) (flutter/flutter#149028) 2024-05-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from f873230ce823 to b4fb08f21757 (2 revisions) (flutter/flutter#149025) 2024-05-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from d78f66753332 to f873230ce823 (1 revision) (flutter/flutter#149021) 2024-05-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from 768c90ee5dc7 to d78f66753332 (3 revisions) (flutter/flutter#149011) 2024-05-23 nate.w5687@gmail.com Reland "Update `FocusManager` platform check to include iOS" (flutter/flutter#148984) 2024-05-23 engine-flutter-autoroll@skia.org Roll Flutter Engine from ad12c5d02a8c to 768c90ee5dc7 (2 revisions) (flutter/flutter#149003) 2024-05-23 tessertaha@gmail.com Fix `SnackBar` action text button overlay color (flutter/flutter#148961) 2024-05-23 42757204+wyqlxf@users.noreply.github.com Fix the second TextFormField to trigger onTapOutside (flutter/flutter#148930) 2024-05-23 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2a48302f6f4e to ad12c5d02a8c (3 revisions) (flutter/flutter#148993) 2024-05-23 engine-flutter-autoroll@skia.org Roll Flutter Engine from 964f087f288c to 2a48302f6f4e (1 revision) (flutter/flutter#148950) 2024-05-23 katelovett@google.com [wiki migration] Remaining pages under docs/contributing/ (flutter/flutter#148790) 2024-05-23 katelovett@google.com [wiki migration] Web team pages (flutter/flutter#148777) 2024-05-23 82763757+NobodyForNothing@users.noreply.github.com Test remaining transitions api examples (flutter/flutter#148302) 2024-05-23 82763757+NobodyForNothing@users.noreply.github.com Test snack bar examples (flutter/flutter#147774) 2024-05-23 82763757+NobodyForNothing@users.noreply.github.com Add tests for material banner example (flutter/flutter#147733) 2024-05-23 engine-flutter-autoroll@skia.org Roll Packages from 6525441 to 1008d9e (6 revisions) (flutter/flutter#148980) 2024-05-23 goderbauer@google.com Revert "const vs. non-const widget build benchmark" (flutter/flutter#148970) 2024-05-23 ian@hixie.ch Remove hidden dependencies on HttpClient (flutter/flutter#148773) 2024-05-23 rmolivares@renzo-olivares.dev Revert "Update `FocusManager` platform check to include iOS" (flutter/flutter#148978) 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,tarrinneal@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

…#148978)

…#148984) It looks like removing `kIsWeb` from the `FocusManager._appLifecycleListener` platform check is causing [memory leaks](flutter#148985) and test failures. This pull request fixes flutter#148475 and prevents the test failures shown in flutter#148978.

…lutter/flutter#148978)

Revert "Update FocusManager platform check to include iOS (#148612)"

92a946d

This reverts commit 8d955cd.

Renzo-Olivares requested review from LongCatIsLooong and nate-thegrate May 23, 2024 17:32

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: focus Focus traversal, gaining or losing focus labels May 23, 2024

Renzo-Olivares added the revert Autorevert PR (with "Reason for revert:" comment) label May 23, 2024

LongCatIsLooong approved these changes May 23, 2024

View reviewed changes

Renzo-Olivares merged commit 84fe3b6 into master May 23, 2024

Renzo-Olivares deleted the revert-148612-focus-manager-fix branch May 23, 2024 17:46

nate-thegrate mentioned this pull request May 23, 2024

Reland "Update FocusManager platform check to include iOS" #148984

Merged

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

Roll Flutter from 8d955cdd644e to 8dd08314d2e4 (22 revisions) flutter/packages#6788

Merged

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

84fe3b6 Revert "Update FocusManager platform check to include iOS" (f…

5306dd4

…lutter/flutter#148978)

victorsanni pushed a commit to victorsanni/flutter that referenced this pull request May 31, 2024

Revert "Update FocusManager platform check to include iOS" (flutter…

7382546

…#148978)

nate-thegrate mentioned this pull request Jul 1, 2024

[CP-stable] Update FocusManager platform check to include iOS #151144

Closed

5 tasks

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

84fe3b6 Revert "Update FocusManager platform check to include iOS" (f…

f8f3b08

…lutter/flutter#148978)

nate-thegrate mentioned this pull request Oct 14, 2024

Notable Mishaps #156720

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Revert "Update `FocusManager` platform check to include iOS" #148978

Revert "Update `FocusManager` platform check to include iOS" #148978

Uh oh!

Renzo-Olivares commented May 23, 2024

Uh oh!

Renzo-Olivares commented May 23, 2024

Uh oh!

LongCatIsLooong commented May 23, 2024 •

edited

Loading

Uh oh!

nate-thegrate commented May 23, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Revert "Update FocusManager platform check to include iOS" #148978

Revert "Update FocusManager platform check to include iOS" #148978

Uh oh!

Conversation

Renzo-Olivares commented May 23, 2024

Uh oh!

Renzo-Olivares commented May 23, 2024

Uh oh!

LongCatIsLooong commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nate-thegrate commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Revert "Update `FocusManager` platform check to include iOS" #148978

Revert "Update `FocusManager` platform check to include iOS" #148978

LongCatIsLooong commented May 23, 2024 •

edited

Loading

nate-thegrate commented May 23, 2024 •

edited

Loading