1379374 - Background tabs that haven't been touched will be lost on the first restart after upgrading to 55

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Vincent Lefevre]

User Agent: Mozilla/5.0 (Android 7.0; Mobile; rv:55.0) Gecko/55.0 Firefox/55.0
Build ID: 20170706211248

Steps to reproduce:

Start Firefox Beta.


Actual results:

Only 3 tabs were listed in the current open tabs.


Expected results:

All the previous tabs should have been listed.

Comment 1

[Jan Henning [:JanH]]

How many tabs did you have open previously and can you remember which tabs survived, i.e. was it
- the first three tabs,
- or the last three tabs,
- or three contiguous tabs from the middle,
- or some random selection of three tabs (but possibly still in the correct relative order)?

Comment 2

[Vincent Lefevre]

(In reply to Jan Henning [:JanH] from comment #1)
> How many tabs did you have open previously

about 8 - 10.

> and can you remember which tabs survived, i.e. was it
> - the first three tabs,
> - or the last three tabs,
> - or three contiguous tabs from the middle,
> - or some random selection of three tabs (but possibly still in the correct
> relative order)?

The first one and the last one (the 3rd tab was opened when I opened a new tab via the Twitter application, i.e. it wasn't there previously).

IIRC, these two tabs were the only tabs I read recently.

Comment 3

[Jan Henning [:JanH]]

(In reply to Vincent Lefevre from comment #2)
> IIRC, these two tabs were the only tabs I read recently.

Hmm, so there could possibly be some logic as to why those particular two tabs survived, although unfortunately without being able to reproduce this myself, I don't see anything obviously wrong there.

Comment 4

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
 - Samsung Galaxy Note 4 (Android 5.0.1);
 - HTC 10 (Android 7.0);

Builds:
 - 55.0b5 & 55.0b7;

Hello,

 I've also tried to reproduce this issue several ways but was unsuccessful. Could you maybe narrow down what you did so that we could find a way to reproduce this? 
 Do you have any addon that might interfere with the normal behavior of Firefox?
 What kind of device and android version do you have?

Methods used:
 1. Closing the app
  a. Install 55.0b5 open ~10 tabs > close app > update to 55.0b7(Play Store) > open app (all tabs present)
  b. Install 55.0b5 open ~10 tabs > close app > update to 55.0b7(using ADB) > open app (all tabs present)

 2. Sending the app in the background
  a. Install 55.0b5 open ~10 tabs > send to background > update to 55.0b7(Play Store) > open app (all tabs present)
  b. Install 55.0b5 open ~10 tabs > send to background > update to 55.0b7(using ADB) > open app (all tabs present)

 3. Restarting the phone
  a. Install 55.0b5 open ~10 tabs > restart phone > update to 55.0b7(Play Store) > open app (all tabs present)
  b. Install 55.0b5 open ~10 tabs > restart phone > update to 55.0b7(using ADB) > open app (all tabs present)

Note:
 I also tried a variant where before updating the app I used only one or 2 tabs and ignored the rest. After the update the app still returned all of my tabs.
 Tried method one using ADB with a Nightly build on the Note 4. The issue was still not reproducible.
 Changing the importance to Normal since we do not have clear steps to reproduce and this might be an edge case.

Comment 5

[Vincent Lefevre]

(In reply to Bogdan Surd, QA [:BogdanS] from comment #4)
> Could you maybe narrow down what you did so that we could find a way to
> reproduce this?

I just suddenly noticed the issue just after opening the list of tabs, being very surprised to see only 3 tabs. But I don't know when exactly they disappeared (except that that was very recent).

FYI, currently my phone has a 430 hours uptime.

>  Do you have any addon that might interfere with the normal behavior of
> Firefox?

I don't currently have any addon except the default "OpenH264 Video Codec provided by Cisco Systems, Inc." one.

>  What kind of device and android version do you have?

Samsung Galaxy S7 with Android 7.0 (Samsung's version), not rooted.

> Methods used:
>  1. Closing the app
>   a. Install 55.0b5 open ~10 tabs > close app > update to 55.0b7(Play Store)
> > open app (all tabs present)
>   b. Install 55.0b5 open ~10 tabs > close app > update to 55.0b7(using ADB)
> > open app (all tabs present)

Note that I did not close the app manually (it is just closed and restarted automatically when there is an update, which usually occurs during the night).

Moreover, in my case, the contents of the tabs that did not survive were probably no longer in the cache (well, I don't know how this works exactly, but I occasionally got errors when selecting tabs while my phone was not connected to a network, because it could not load the contents). And when Firefox was last restarted after update, I don't know whether or not my phone was connected to a network.

Comment 6

[Bogdan Surd, QA [:bsurd, NI]]

Hello,

 Left a device open over night and than ran an update on it the next day. Still could not reproduce this as all of my tabs were still there. Leaving this open for now.

Comment 9

[Jan Henning [:JanH]]

There's nothing special about app upgrades except that maybe it is more likely that Firefox is being killed while the user could be actively browsing.

If somebody wants to get the to the bottom of this, the best way would probably be to attempt automating the process along the lines of
- Turn on "browser.sessionstore.debug_logging"
- Open a Firefox session with a number of tabs
- Create some activity that triggers session store file writes (navigate to different URLs, or just scroll the page around)
- Force-stop Firefox at some random point
- Restart Firefox and check whether the expected number of tabs is open
- If yes, rinse and repeat, if no, dump the logcat and start afresh.

For the complete failures to restore it might help to create additional backups that are only written to once per session (or after history/tabs have been cleared) and otherwise left alone to minimise any chance of them being corrupted by an interrupted write. That wouldn't recover any short-term browsing, but might at least help to preserve any tabs used as long-term storage.

I'd still be happier, though, if I could actually understand why the backup file we already have doesn't help here before adding even more layers of backup.

Also, that wouldn't help for those mysterious instances where a session is only partially restored - it's rather unlikely (although who knows) that some simple file corruption manages to hit sessionstore.js so selectively that a number of tabs go missing but the file remains syntactically valid and doesn't trigger any JSON parsing errors. If so, the tabs must have been lost at some other stage, in which case the session parser cannot easily detect that fact and switch to a backup.

Comment 10

[Vincent Lefevre]

(In reply to Jan Henning [:JanH] from comment #9)
> There's nothing special about app upgrades except that maybe it is more
> likely that Firefox is being killed while the user could be actively
> browsing.

In my case, Firefox was upgraded while I wasn't using it, probably during the night, when I was not using my phone.

> If somebody wants to get the to the bottom of this, the best way would
> probably be to attempt automating the process along the lines of
> - Turn on "browser.sessionstore.debug_logging"

I don't have the time to do extensive tests. Perhaps the bug will occur again in the future, but since I don't know when, I can't turn on browser.sessionstore.debug_logging just before. Is there anything wrong if browser.sessionstore.debug_logging is turned on permanently?

Note: Memory space should not be a problem for me as I have dozens of GB free memory on my SD card.

> Also, that wouldn't help for those mysterious instances where a session is
> only partially restored - it's rather unlikely (although who knows) that
> some simple file corruption manages to hit sessionstore.js so selectively
> that a number of tabs go missing but the file remains syntactically valid
> and doesn't trigger any JSON parsing errors. If so, the tabs must have been
> lost at some other stage, in which case the session parser cannot easily
> detect that fact and switch to a backup.

It's well-know that Firefox can loose a URL in case of failure to fetch the page, and tabs can be lost for this reason; see bug 1163981 ("When Firefox is restarted, the tab is missing."). It might be a similar issue here in case Firefox tried to reload the tabs at some point (before the upgrade? at startup after the upgrade?) but couldn't, making it to regard them as blank tabs.

This might also be related to bug 956974, which is about the same problem as bug 1163981.

Comment 11

[Djfe]

Had the same issue yesterday after the automatic update from the playstore (release).
Firefox was likely still open in the background, shortly before the update.
The update happened while I was watching videos with the YouTube app. (my device has 6gb of ram so Firefox was likely still loaded)

one thing to mention:
my oneplus 3t (stock) kills apps which are running in the background, when they are using too much CPU. 
it's non standard and a hard kill (which can likely lead to data loss)

this is the first time ever though, that my Firefox has lost tabs (on Android)

it's suspicious though that this issue popped up right with the 55 update, so I would rule this out for now.
(but it might be a combination of issues)

what I know is:
my device had an uptime > 100h and quite a bit over 100 open tabs.
after the auto upgrade: only 3 tabs remained (the last 3 from the list, which where previously at the bottom)

It likely has happened to more people (see the other duplicated bug report)

Pls invest some time into this.
Maybe it actually is device/firmware related.
I'm running Android 7.0 (Oxygen OS 4.0.3)

Comment 12

[Djfe]

at least I think it were the last three tabs from the previous list, I'm not sure about it

Comment 13

[Jan Henning [:JanH]]

[Tracking Requested - why for this release]: We're losing all tabs for users who are upgrading from a previous version.

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

Liz, we should get the patch for this into 55 once Jan puts it up. We may also want to hold the 55 rollout if that's still possible.

Comment 16

[Jan Henning [:JanH]]

Attachment: Bug 1379374.patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1351808
[User impact if declined]: When upgrading from a previous version to 55, all restored background tabs that aren't touched during the first session after the update will be lost.
[Is this code covered by automated tests?]: Partially.
[Has the fix been verified in Nightly?]: Reproduced locally.
[Needs manual test from QE? If yes, steps to reproduce]: Install 54, open some tabs, background Firefox again. Upgrade to 55, start Firefox and *don't* touch any of the other restored tabs. Kill Firefox (swipe away in the task manager) and launch it again. Without this patch, all other tabs should be lost. With this patch, everything should work normally.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This just checks that the tab type attribute is present at all before making any decisions based on it. If the tab type attribute isn't present, we just default to saving the tab data as normal.
[String changes made/needed]: none

Comment 17

[Djfe]

this proofs migrating data isn't trivial.
Even though I lost my tabs, it's not too much of a loss.
I'm glad this will be fixed now. Ty for investing your time, looking into this :)

Hopefully the added test file will prevent this kind of stuff from happening again.

Comment 18

[Djfe]

I just realized two things:
Does anything speak against setting tab.type to BROWSING if the attribute doesn't exist? (on import)
Else people with over 100 tabs will have tabs without the attribute for a longer time,
which could lead to further issues when you're probing that attribute in future code, right? (I'm not that familiar with Firefox code, just a user with a bit knowledge about coding)

Will there actually be test code, that tests whether tabs are saved correctly?

Comment 19

[Jan Henning [:JanH]]

Just so there isn't any confusion - only 55 is affected, so the patch needs to land on release only. With the move to GeckoView-based custom tabs/web apps, the whole tab type infrastructure was removed again in bug 1366098.

(In reply to Djfe from comment #18)
> I just realized two things:
> Does anything speak against setting tab.type to BROWSING if the attribute
> doesn't exist? (on import)
> Else people with over 100 tabs will have tabs without the attribute for a
> longer time,
> which could lead to further issues when you're probing that attribute in
> future code, right? (I'm not that familiar with Firefox code, just a user
> with a bit knowledge about coding)

See above - the tab type code was removed again in Firefox 56 because it wasn't necessary after all. Since that check is the only place where we query the tab type as stored on the *session store* data - as opposed to the actual tab object - that fix will be enough.

> Will there actually be test code, that tests whether tabs are saved
> correctly?

See bug 1389564.

Comment 20

[Djfe]

since quite a lot of users left reviews about this on the play store,
you might wanna mention it in the upcoming changelog there (and maybe write an apology?)

English is probably enough since it's temporary
translation would add too much delay to the release of this fix

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8896387 [details] [diff] [review]
Bug 1379374.patch

Taking it, this is a driver for 55.0.2 for fennec.

Comment 22

[Bayle Shanks]

I was hit by this bug. Even if i cannot restore my tabs, is there some way for me to manually look at my old sessionstore backup files to see what my old tabs were? My Android device is not rooted, and i don't seem to be able to access my profile folder through adb or a file explorer app. Sessionmanager add-ons seem to not be compatible with Firefox 55. Perhaps there is some way to install an old version of Firefox, then install a session manager, in order to browse old sessionstores?

If so, this should be added to the notes in the Play Store.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/db83b30e6c00

Comment 24

[Bayle Shanks]

If there is not currently a way to view old sessionstores, and if old sessionstores have the lost tabs, then you might consider adding an about: page that allows users to view and export sessionstore backups (or, better, the entire profile folder). Assuming there are still old sessionstore backups, this would allow users to recover from this bug.

Comment 25

[Bayle Shanks]

Alternately, perhaps Mozilla has a backup of the sync server from yesterday that would still have copies of recently deleted tabs, for users who were synced?

Comment 26

[Jan Henning [:JanH]]

It landed on Release and doesn't need to land anywhere else, so this can be marked fixed.

Comment 27

[u549602]

Verified as fixed on the latest RC 55.0.2. 

Tried this issue on Samsung Galaxy S8 - Android 7 and Nexus 6P - Android 8. 
Could not reproduce the issue by following the STR mentioned @comm 6 

I have to mention that the method used to test this was to install the 55.0.2 apk over the already present 54.0.1 build.

Comment 28

[Sylvestre Ledru [:Sylvestre]]

Added in the release notes of 55:
	Fix an issue with tab restoration