1351808 - Exclude custom tabs from session store

Status: RESOLVED FIXED

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

Description

[Jan Henning [:JanH]]

Until we can figure out further details of whether and how we're going to handle custom tabs and web app tabs in session restore, we should exclude at least the former, otherwise after a restart, they'll be restored as normal tabs.

Web app tabs could potentially even work out okay without further changes - when they're restored on startup, we notice that their URL is within the scope of a web app, so we turn the tab over to the WebAppActivity. In practice this might still require some bug fixes, but in any case that's a separate bug...

Comment 1

[Jan Henning [:JanH]]

Actually web apps need to be excluded as well for now - they run in their own activity and there's no such thing as a background activity restore.

If we want anything more advanced, we can try figuring it out in bug 1346008 if there's any demand for it. Until that time, the minimal behaviour with bug 1352997 in place will be that if we can't find the tab belonging to the activity (it's been closed, or Firefox as whole has terminated), we'll relaunch it from the intent data.

Comment 2

[Jan Henning [:JanH]]

Attachment: Bug 1351808 - Part 1 - Replace some magic numbers in session store.

Review commit: https://reviewboard.mozilla.org/r/128066/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128066/

Comment 3

[Jan Henning [:JanH]]

Attachment: Bug 1351808 - Part 2 - Exclude non-standard tab types from session store.

Restoring anything other than normal browsing tabs (e.g. custom tabs, web apps) is more involved because those tabs
- don't appear in our normal tabs UI
- are opened in separate activities
- when we're starting up, Android's task switcher might or might not still have available task entries corresponding to such tabs from the last session

Therefore, for now, the session store will simply exclude those kinds of tabs from being saved in the session store data.

Instead of a real restore, if the corresponding tab has been closed or Gecko stopped running, we just recreate the custom tab/web app based on the stored Activity intent data we have available (bug 1352997).
Tab zombification while Gecko is running however remains fully supported, as we continue collecting session history data for all tab types, even if we don't necessarily save it to disk.

Because custom tabs/web apps currently still share a common Gecko browser window with normal tabs, we also have to modify our selected tab tracking logic accordingly, so that selecting one of these special tab types doesn't overwrite the last selected normal browsing tab.

To that effect, we now track the selected tab *ID* in memory and only convert that to a tab index when writing the data to disk. As the ID remains stable while Gecko is running, this makes tracking changes for a sub-group of tabs only easier, as we don't have to watch out for closing tabs of *any* kind affecting the tab index of everything behind them.

Bug 1346008#c3 has some preliminary ideas on how session restoring for custom tabs/web apps could be made to work.

Review commit: https://reviewboard.mozilla.org/r/128068/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/128068/

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8856159 [details]
Bug 1351808 - Part 1 - Replace some magic numbers in session store.

https://reviewboard.mozilla.org/r/128066/#review131960

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8856167 [details]
Bug 1351808 - Part 2 - Exclude non-standard tab types from session store.

https://reviewboard.mozilla.org/r/128068/#review131964

Comment 6

[Jan Henning [:JanH]]

Comment on attachment 8856159 [details]
Bug 1351808 - Part 1 - Replace some magic numbers in session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128066/diff/1-2/

Comment 7

[Jan Henning [:JanH]]

Comment on attachment 8856167 [details]
Bug 1351808 - Part 2 - Exclude non-standard tab types from session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128068/diff/1-2/

Comment 8

[Jan Henning [:JanH]]

Comment on attachment 8856159 [details]
Bug 1351808 - Part 1 - Replace some magic numbers in session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128066/diff/2-3/

Comment 9

[Jan Henning [:JanH]]

Comment on attachment 8856167 [details]
Bug 1351808 - Part 2 - Exclude non-standard tab types from session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128068/diff/2-3/

Comment 10

[Jan Henning [:JanH]]

Comment on attachment 8856159 [details]
Bug 1351808 - Part 1 - Replace some magic numbers in session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128066/diff/3-4/

Comment 11

[Jan Henning [:JanH]]

Comment on attachment 8856167 [details]
Bug 1351808 - Part 2 - Exclude non-standard tab types from session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128068/diff/3-4/

Comment 12

[Jan Henning [:JanH]]

Comment on attachment 8856159 [details]
Bug 1351808 - Part 1 - Replace some magic numbers in session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128066/diff/4-5/

Comment 13

[Jan Henning [:JanH]]

Comment on attachment 8856167 [details]
Bug 1351808 - Part 2 - Exclude non-standard tab types from session store.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/128068/diff/4-5/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f349db3287ad
Part 1 - Replace some magic numbers in session store. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/4aa74fee0d37
Part 2 - Exclude non-standard tab types from session store. r=sebastian

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for bustage on Android in GeckoApp.java:

https://hg.mozilla.org/integration/autoland/rev/e8f1286bc87e54d0cee728cfd46853ae3176ad30
https://hg.mozilla.org/integration/autoland/rev/742f57fd66dd09f309940015dd0841c2b921369b

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4aa74fee0d37fedef9d3c416f41917b09223836f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=94917738&repo=autoland

[task 2017-04-27T18:30:37.802300Z] 18:30:37     INFO -  /home/worker/workspace/build/src/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java:1866: error: cannot find symbol
[task 2017-04-27T18:30:37.802547Z] 18:30:37     INFO -                      windowObject.put("selectedTabId", parser.getStoredSelectedTabId());
[task 2017-04-27T18:30:37.802723Z] 18:30:37     INFO -                                                              ^
[task 2017-04-27T18:30:37.802886Z] 18:30:37     INFO -    symbol:   method getStoredSelectedTabId()
[task 2017-04-27T18:30:37.803086Z] 18:30:37     INFO -    location: variable parser of type LastSessionParser
[task 2017-04-27T18:30:39.031169Z] 18:30:39     INFO -  Note: Some input files use or override a deprecated API.
[task 2017-04-27T18:30:39.031421Z] 18:30:39     INFO -  Note: Recompile with -Xlint:deprecation for details.
[task 2017-04-27T18:30:39.031583Z] 18:30:39     INFO -  1 error

Comment 16

[Jan Henning [:JanH]]

Ooops, yes, forgot that this bug wasn't JS only and depends on a previous bug as well (which failed to land because there are conflicting changes on autoland).

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50de746d569e
Part 1 - Replace some magic numbers in session store. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/11dceec1864b
Part 2 - Exclude non-standard tab types from session store. r=sebastian

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/50de746d569e
https://hg.mozilla.org/mozilla-central/rev/11dceec1864b