Skip to content

Conversation

@felangel
Copy link

No description provided.

@felangel felangel self-assigned this Oct 31, 2023
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly ubuntu is fine to start, but if the other two are easy/useful OK to leave them.

name: 🧪 Test

env:
FLUTTER_STORAGE_BASE_URL: https://download.shorebird.dev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh. Wild to see us explicitly set this. I wonder if we would rather wrap/fork the flutter script instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seemed like the least invasive way since it will result in fewer merge conflicts over time but happy to change the script itself

Copy link

@bryanoltman bryanoltman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably document the changes we had to make to get these working somewhere outside of this PR. LGTM!

@felangel felangel merged commit fbc4718 into shorebird/dev Oct 31, 2023
@felangel felangel deleted the ci/run-tests branch October 31, 2023 20:10
felangel pushed a commit that referenced this pull request Dec 11, 2023
This is a follow up to the following pull requests:
- flutter#124514

I was finally able to reproduce this bug and found out why it was happening. Consider this code:

```dart
GestureDetector(
  behavior: HitTestBehavior.translucent,
  // Note: Make sure onTap is not null to ensure events
  // are captured by `GestureDetector`
  onTap: () {},
  child: _shouldShowSlider
    ? Slider(value: _value, onChanged: _handleSlide)
    : const SizedBox.shrink().
)
```

Runtime exception happens when:

1. User taps and holds the Slider (drag callback captured by `GestureDetector`)
2. `_shouldShowSlider` changes to false, Slider disappears and unmounts, and unregisters `_handleSlide`. But the callback is still registered by `GestureDetector`
3. Users moves finger as if Slider were still there
4. Drag callback is invoked, `_SliderState.showValueIndicator` is called
5. Exception - Slider is already disposed

This pull request fixes it by adding a mounted check inside `_SliderState.showValueIndicator` to ensure the Slider is actually mounted at the time of invoking drag event callback. I've added a unit test that will fail without this change.

The error stack trace is:

```
The following assertion was thrown while handling a gesture:
This widget has been unmounted, so the State no longer has a context (and should be considered
defunct).
Consider canceling any active work during "dispose" or using the "mounted" getter to determine if
the State is still active.

When the exception was thrown, this was the stack:
#0      State.context.<anonymous closure> (package:flutter/src/widgets/framework.dart:950:9)
#1      State.context (package:flutter/src/widgets/framework.dart:956:6)
#2      _SliderState.showValueIndicator (package:flutter/src/material/slider.dart:968:18)
#3      _RenderSlider._startInteraction (package:flutter/src/material/slider.dart:1487:12)
#4      _RenderSlider._handleDragStart (package:flutter/src/material/slider.dart:1541:5)
#5      DragGestureRecognizer._checkStart.<anonymous closure> (package:flutter/src/gestures/monodrag.dart:531:53)
#6      GestureRecognizer.invokeCallback (package:flutter/src/gestures/recognizer.dart:275:24)
#7      DragGestureRecognizer._checkStart (package:flutter/src/gestures/monodrag.dart:531:7)
#8      DragGestureRecognizer._checkDrag (package:flutter/src/gestures/monodrag.dart:498:5)
#9      DragGestureRecognizer.acceptGesture (package:flutter/src/gestures/monodrag.dart:431:7)
#10     _CombiningGestureArenaMember.acceptGesture (package:flutter/src/gestures/team.dart:45:14)
#11     GestureArenaManager._resolveInFavorOf (package:flutter/src/gestures/arena.dart:281:12)
#12     GestureArenaManager._resolve (package:flutter/src/gestures/arena.dart:239:9)
#13     GestureArenaEntry.resolve (package:flutter/src/gestures/arena.dart:53:12)
#14     _CombiningGestureArenaMember._resolve (package:flutter/src/gestures/team.dart:85:15)
#15     _CombiningGestureArenaEntry.resolve (package:flutter/src/gestures/team.dart:19:15)
#16     OneSequenceGestureRecognizer.resolve (package:flutter/src/gestures/recognizer.dart:375:13)
#17     DragGestureRecognizer.handleEvent (package:flutter/src/gestures/monodrag.dart:414:13)
#18     PointerRouter._dispatch (package:flutter/src/gestures/pointer_router.dart:98:12)
#19     PointerRouter._dispatchEventToRoutes.<anonymous closure> (package:flutter/src/gestures/pointer_router.dart:143:9)
#20     _LinkedHashMapMixin.forEach (dart:collection-patch/compact_hash.dart:625:13)
#21     PointerRouter._dispatchEventToRoutes (package:flutter/src/gestures/pointer_router.dart:141:18)
#22     PointerRouter.route (package:flutter/src/gestures/pointer_router.dart:127:7)
#23     GestureBinding.handleEvent (package:flutter/src/gestures/binding.dart:488:19)
#24     GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:468:22)
#25     RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:439:11)
#26     GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:413:7)
#27     GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:376:5)
#28     GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:323:7)
#29     GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:292:9)
#30     _invoke1 (dart:ui/hooks.dart:186:13)
#31     PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:433:7)
#32     _dispatchPointerDataPacket (dart:ui/hooks.dart:119:31)

Handler: "onStart"
Recognizer:
  HorizontalDragGestureRecognizer#a5df2
```

*List which issues are fixed by this PR. You must list at least one issue.*

Internal bug: b/273666179, b/192329942

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
felangel added a commit that referenced this pull request Jan 16, 2024
bryanoltman pushed a commit that referenced this pull request Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants