Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] [Crash] Selected session may be rendered in ExternalAppBrowserActivity #5678

Closed
csadilek opened this issue Sep 30, 2019 · 9 comments
Closed
Assignees
Labels
b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info 🐞 bug Crashes, Something isn't working, ..
Milestone

Comments

@csadilek
Copy link
Contributor

csadilek commented Sep 30, 2019

We've fixed different bugs causing this crash in the past, but it is still happening (although at a lower rate than before). Definitely needs investigation and STRs:

https://sentry.prod.mozaws.net/operations/fenix-nightly/issues/6170288/?environment=fenixNightly

https://sentry.prod.mozaws.net/operations/fenix-nightly/issues/6191212/?environment=fenixNightly

It seems to happen for both regular and custom tabs (looking at the breadcrumbs). The root cause is always that we're trying to set the same session on multiple views. See:

#360
#575
#897
#4438
#4486

┆Issue is synchronized with this Jira Task

@csadilek csadilek added 🐞 bug Crashes, Something isn't working, .. b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info labels Sep 30, 2019
@ekager ekager added the needs:STR steps to reproduce label Sep 30, 2019
@csadilek csadilek added this to ⏳ Sprint Backlog in A-C: Android Components Sprint Planning Sep 30, 2019
@csadilek csadilek self-assigned this Sep 30, 2019
@csadilek
Copy link
Contributor Author

Assigning to me for investigation. @kbrosnan, it would be great if we could get help from QA to find STRs, which helped us tracked down other cases of this last time:

#4438
#4486

@csadilek csadilek moved this from ⏳ Sprint Backlog to 🏃‍♀️ In Progress in A-C: Android Components Sprint Planning Oct 7, 2019
@sv-ohorvath
Copy link
Contributor

@csadilek I've tried anything I could think of: sharing from external apps, from custom tabs, using private launcher, using pwa's, sending tabs, but can't get a crash :) If I encounter something I'll make sure to get a log.

@csadilek
Copy link
Contributor Author

Thank you @sv-ohorvath for investigating. Same here. I've tried it all as well. Only thing I found is a different crash: #6029

Since this is a top crash let's keep it open and investigate more. Will post an update once I have news.

@csadilek
Copy link
Contributor Author

Interestingly one of the two variants of this crash appear to be fixed since last weeks GV upgrade: https://sentry.prod.mozaws.net/operations/fenix-nightly/issues/6191212/

For the other we'll add additional information to the exception as part of:
mozilla-mobile/android-components#4829

@csadilek
Copy link
Contributor Author

The additional exception details helped us understand the problem. We're running into cases where the customTabSessionId [1] (which we get from the intent [2]) is null i.e. because the process was killed.

Consequently, we're trying to open the selected session (instead of the custom tab session) in a custom tab and two things can happen:

[1] https://github.com/mozilla-mobile/fenix/blob/master/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivity.kt#L49
[2] https://github.com/mozilla-mobile/fenix/blob/master/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivity.kt#L37

@csadilek csadilek changed the title [Bug] [Crash] Display already acquired [Bug] [Crash] Selected session may be rendered in ExternalAppBrowserActivity Oct 25, 2019
csadilek added a commit to csadilek/fenix that referenced this issue Oct 25, 2019
csadilek added a commit to csadilek/fenix that referenced this issue Oct 25, 2019
csadilek added a commit to csadilek/fenix that referenced this issue Oct 25, 2019
csadilek added a commit to csadilek/fenix that referenced this issue Oct 25, 2019
csadilek added a commit to csadilek/fenix that referenced this issue Oct 25, 2019
csadilek added a commit to csadilek/fenix that referenced this issue Oct 28, 2019
csadilek added a commit to csadilek/fenix that referenced this issue Oct 28, 2019
@ekager ekager closed this as completed in ee3871c Oct 28, 2019
A-C: Android Components Sprint Planning automation moved this from 🏃‍♀️ In Progress to 🏁 Done Oct 28, 2019
mcarare pushed a commit to mcarare/fenix that referenced this issue Oct 29, 2019
@severinrudie severinrudie mentioned this issue Nov 6, 2019
30 tasks
@severinrudie severinrudie added this to the v3.0 milestone Nov 7, 2019
@severinrudie severinrudie added the eng:qa:needed QA Needed label Nov 7, 2019
@sv-ohorvath
Copy link
Contributor

@csadilek Do you have some reliable STR for this crash, for QA?

@csadilek
Copy link
Contributor Author

csadilek commented Nov 8, 2019

@sv-ohorvath Unfortunately, we never found reliable STRs, so we had to simulate the crashes by setting the intent to null. However, after we landed the fixes, the crashes went away (you can see in the two Sentry links above). Both crashes stopped happening.

@sv-ohorvath
Copy link
Contributor

Ok, thank you then :)

@sv-ohorvath sv-ohorvath removed the eng:qa:needed QA Needed label Nov 8, 2019
@abodea
Copy link
Member

abodea commented Mar 23, 2020

Removing the needs:STR as there is no need for STR and both crashes were fixed.

@abodea abodea removed the needs:STR steps to reproduce label Mar 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info 🐞 bug Crashes, Something isn't working, ..
Projects
No open projects
Development

No branches or pull requests

5 participants