Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #7857: Fix migration of screenshot data when there's thousands of tabs #7865

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

Brandon-T
Copy link
Collaborator

@Brandon-T Brandon-T commented Aug 10, 2023

Summary of Changes

  • Remove migration of screenshot data when migrating thousands of tabs

This pull request fixes #7857

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

Test on dev channel only, use exact builds mentioned in the test plan, it's a very specific crash.

  1. Install 1.50 (23.8.8.15)
  2. 3dot menu scroll down to 'Add 1000 tabs button'. Test with 1000 or 2000 tabs
  3. Upgrade to 1.51 (23.8.10.16)
  4. Verify that this build loads without issues.

As an additional test you can test upgrade scenario from 1.56 to 1.57, also with 1000-2000 tabs. Keep in mind we consider capping the number of created tabs to 500 in the future

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

@Brandon-T Brandon-T added this to the 1.57 milestone Aug 10, 2023
@Brandon-T Brandon-T requested a review from a team as a code owner August 10, 2023 14:36
@Brandon-T Brandon-T self-assigned this Aug 10, 2023
Copy link
Contributor

@iccub iccub left a comment

Choose a reason for hiding this comment

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

still crashes for me :(
I submitted a crash log,. more details soon

@iccub
Copy link
Contributor

iccub commented Aug 10, 2023

It seemed to crash because the branch i work on missed Kyle's other fix for big tabs number. Cherry picked it and running the test agian

@iccub
Copy link
Contributor

iccub commented Aug 10, 2023

It works with Kyle's fix applied

@iccub iccub removed the QA/Yes label Aug 10, 2023
@iccub iccub merged commit 504e896 into development Aug 10, 2023
7 checks passed
@iccub iccub deleted the bugfix/TabMigrationScreenshots branch August 10, 2023 17:08
arthuredelstein pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
…'s thousands of tabs (brave/brave-ios#7865)

Remove migration of screenshot data when migrating thousands of tabs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1.51 migration crash when lot of tabs are open
3 participants