830358 - 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

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Jason Smith [:jsmith]]

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.

Comment 1

[Jason Smith [:jsmith]]

Hmm...very easy to reproduce too.

Comment 2

[(inactive after 6/18) Alive Kuo [:alive]]

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.

Comment 3

[(inactive after 6/18) Alive Kuo [:alive]]

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
?

Comment 4

[Jason Smith [:jsmith]]

(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.

Comment 5

[(inactive after 6/18) Alive Kuo [: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.

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

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..

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/7656

Patch v1:
1. Move TrustedUI related logic out of Window Manager
2. TrustedUI cleanup

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

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.

Comment 9

[Jason Smith [:jsmith]]

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?

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

Jason,
I will look into what should be done to trusted ui with payment according to comment 8 and give feedback soon.

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

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.

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

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....

Comment 13

[Jason Smith [:jsmith]]

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.

Comment 14

[Fernando Jiménez Moreno [:ferjm]]

(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.

Comment 15

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

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.

Comment 16

[Alex Keybl [:akeybl]]

Tracking for uplift consideration to v1.x if a low risk fix is found.

Comment 17

[BMO Automation]

Firefox OS is not being worked on