Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Custom Tab: Session shows blank page when the browser is killed #3773

Closed
jonalmeida opened this issue Jul 15, 2019 · 15 comments
Closed

Custom Tab: Session shows blank page when the browser is killed #3773

jonalmeida opened this issue Jul 15, 2019 · 15 comments
Assignees
Labels
🐞 bug Something isn't working <customtabs> Component: feature-customtabs 🐉 Fenix Feature needed for Fenix 🌐 reference browser Features, bugs, issues related to the reference browser implementation

Comments

@jonalmeida
Copy link
Contributor

jonalmeida commented Jul 15, 2019

We lose the intent that launched the CT when the app was killed so we can't restore styling and the session that was used i.e. the intent and the Session is lost.

Related issues:

┆Issue is synchronized with this Jira Task

@jonalmeida jonalmeida added 🐞 bug Something isn't working 🐉 Fenix Feature needed for Fenix 🌐 reference browser Features, bugs, issues related to the reference browser implementation <customtabs> Component: feature-customtabs labels Jul 15, 2019
@jonalmeida
Copy link
Contributor Author

This might be related to #3310 so should be re-tested after that lands.

Assigning self to follow-up.

@jonalmeida jonalmeida self-assigned this Jul 22, 2019
@jonalmeida
Copy link
Contributor Author

@jonalmeida test this in 24 hours.

@jonalmeida
Copy link
Contributor Author

I tested this with custom tabs on r-b and I still see this issue.

@pocmo
Copy link
Contributor

pocmo commented Jul 31, 2019

@jonalmeida Do you have STR for this?

Is it possible to quick-fix this by finishing the custom tab activity if there's no intent?

@jonalmeida
Copy link
Contributor Author

My STR:

  • Note the last selected session in r-b (e.g. google.com)
  • Open a custom tab
  • Play a mobile game for an hour.
  • Go back to the custom tab and observe the session there.
    • If it shows google, then you know that the CT session was lost.

Is it possible to quick-fix this by finishing the custom tab activity if there's no intent?

I think so, iirc this is roughly what Chrome was doing.

@jonalmeida
Copy link
Contributor Author

jonalmeida commented Jul 31, 2019

I've found the issue to our problem.

When we have an activity that was opened via an intent, and we background that app so that it gets reclaimed by the OS, the last activity is re-created and the original intent is still there.

In our designs, we assumed the IntentReceiverActivity was the only entry point, so intent processing was done there, then the activity is finished and the BrowserActivity is started to load the processed intent in the new activity.

Now we see that we're getting the original intent in the BrowserActivity but we don't know how to handle it because no session was created for it (which is done by the intent processor).

This would also be the case for Fenix as well (but s/BrowserActivity/HomeActivity/). I tried to see if this simple change would work for Fenix, but it's a much more complex beast with it's complexity.

The quick fix for this in r-b/f-b is to do components.utils.intentProcessor.process(intent) in the CustomTabActivity before calling super (the BrowserActivity). I'm not sure if this is the right fix though. ¯\_(ツ)_/¯

@jonalmeida
Copy link
Contributor Author

jonalmeida commented Aug 28, 2019

What I've found so far with some extra logging when launching a Custom Tab.

The initial trace creates the activity, the fragment and then launches the url with UI customizations:

11637-11637/org.mozilla.rb.debug: IntentReceiverActivity: onCreate
11637-11637/org.mozilla.rb.debug: BrowserActivity: onCreate - 8f2cef56-2650-4799-8c78-3cc0fd149f83
11637-11637/org.mozilla.rb.debug: ExternalAppBrowserActivity: createBrowserFragment - 8f2cef56-2650-4799-8c78-3cc0fd149f83
11637-11637/org.mozilla.rb.debug: ExternalAppBrowserFragment: onViewCreated - 8f2cef56-2650-4799-8c78-3cc0fd149f83
11637-11637/org.mozilla.rb.debug: BaseBrowserFragment: onViewCreated - 8f2cef56-2650-4799-8c78-3cc0fd149f83
11637-11637/org.mozilla.rb.debug: session's CT config: CustomTabConfig(id=6d1dc9fb-2500-4f53-8f6c-eb349b650586, toolbarColor=-16777216)

Then, without backing out of the Custom Tab, and switching to use other apps for bit before returning shows the following trace (where the Custom Tab is blank without styling):

16255-16255/org.mozilla.rb.debug: BrowserActivity: onCreate - 8f2cef56-2650-4799-8c78-3cc0fd149f83
16255-16255/org.mozilla.rb.debug: ExternalAppBrowserFragment: onViewCreated - 8f2cef56-2650-4799-8c78-3cc0fd149f83
16255-16255/org.mozilla.rb.debug: BaseBrowserFragment: onViewCreated - 8f2cef56-2650-4799-8c78-3cc0fd149f83
16255-16255/org.mozilla.rb.debug: session's CT config: null

Observations:

  • So it seems we still have the same session ID.
  • We're losing the Session and hence the CustomTabConfig.

@pocmo
Copy link
Contributor

pocmo commented Aug 28, 2019

@jonalmeida Regarding switching to use other apps for bit -> This can't be reproduced with "don't keep activities" or disabling background processes in dev settings?

@jonalmeida
Copy link
Contributor Author

Interesting.. with a modified version of LegacySessionManager#createSnapshot to persist the custom tabs, we can see this working better.

  • The session is persisted so the URL can be re-loaded.
  • The CustomTabConfig is not serialized so we still lose that.

@jonalmeida
Copy link
Contributor Author

@jonalmeida Regarding switching to use other apps for bit -> This can't be reproduced with "don't keep activities" or disabling background processes in dev settings?

@pocmo From my last update it seems that it's persistence related, so re-creating the activity doesn't make a difference.

@jonalmeida
Copy link
Contributor Author

jonalmeida commented Aug 28, 2019

Should we:
a. Persist Custom Tab sessions (which can also be PWA/TWA sessions soon)?
b. Acknowledge that the session no longer exists and just gracefully close the session? (this is what Chrome does)

@pocmo
Copy link
Contributor

pocmo commented Aug 28, 2019

@pocmo From my last update it seems that it's persistence related, so re-creating the activity doesn't make a difference.

Then disabling background processes should reproduce it if you move the app to the background and then resume?

a. Persist Custom Tab sessions (which can also be PWA/TWA sessions soon)?
b. Acknowledge that the session no longer exists and just gracefully close the session? (this is what Chrome does)

Not persisting them was a design choice so far. So I'd vote for (B) for now.

@jonalmeida
Copy link
Contributor Author

jonalmeida commented Aug 28, 2019

Then disabling background processes should reproduce it if you move the app to the background and then resume?

I tried this too, and it makes it easier to reproduce but I still have to use other apps for a bit for some reason before I can return to the Custom Tab. 🤷‍♂

Not persisting them was a design choice so far. So I'd vote for (B) for now.

Spoke with @NotWoods about this for a bit offline: it should be safe to do this for TWAs as well, although the user flow seems unfortunate: if you went from page A -> B, and returned later, you would always start back at A instead of the last context you were in (B).

A nice-to-have fix would be to persist the session and config, so you can return to the original context, but we can persist the session forever if we lose the CT from an app kill without ever deleting it. We could also add a TTL to the session to remove them if they build up and remove them periodically, similar to autosave.

@jonalmeida
Copy link
Contributor Author

I've put up a fix for R-B where we check if the session exists in the CustomTabIntegration.kt and finish the activity if the session for that ID no longer exists.

@jonalmeida
Copy link
Contributor Author

Going to close this as done since the fixes for it need to go into each individual app so that it handles removed sessions in the CustomTabIntegration classes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Something isn't working <customtabs> Component: feature-customtabs 🐉 Fenix Feature needed for Fenix 🌐 reference browser Features, bugs, issues related to the reference browser implementation
Projects
None yet
Development

No branches or pull requests

3 participants