If a trusted UI within an app closes in the background, the homescreen disappears until the phone put to sleep + unlocked & the app using the trusted UI gets stuck

RESOLVED WONTFIX

Status

Firefox OS
Gaia::System::Window Mgmt
--
critical
RESOLVED WONTFIX
5 years ago
3 months ago

People

(Reporter: jsmith, Assigned: alive)

Tracking

({hang, regression, reproducible})

unspecified
ARM
Gonk (Firefox OS)
hang, regression, reproducible
Dependency tree / graph

Firefox Tracking Flags

(b2g18+)

Details

(Whiteboard: [Triaged:1/17])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Build: B2G 18 1/14/2013
Device: Unagi

Steps:

1. Go to http://www.se.rit.edu/~jds2501/persona_observer_test.html
2. Start the login process
3. After your submit your creds, hit the home button
4. Wait a few seconds

Expected:

The homescreen should remain functional. When I decide to return to that app, I should be logged in with no trusted UI present.

Actual:

The homescreen completely disappears. If I then lock and unlock the phone, I can get the homescreen to reappear. If I decide to return to the browser app, the trusted UI is still shown, but remains unfunctional with white content. The only work-around at this point would be to kill the app running the trusted UI.

Additional Notes:

Bad regression from bug 825806. We need to block - one app shouldn't be able to take down the homescreen temporarily and lock itself up.
(Reporter)

Updated

5 years ago
Blocks: 825806
blocking-b2g: --- → tef?
tracking-b2g18: --- → ?
(Reporter)

Updated

5 years ago
Keywords: regression
(Reporter)

Updated

5 years ago
Keywords: hang
(Reporter)

Updated

5 years ago
Severity: normal → critical
(Reporter)

Updated

5 years ago
Blocks: 801561
(Reporter)

Comment 1

5 years ago
Hmm...very easy to reproduce too.
Keywords: reproducible
blocking-b2g: tef? → tef+
Depends on: 828182
828182 is the problem.

Currently gaia doesn't know the closed browserid iframe is owned by which dialog. It just 'guess'es. Exactly this is not a regression because bug 825806 was hit beyond this bug.
I have to say, currently the implementation of trustedui and identifier needs to be improved -- otherwise we will encounter more issues like this.
(Reporter)

Updated

5 years ago
No longer blocks: 825806
I just took a look again. Maybe we could use chromeEventId in as the unique identifier...
But this is still not very reliable.

Jason, do you have a test to setTimeout to identifier login?
I mean if you delay the call to navigator.id.login to 5 sec and then switch to another app, would the trustedUI show
?
(Reporter)

Comment 4

5 years ago
(In reply to Alive Kuo [:alive] from comment #3)
> I just took a look again. Maybe we could use chromeEventId in as the unique
> identifier...
> But this is still not very reliable.
> 
> Jason, do you have a test to setTimeout to identifier login?
> I mean if you delay the call to navigator.id.login to 5 sec and then switch
> to another app, would the trustedUI show
> ?

In my case, I wasn't necessarily testing that. I actually tested the flow of submitting my creds and hitting the home button immediately and waiting a few seconds.
Assignee: nobody → alive
(In reply to Alive Kuo [:alive] from comment #3)
> I just took a look again. Maybe we could use chromeEventId in as the unique
> identifier...
OK I am wrong, the id is different every time gaia gets a mozChromeEvent. Uh.
So it's impossible to make a reasonable multiple trustedUI implementation now unless bug 828182 gets fixed.

Another way:
Make trustedUI to be system wide. That means only one app is allowed to open trustedUI at the same time.
Seems like bug 828182 patch ensures that chromeEventId of open/close event of the same dialog is the same.
This could be fixed if 828182 is landed and check the id/frameOrigin of dialog when closing it.

Ideally I would like to move restoreCurrentApp/hideCurrentApp out of Window Manager..
(Reporter)

Updated

5 years ago
tracking-b2g18: ? → ---
Created attachment 703176 [details]
https://github.com/mozilla-b2g/gaia/pull/7656

Patch v1:
1. Move TrustedUI related logic out of Window Manager
2. TrustedUI cleanup
Attachment #703176 - Flags: review?(ferjmoreno)
Whiteboard: [Triaged:1/17]
Comment on attachment 703176 [details]
https://github.com/mozilla-b2g/gaia/pull/7656

Thanks Alive. I didn't look deep at the code, but I am afraid that this patch would break some of the functionality that we currently support. Specifically, we can't have two trusted UIs owned by to different apps at the same time.

You can test this scenario by calling mozPay() from two different apps. Unfortunately, the mock payment provider preferences were removed from Gaia, so you would have to add them yourself to 'build/payment-prefs.js':

pref("dom.payment.skipHTTPSCheck", true);
pref("dom.payment.provider.0.name", "mockpayprovider");
pref("dom.payment.provider.0.description", "Mock Payment Provider");
pref("dom.payment.provider.0.type", "mock/payments/inapp/v1");
pref("dom.payment.provider.0.uri", "http://people.mozilla.com/~kmcmillan/pay-provider.html?req=");
pref("dom.payment.provider.0.requestMethod", "GET");

You can use the mozPay test from the UITests app and the tests at http://people.mozilla.com/~kmcmillan/gaia-pay.html.

STR:
- Open the browser and load http://people.mozilla.com/~kmcmillan/gaia-pay.html
- Click on 'Pay PMPP'.
- Once trusted UI is shown with the pay flow, close the dialog by clicking on the HOME button.
- Open the UITests app and choose the 'navigator.mozPay' tests.
- Click on 'Pay PMPP'.
- Once trusted UI is shown with the pay flow, close the dialog by clicking on the HOME button.
- Open the browser app again, which should show the trusted UI again.
- Click on 'Success'.

Expected:
- The trusted UI should close and show the browser app.

Current: 
- The payment flow opened from the UITests is shown and the trusted UI remains unusable.

FTR, the payment callbacks are properly triggered.
Attachment #703176 - Flags: review?(ferjmoreno) → review-
(Reporter)

Comment 9

5 years ago
Alive or Ferjm - Is it possible to fix this bug is a low risk way?

The comments above concern me that we're trying to do a rewrite here, which will put us at risk for blocking regressions.

If it's not possible to find a low risk way, should we continue to block on this?
Jason,
I will look into what should be done to trusted ui with payment according to comment 8 and give feedback soon.
When testing with payment, there is another bug:

STR:
- Open the browser and load http://people.mozilla.com/~kmcmillan/gaia-pay.html
- Click on 'Pay PMPP'.
- Once trusted UI is shown with the pay flow, close the dialog by clicking on the HOME button.
- Open the UITests app and choose the 'navigator.mozPay' tests.
- Click on 'Pay PMPP'.
- Once trusted UI is shown with the pay flow, close the dialog by clicking on the HOME button.
- Open the browser app again, which should show the trusted UI again.
- Click on 'Success'.
- Click on 'Pay PMPP' again in the browser page.

Expected:
Trusted UI shows on homescreen

Actual:
Trusted UI shows on browser app

Log:

E/GeckoConsole(  108): Content JS LOG at app://system.gaiamobile.org/js/trusted_ui.js:80 in trui_open: open-payment-flow-dialog-{3cab09cf-758f-4986-8549-8e992893f503} app://browser.gaiamobile.org =======
I/Gecko   (  108): ======================= payment.js ======================= 
E/GeckoConsole(  108): Content JS LOG at app://system.gaiamobile.org/js/trusted_ui.js:80 in trui_open: open-payment-flow-dialog-{c2169b1a-b3df-4ef2-be02-264224a93cad} app://uitest.gaiamobile.org =======
I/Gecko   (  108): ======================= payment.js ======================= 
E/GeckoConsole(  108): Content JS LOG at app://system.gaiamobile.org/js/trusted_ui.js:98 in trui_close: open-payment-flow-dialog-{c2169b1a-b3df-4ef2-be02-264224a93cad} app://browser.gaiamobile.org =======

Note that the id returned by close payment in browser just gives the chrome id of ui-test got...this is totally wrong. This is related to Bug 828182 I think.
I agree not to block on this unless we have much time to redesign and re-test.

Bug 818292 doesn't solve the problem...chrome event id is NOT related to payment one by one. Or there is something wrong in platform....
blocking-b2g: tef+ → tef?
(Reporter)

Comment 13

5 years ago
Sounds fine to not block given the risk. Let's get this on tracking radar for a future release though - Jed indicated concern that some cleanup of the trusted UI code for a bug like this is necessary eventually (I believe the reason was fragility being the concern), although we'll have to be careful when doing the cleanup to avoid regressions.
blocking-b2g: tef? → ---
tracking-b2g18: --- → ?
Keywords: relnote
(In reply to Alive Kuo [:alive] from comment #11)
> When testing with payment, there is another bug:
> 
> STR:
> - Open the browser and load
> http://people.mozilla.com/~kmcmillan/gaia-pay.html
> - Click on 'Pay PMPP'.
> - Once trusted UI is shown with the pay flow, close the dialog by clicking
> on the HOME button.
> - Open the UITests app and choose the 'navigator.mozPay' tests.
> - Click on 'Pay PMPP'.
> - Once trusted UI is shown with the pay flow, close the dialog by clicking
> on the HOME button.
> - Open the browser app again, which should show the trusted UI again.
> - Click on 'Success'.
> - Click on 'Pay PMPP' again in the browser page.
> 
> Expected:
> Trusted UI shows on homescreen
> 
> Actual:
> Trusted UI shows on browser app
> 
> Log:
> 
> E/GeckoConsole(  108): Content JS LOG at
> app://system.gaiamobile.org/js/trusted_ui.js:80 in trui_open:
> open-payment-flow-dialog-{3cab09cf-758f-4986-8549-8e992893f503}
> app://browser.gaiamobile.org =======
> I/Gecko   (  108): ======================= payment.js
> ======================= 
> E/GeckoConsole(  108): Content JS LOG at
> app://system.gaiamobile.org/js/trusted_ui.js:80 in trui_open:
> open-payment-flow-dialog-{c2169b1a-b3df-4ef2-be02-264224a93cad}
> app://uitest.gaiamobile.org =======
> I/Gecko   (  108): ======================= payment.js
> ======================= 
> E/GeckoConsole(  108): Content JS LOG at
> app://system.gaiamobile.org/js/trusted_ui.js:98 in trui_close:
> open-payment-flow-dialog-{c2169b1a-b3df-4ef2-be02-264224a93cad}
> app://browser.gaiamobile.org =======
> 
> Note that the id returned by close payment in browser just gives the chrome
> id of ui-test got...this is totally wrong. This is related to Bug 828182 I
> think.

You are right Alive. Thanks for catching this!

I failed reviewing this code, we always take the latest chrome event Id when calling the TrustedUIManager.close() function from the payment implementation.

I am afraid that we might need to match the trusted dialogs with DOMRequest ids, instead of chrome event ids :\... I'll file a bug for this.
Thanks, Fernando and Alive.  That was my bug in the way chromeEventIds were being used.

I like the association with the DOM request ID very much.  I think it makes the logic of the flow much more transparent.  It also makes me wonder whether we need the chromeEventIds at all.  Seems like updating payment and identity callers to use the requestId for messaging and clean-up functions would make things easier to understand and maintain.
Tracking for uplift consideration to v1.x if a low risk fix is found.
tracking-b2g18: ? → +
(Reporter)

Updated

5 years ago
Blocks: 846517
(Reporter)

Updated

5 years ago
No longer blocks: 794552, 801561
(Reporter)

Updated

5 years ago
Component: Gaia::System → Gaia::System::Window Mgmt
Keywords: relnote

Comment 17

3 months ago
Firefox OS is not being worked on
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.