Skip to content

Conversation

@0nko
Copy link
Member

@0nko 0nko commented Aug 6, 2024

Task/Issue URL: https://app.asana.com/0/1202552961248957/1207981775109546/f
Possibly also: https://app.asana.com/0/1202552961248957/1207981775109549

Description

This PR adds bounds checks for the tab getter in the RecyclerView adapter. The first crash is caused by a rapid double tap on the tab close button, which results in an invalid index being returned.

The second crash is possibly a different manifestation of the same problem but due to a race condition it happens elsewhere. It should be resolved by 8bfcfbd.

Steps to test this PR

Duble tap

  • Go to the tab manager
  • Add a few tabs
  • Tap on a tab close button very quickly
  • Notice the app doesn’t crash

Copy link
Contributor

@karlenDimla karlenDimla left a comment

Choose a reason for hiding this comment

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

Works as expected

@0nko 0nko merged commit cda5825 into develop Aug 6, 2024
@0nko 0nko deleted the fix/ondrej/tab-delete-crash branch August 6, 2024 12:50
@0nko 0nko restored the fix/ondrej/tab-delete-crash branch August 6, 2024 14:07
@0nko 0nko deleted the fix/ondrej/tab-delete-crash branch August 6, 2024 14:13
karlenDimla pushed a commit that referenced this pull request Aug 6, 2024
Task/Issue URL:
https://app.asana.com/0/1202552961248957/1207981775109546/f
Possibly also: https://app.asana.com/0/1202552961248957/1207981775109549

### Description

This PR adds bounds checks for the tab getter in the RecyclerView
adapter. The first crash is caused by a rapid double tap on the tab
close button, which results in an invalid index being returned.

The second crash is possibly a different manifestation of the same
problem but due to a race condition it happens elsewhere. It should be
resolved by
[8bfcfbd](8bfcfbd).

### Steps to test this PR

_Duble tap_
- [x] Go to the tab manager 
- [x] Add a few tabs
- [x] Tap on a tab close button very quickly
- [x] Notice the app doesn’t crash
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.

2 participants