Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash in added views #292

Merged
merged 6 commits into from
Apr 5, 2020

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Apr 4, 2020

Closes #291

Builds on top of PR #290

2 things were happening:

  1. if an exception occurred in saving an Image-based Story frame and this made removing their AddedViews from the ghost (offscreen) photoEditorView impossible (as it would derail execution to the Exception handler), then those AddedViews would be left depending on that ghost view, and once we want to add them back it would be impossible, producing the crash. For this, we added a finally block to the exception handler to make sure to release those AddedViews, in 601463b.

  2. when the service finishes working, the EventBus listener in the ComposeLoopFrameActivity would try and re-select the current frame, re-adding the AddedViews to the real (visible) photoEditorView and as such, leaving them impossible to be re-added to a new instance of photoEdiitorView later (as it would be the case when you need to manage errors, opening a new instance of the Activity from an error notification or snackbar). This frame selection refresh behavior we have in that listener was an old behavior we wanted to have before we decided to force exiting the Activity once the Service has been triggered (as per our use case) so, removed that code altogether in 6d5df8c

To test:

  1. add an artificial crash, for example add a throw Exception("this is a test") right before this line, this way the error handling mechanism takes course.
  2. verify the steps described in Crash when selecting specific errored frame #291 don't lead to a crash anymore.

mzorz added 2 commits April 4, 2020 13:56
… make sure these are not added to a parent no matter what happened in the process of creating a ghost view snapshot
@mzorz mzorz requested review from marecar3 and jd-alexander April 4, 2020 17:05
@peril-automattic
Copy link

peril-automattic bot commented Apr 4, 2020

You can test the changes on this Pull Request by downloading the APK here.

@mzorz
Copy link
Contributor Author

mzorz commented Apr 5, 2020

Going to merge this one as I need it to make #290 reviewable - we can review the code introduced in this PR on the other PR 👍

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.

1 participant