Open Bug 979899 Opened 6 years ago Updated 1 year ago

Regression: Swiping away FxAccountStatusActivity will disable all widget UI in GeckoPreferences back in the browser

Categories

(Firefox for Android :: General, defect, P5)

30 Branch
ARM
Android
defect

Tracking

()

Tracking Status
firefox29 --- affected
firefox30 --- affected
firefox31 --- affected
firefox32 --- affected
firefox33 --- affected
firefox34 --- affected
firefox35 --- affected
firefox36 --- affected
firefox37 --- affected
firefox38 --- affected
fennec + ---

People

(Reporter: aaronmt, Unassigned)

References

Details

(Keywords: regression, reproducible, Whiteboard: [has partial patches])

Attachments

(1 file)

Currently in Nightly (03/05), when one swipes away an active FxAccountStatusActivity from Android's recent application listing, and then returning back to the browser, all accessible widget UI becomes disabled.

This does not happen when tapping the directional arrow in the top left corner to return from FxAccountStatusActivity back to GeckoPreferences

--
Nightly (03/05)
Samsung Galaxy SIV (Android 4.4.2)
This resolves by hitting back (returning to the browser) and then re-entering GeckoPreferences (Settings).
I saw this once when testing against Android 2.3.6; it manifested itself as the menu no longer responding.  Presumably we're doing something wrong with the Activity/Fragment onPause cycle. rnewman, do you have any thoughts?

I will investigate this ASAP because this is a ship-blocker for the new StatusActivity from my perspective.  I wonder if this was an issue with the old StatusActivity?  More to come on this ticket.
Duplicate of this bug: 979915
Status: NEW → ASSIGNED
Three hypotheses:

* It's a bug in GeckoPreferences' activity lifecycle.
* It's a bug in Android's handling of the activity lifecycle.
* It's a bug in the preferences UI system.
(bug 974134 was aurora+'ed, so going to mark this as 29 affected)
Ah this is reproducible w/o bug 974134 on 29 regardless.
(In reply to Aaron Train [:aaronmt] from comment #6)
> Ah this is reproducible w/o bug 974134 on 29 regardless.

This is really valuable, thanks so much.  (I'll verify, but this is the first thing I wanted to understand.)  This suggests that the unknown rat's nest of new PreferencesFragment code is not the issue; but instead, there are Activity lifecycle issues between GeckoPreferences and its child activities.

Investigation proceeding today.
tracking-fennec: ? → 29+
Nick, can you assign this to someone else or give a status update?
Flags: needinfo?(nalexander)
(In reply to Richard Newman [:rnewman] from comment #8)
> Nick, can you assign this to someone else or give a status update?

Status update is I haven't had time to look at it.  I expect this has nothing to do with FxA per se, and something to do with spawning activities from GeckoPreferences, so I would expect any front end dev to be able to make progress investigating.
Flags: needinfo?(nalexander)
Assignee: nalexander → nobody
Status: ASSIGNED → NEW
Initial findings:

This is related to setting FLAG_ACTIVITY_NEW_TASK at:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/preferences/SyncPreference.java#39

If we add that flag and launch the Sync 1.1 activity instead, we get the same (buggy) behaviour when killing the task via the launcher.  Since we don't currently start a new task for Sync 1.1, we don't see this behaviour.
See Bug 965020 (derived from Bug 964883).

I'd love for things to work if we take it out, but I probably added it for a reason.

I guess the interesting question here is why GeckoPreferences doesn't resume correctly when its task is restored.
(In reply to Richard Newman [:rnewman] from comment #11)
> See Bug 965020 (derived from Bug 964883).
> 
> I'd love for things to work if we take it out, but I probably added it for a
> reason.

It does -- technically -- work if the flag is removed, but the new task behaviour is actually desirable.

> I guess the interesting question here is why GeckoPreferences doesn't resume
> correctly when its task is restored.

That appears to be a little involved.  Everything seems fine with the Activity and Fragment lifecycle, save that the GeckoPrefs invocation called by the second Activity.onCreate (the one setting up the Preferences Activity after swiping away) never gets the Preferences:Data message.  Clearly, there's an issue with adding/removing/triggering a fresh Preferences:Get message.  So it appears to be more that GeckoPrefs is fragile or not used correctly, rather than GeckoPreferences itself.
I suspect this is related (if only by lifecycle siblinghood) to Bug 949495, because the rehydrated browser exhibits that bug.

Adding a bunch of logging shows that after the resume we've got a new EventDispatcher, at least.
Ah, Gecko not running, and BrowserApp (and ancestors) not expecting anyone else to have run before them.

03-24 19:02:54.388  6967  6967 I GeckoEventDispatcher: Registering Preferences:Data.
03-24 19:02:54.388  6967  6967 I GeckoEventDispatcher: Registered Preferences:Data.
03-24 19:02:54.388  6967  6967 I GeckoPreferenceFragment: Request is 1

...
  hit Back to launch BrowserApp:
...

03-24 19:04:14.748  6967  6992 I GeckoEventDispatcher: EventDispatcher got {"type":"Preferences:Data","requestId":1,"preferences":[]}
03-24 19:04:14.748  6967  6992 D GeckoPrefsHelper: Preferences:Data message had an unknown requestId; ignoring
I started hacking away until I got GeckoPreferences to load prefs on resume. As I mentioned in an earlier comment, the issue is that Gecko isn't running, and thus isn't listening for the prefs fetch until you trigger it to load by opening the browser.

Note that hitting "Back" to get to BrowserApp will now cause a crash, because BrowserApp didn't set up the viewport, but GeckoAppShell et al report themselves as initialized.

This cuts to some of the same issues in Bug 969637 -- GeckoApp et al assume that they're going to be the first activity to come to life, and thus init of Gecko is intimately tied to the browser, despite it being quite possible to enter Fennec in a non-browser Activity.

That we need to do all of the crap in this patch, complete with lying about a LayerView, makes me very sad -- there should be no reason why we can't pass a headless Gecko around. My guess is that snorp is working on that. If those improvements won't be ready in time, we need to proceed with the core of this patch.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
> That we need to do all of the crap in this patch, complete with lying about
> a LayerView, makes me very sad -- there should be no reason why we can't
> pass a headless Gecko around. My guess is that snorp is working on that. If
> those improvements won't be ready in time, we need to proceed with the core
> of this patch.

I disagree that we want to hack around this.  This just isn't that big a deal, yet. Lots of Android devices don't even allow you to do this "swiping" away.  We only launch one or two activities from GeckoPreferences, and only this one is launched as a separate task.

So I'd gently suggest that we want to address the underlying issues on a reasonable time frame, and mitigate here, or ignore, depending on the pain of the underlying issues and the hacks in your patch (which I am pointedly not looking at tonight).
(In reply to Nick Alexander :nalexander from comment #16)

> I disagree that we want to hack around this.

Broadly I concur. Mentally couch my statements as "assuming we consider this worth fixing".

> Lots of Android devices don't even allow you to do this "swiping"
> away.

Bear in mind that this will also bite if you task switch and the activity has been killed, I think.
Slipping this to 30, 'cos it won't get done.
tracking-fennec: 29+ → 30+
Assignee: rnewman → nobody
Status: ASSIGNED → NEW
tracking-fennec: 30+ → +
Whiteboard: [has partial patches]
Duplicate of this bug: 1058619
filter on [mass-p5]
Priority: -- → P5
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
You need to log in before you can comment on or make changes to this bug.