mozilla-mobile / focus-android

⚠️ Firefox Focus (Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
2.11k stars 711 forks source link

When a tab crashes, all others are crashed too #4006

Closed sv-ohorvath closed 3 years ago

sv-ohorvath commented 5 years ago

Steps to reproduce

  1. Open a few webpages in different tabs.
  2. Go to about:crashcontent in one of the tabs.
  3. Close crashed tab.
  4. Open the other tabs.

    Expected behavior

    Since the crash reporter refers to a single tab, the others should still be functional(?).

    Actual behavior

    All other tabs are showing blank pages.

    Device information

rpappalax commented 5 years ago

I'm unable to repro, however, when I go to about:crashcontent, I get redirected to about:blank (white page)?? All other tabs working as expected.

Android device: Huawei Nexus 6P (Android 8.0.0) Klar version: 8.0.3

Sdaswani commented 5 years ago

Clearing @boek from assignee since he is on PTO.

sblatz commented 5 years ago

I'm getting the same results as @rpappalax. I believe about:blank showing is correct behavior (see this change).

What's confusing is that the crash reporter is not displaying at all. @sv-ohorvath are you able to get the crash reporting fragment to appear in any circumstance? It should have been implemented in #3074.

I believe we may need to create a new ticket for crash-reporter not appearing and potentially close this one if @sv-ohorvath is unable to reproduce now.

sv-ohorvath commented 5 years ago

On Klar GV the crash reporter doesn't show up indeed. I'll file a new bug for that. On Focus, GV, the issue is still reproducible, but now the other tabs are closed and all is left is an about:blank page. (It used to keep the tabs but show blank on all of them.)

I made 2 recordings with the latest release builds from taskcluster:

sblatz commented 5 years ago

Okay so I'm noticing a few different issues here.

  1. In android emulator (Pixel 2 XL API 28, Focus 8.0.4 GV) I'm unable to get the crash report page. The current tab becomes about:blank, and the rest stay open.

  2. On physical device (Moto G Play 6.0.1, Focus 8.0.4 GV) when I crash one tab it crashes them all. I get crash reporter, but once I press "close tab" I'm stuck on about:blank. Pressing the erase button does nothing so I must force close the app. demo

  3. On physical device (Klar 8.0.4 GV) all the tabs become about:blank and crash reporter is not shown.

Seems like this ticket may be rather convoluted and entangled with multiple issues?

sblatz commented 5 years ago

@vesta0 how do we want to handle this? It seems like three different issues are occurring related to this one ticket. I would argue that the second one I mentioned is the most concerning as it leaves the user in a state where they must force close the app.

I think it would also be helpful if people with more physical devices could follow the STR to see if they're getting any different cases other than the ones I mentioned above.

vesta0 commented 5 years ago

@npark-mozilla could we have QA try to replicate the above scenarios?

nojunpark commented 5 years ago

@sblatz so I tested this on release 7.0 WV, (I'm not part of the other 50%) and I see 1. When I change the renderer to GV, then I see 2.

sblatz commented 5 years ago

Spoke with @npark-mozilla and he will be testing a locally built master on more devices as well once master is building again.

My hunch is that we should break out one more ticket: crash reporter "close tab" leaves app in unusable state.

nojunpark commented 5 years ago

I also tested on local build of 8.0.5,(on moto g5) and it shows the same behavior. On webview, it shows about:blank, no crashes on GV, it shows crah dialog, all tabs become about:blank, but I can click trash can and retype the URL to see webpages.

sblatz commented 5 years ago

Example from @npark-mozilla's device

crash no-jun

And the stacktrace he is getting:


--------- beginning of crash
01-03 11:05:23.992 10306 10357 E AndroidRuntime: FATAL EXCEPTION: IntentService[CrashHandlerService]
01-03 11:05:23.992 10306 10357 E AndroidRuntime: Process: org.mozilla.focus.debug:mozilla.components.lib.crash.CrashHandler, PID: 10306
01-03 11:05:23.992 10306 10357 E AndroidRuntime: java.lang.IllegalStateException: You need to call install() on your CrashReporter instance from Application.onCreate().
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at mozilla.components.lib.crash.CrashReporter$Companion.getRequireInstance$lib_crash_release(CrashReporter.kt:171)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at mozilla.components.lib.crash.handler.CrashHandlerService$logger$2.invoke(CrashHandlerService.kt:18)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at mozilla.components.lib.crash.handler.CrashHandlerService$logger$2.invoke(CrashHandlerService.kt:17)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at mozilla.components.lib.crash.handler.CrashHandlerService.getLogger(Unknown Source:7)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at mozilla.components.lib.crash.handler.CrashHandlerService.onHandleIntent(CrashHandlerService.kt:28)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:76)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at android.os.Handler.dispatchMessage(Handler.java:106)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at android.os.Looper.loop(Looper.java:164)
01-03 11:05:23.992 10306 10357 E AndroidRuntime:    at android.os.HandlerThread.run(HandlerThread.java:65)
01-03 11:05:23.999  1670  1754 I ActivityManager: Showing crash dialog for package org.mozilla.focus.debug u0
01-03 11:05:24.031  1670  1753 W BroadcastQueue: Background execution not allowed: receiving Intent { act=android.intent.action.DROPBOX_ENTRY_ADDED flg=0x10 (has extras) } to com.google.android.gms/.stats.service.DropBoxEntryAddedReceiver```
vesta0 commented 5 years ago

@sblatz I agree that we need to track this as 2 separate issue, prioritizing the issue with open blank tabs having to be forces closed.

A few observations to help narrow down the issue (@npark-mozilla please confirm) -This is only an issue when more than one tab is open. Crash reporter shows up AND closes the tab when we only have one tab: -If I open 2 tabs, and try crashing the second tab, the crash reporter works as expected - no force closing required.

(I am on 8.0.5 with GV)

sv-ohorvath commented 5 years ago

@vesta0 It's true, it happens only if there are at least 2 tabs opened and the first crashes. I tried to narrow it down in #4090. On nightly, it behaves differently now than Release.

vesta0 commented 5 years ago

@sblatz let's confirm that none of these issues impact a single tab

sblatz commented 5 years ago

On Moto G4 (6.0.1) I am having the following results with a single tab

Master

  1. WebView: about:crashcontent sends me directly to about:blank without a crash reporter
  2. GV: about:crashcontent gives me the "Firefox Focus has stopped" message and gives me about:blank without a crash reporter

Nightly (8.0.4)

  1. WebView: about:crashcontent sends me directly to about:blank without a crash reporter
  2. GV: about:crashcontent gives me the crash reporter and works as expected

Essentially this looks like it only affects WebView or builds off of master, so I don't think it's an issue with single tabs.

~@boek any insight as to why crash reporter would not be showing on a build off master but does on nightly; is this something to be concerned about (#3854)?~

Confirmed with Jeff that crash reporter will not be displayed unless Telemetry is enabled.

TL;DR: Crashing is working as expected with single tabs :)

vesta0 commented 5 years ago

Thank you for the detailed investigation @sblatz :)

cpeterso commented 5 years ago

GeckoView doesn't use a separate content process for each tab yet (aka "e10s-multi": https://bugzilla.mozilla.org/show_bug.cgi?id=1530770). So if one tab crashes, then it is expected that all tabs will crash in Focus using GV.

ebalazs-sv commented 3 years ago

This issue is reproducible on Focus 8.11.0 GV 84 with Pixel 2 (Android 9).

lobontiumira commented 3 years ago

This issue still reproduces on 8.15.1 build with Samsung Galaxy Note 8 (Android 9).

lobontiumira commented 3 years ago

This is still reproducible on 8.15.2 build with Sony Xperia Z5 Premium (Android 7.1.1). I had 4 tabs opened, triggered about:crashcontent on one of them, closed the tab, and the three tabs remaining were blank.

cpeterso commented 3 years ago

This bug should be (partially) fixed when Android e10s-multi is rides the trains in https://bugzilla.mozilla.org/show_bug.cgi?id=1699464. Fenix will then have multiple tab (content) processes, but I think all the tabs will need to share three content processes. If you have many tabs open and one crashes, then all the tabs sharing that same content process (so probably about 1/3 of your tabs) will also crash.

lobontiumira commented 3 years ago

This is still reproducible on 8.16.0 build with Google Pixel (Android 10).

lobontiumira commented 3 years ago

I was able to reproduce it on Focus 8.17.0 with GV 90, with OnePlus 5T (Android 10), and Google Pixel (Android 10), following these STR:

  1. Open cnn.com.
  2. Long-tap on a linked text, and select to open a new tab.
  3. Don't switch to the newly opened tab.
  4. In the URL bar insert about:crashcontent, and select to close the tab.
  5. The latest opened tab remains blank, unusable.

Note: If I switch to the newly opened tab search bar, insert about:crashcontent, select to close the tab, then I'm redirected to the previous page, which works as expected.

lobontiumira commented 3 years ago

Reproducible on 91.1.0 Focus build with these scenarios, using a Google Pixel (Android 10).

LaurentiuApahideanSV commented 3 years ago

The issue is still reproducible on Focus 91.2.2 following these steps:

  1. Open cnn.com;
  2. Open several links in new tabs by long tapping on them;
  3. Don't switch to the newly opened tab;
  4. In the URL bar insert about:crashcontent, and select to close the tab;
  5. One of the tabs opened at step 2 will become blank and unusable.

Device used:

lobontiumira commented 3 years ago

I was able to reproduce this issue on the 93.0.2 (build 352522329 - GV 93.0a1-20210906031657) with Lenovo Tab M10 (Android 10).

mcarare commented 3 years ago

@lobontiumira Is this behaviour different in Fenix?

lobontiumira commented 3 years ago

Yes. On the 94.0a1 Nightly from 9/14, after closing the crashed tab, the other tabs are loading, and can be visited without issues.

Sdaswani commented 3 years ago

Glad this was finally fixed!

lobontiumira commented 3 years ago

Verified as fixed on the latest debug build from main from 9/16 (build 11 - GV 93.0a1-20210906031657) with OnePlus 5T (Android 10), and Samsung Galaxy Note 8 (Android 9).

lobontiumira commented 3 years ago

Verified also on 93.0.3 build with Lenovo Tab M10 (Android 10).