krupa raj[:krupa] Reporter
	Description • 12 years ago

unagi build: 03/29 from b2g18 branch (v1.0)

Pre-conditions:
* Tester has their unagi phone set up for payments on -dev
* Tester has marketplace-dev app installed


steps to reproduce:
1. Launch marketplace-dev app
2. Start the purchase flow for any paid app
3. Sign in using Identity within the trustedUI
4. Enter PIN
5. Confirm your mobile number and enter the bango PIN
6. Notice that the Confirm your Purchase screen loads
7. Close the app and launch some other apps (I launched Settings and another app I had installed)
8. Return to the Marketplace app

expected behavior:
After step #8, user returns to the purchase flow within the trustedUI

actual behavior:
After step #8, user returns to the marketplace page where they initiated the payment from (details or search results page) and then the homepage loads.

reproducible? I was able to reproduce this behavior thrice.

	Jason Smith [:jsmith]
	Comment 2 • 12 years ago

(In reply to krupa raj 82[:krupa] from comment #0)
> actual behavior:
> After step #8, user returns to the marketplace page where they initiated the
> payment from (details or search results page) and then the homepage loads.

This last step sounds like the marketplace app process *was* killed after you launched the other app - the user returns back to where the payment was from and then the homepage loads means you saw a screenshot of the old payment marketplace site and was returned to the homepage of the app since the app process was previously killed.

The strange issue is that we're getting killed a lot quicker than I initially expected. So my hunch is this likely a process manager bug in Justin's area of expertise. Likely a regression too, cause you mentioned, we weren't seeing this not too long ago. leo? nom for blocking consideration.

	Justin Lebar (not reading bugmail)
	Comment 3 • 12 years ago

Can you please include dmesg logs?  Unfortunately dmesg has some information that I'm not able to put into logcat.

Anyway Jason is probably right, it looks from the logs like something is getting killed here.  dmesg will give us more information.

	krupa raj[:krupa] Reporter
	Comment 4 • 12 years ago

Let me know if this works.

	Justin Lebar (not reading bugmail)
	Comment 5 • 12 years ago

Comment on attachment 732067 [details]
dmseg logging

> <4>[04-01 21:37:31.369] [28: kswapd0]select 693 (Marketplace Dev), adj 6, size 8392, to kill\
> <4>[04-01 21:37:31.369] [28: kswapd0]send sigkill to 693 (Marketplace Dev), adj 6, size 8392\

There you go; the marketplace app is in the background ("adj 6"), and we're killing it.

I think Gaia probably controls this.

	Jason Smith [:jsmith]
	Comment 6 • 12 years ago

Now this is starting to sound like another identity.js bug in Gaia::System.

Jed - Any ideas?

	Justin Lebar (not reading bugmail)
	Comment 7 • 12 years ago

Sounds like we just shouldn't setVisible(false) on the relevant frame.

	Justin Lebar (not reading bugmail)
	Comment 8 • 12 years ago

...until we have LRU logic in the process-priority manager (working on it).

	Jed Parsons (use needinfo, please) [:jedp, :jparsons]
	Comment 9 • 12 years ago

(In reply to Jason Smith [:jsmith] from comment #6)
> Now this is starting to sound like another identity.js bug in Gaia::System.
> 
> Jed - Any ideas?

Hi, Jason: I think Justin has your answer: the marketplace app is in the background and so is more susceptible to being killed in an OOM situation.

(I had actually been wondering whether we would encounter precisely this OOM situation, with the os killing the process that spawned the process in the trusted ui.  I have been testing with this in mind, manually killing apps that call identity while the trusted ui is open, in order to ensure that things aren't left in some weird state if they crash or are crashed.)

I didn't know that LRU was in the works for the process-priority manager.  That's awesome news.

	Fernando Jiménez Moreno [:ferjm]
	Comment 10 • 12 years ago

(In reply to Justin Lebar [:jlebar] from comment #5)
> I think Gaia probably controls this.

It does since bug 834871 landed. We remove the trusted UI associated with a killed app.

(In reply to krupa raj 82[:krupa] from comment #0)
> actual behavior:
> After step #8, user returns to the marketplace page where they initiated the
> payment from (details or search results page) and then the homepage loads.

That first page that you mention sounds like the app screenshot that is shown during the app launch.

	ewong
	Comment 11 • 12 years ago

linking bug to github project issues: https://github.com/mozilla/browserid/issues/3191

	Lucas Adamski [:ladamski]
	Comment 12 • 12 years ago

I think this needs to be resolved either way.  Depending on how long it takes for the LRU work to land, the setVisible interim fix may or may not be worth the churn (since I'm guessing we'll want to back it out once LRU lands).

	Jed Parsons (use needinfo, please) [:jedp, :jparsons]
	Comment 13 • 12 years ago

(In reply to Lucas Adamski [:ladamski] (plz needinfo) from comment #12)
> I think this needs to be resolved either way.  Depending on how long it
> takes for the LRU work to land, the setVisible interim fix may or may not be
> worth the churn (since I'm guessing we'll want to back it out once LRU
> lands).

Trying setVisible as an interim fix sounds like a good plan to me.  I see you assigned it to me; I'll try to have a patch soon.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 14 • 12 years ago

Thanks Justin for c.c. me, after reading I think I could solve this but leave Jed to determine if he wants to keep this bug.

	Jed Parsons (use needinfo, please) [:jedp, :jparsons]
	Comment 15 • 12 years ago

(In reply to Alive Kuo [:alive] from comment #14)
> Thanks Justin for c.c. me, after reading I think I could solve this but
> leave Jed to determine if he wants to keep this bug.

Hi, Alive!  If you have a solution, please feel free to take the bug.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 16 • 12 years ago

Before the backend solution comes true, the only way I could figure out is maintaining a "I-CANNOT-BE-BROUT-TO-BACKGROUND" list of apps in somewhere of system and let window manager ignore setVisible(false) to that when it does app window transition(switch apps as bug description #0).

Yeah it'd be a dirty workaround.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 17 • 12 years ago

Justin, could you give me the bug # you mentioned in comment 8 so that I could add a comment about it? Thanks.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 18 • 12 years ago

After discussing w/ Tim, we are concerning to pollute Window Manager with such workaround may introduce some regression about visibility maintenance -- it now seems to be memory maintenance in gaia and this seems an edge case that, during trusted UI progress the user is supposed not to do app switch.

Before I start to do this workaround I'd like to know if this is necessary and really blocking.

	Justin Lebar (not reading bugmail)
	Comment 19 • 12 years ago

> Justin, could you give me the bug # you mentioned in comment 8 so that I could add a comment about 
> it?

bug 822325

We can probably have that in the leo timeframe, but personally I think you should probably do the workaround if it's relatively simple, and then we can back it out later.

	Wayne Chang [:wchang]
	Comment 20 • 12 years ago

Triage 4/10 - Leo+ this as it is related with payments, which users will be more concerned about if interrupted.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 21 • 12 years ago

(In reply to Justin Lebar [:jlebar] from comment #19)
> > Justin, could you give me the bug # you mentioned in comment 8 so that I could add a comment about 
> > it?
> 
> bug 822325
> 
> We can probably have that in the leo timeframe, but personally I think you
> should probably do the workaround if it's relatively simple, and then we can
> back it out later.

I'm marking this as blocking bug 822325 and do the workaround in gaia at first, thanks.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 22 • 12 years ago

WIP
https://github.com/mozilla-b2g/gaia/pull/9083

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 23 • 12 years ago

I am not able to test any identity function now :/

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 24 • 12 years ago

(In reply to Alive Kuo [:alive] from comment #23)
> I am not able to test any identity function now :/

Identify works fine in v1.0.1(From marketplace)
But it breaks (trustedUI doesn't show) in v1-train and master.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 25 • 12 years ago

I'd verified the patch on v1.0.1 by:

Every 1.0s: adb shell b2g-ps --oom                                                     Thu Apr 11 15:07:36 2013

APPLICATION       OOM_ADJ  OOM_SCORE  OOM_SCORE_ADJ  USER     PID   PPID  VSIZE  RSS     WCHAN    PC         NA
ME
b2g                  0        259         0          root      109   1     214136 54116 ffffffff 400ee330 S /sy
stem/b2g/b2g
Marketplace          2        241         134        app_434   434   109   89132  20072 ffffffff 40022594 R /sy
stem/b2g/plugin-container
Browser              2        200         134        app_474   474   109   62340  12424 ffffffff 40108330 S /sy
stem/b2g/plugin-container
Browser              2        237         134        app_487   487   109   68280  19260 ffffffff 400b5330 S /sy
stem/b2g/plugin-container
Homescreen           4        386         267        app_517   517   109   67604  22304 ffffffff 40099330 S /sy
stem/b2g/plugin-container
(Preallocated a      2        215         134        app_567   567   109   57156  15204 ffffffff 40068330 S /sy
stem/b2g/plugin-container
Gallery              2        254         134        app_2400  2400  109   85236  22400 ffffffff 4009e330 S /sy
stem/b2g/plugin-container
(Preallocated a      6        472         400        root      2669  109   57156  19124 ffffffff 4008e330 S /sy
stem/b2g/plugin-container

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 26 • 12 years ago

Comment on attachment 736159 [details]
https://github.com/mozilla-b2g/gaia/pull/9083

Hum, I don't understand why am I missing this.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 27 • 12 years ago

master
https://github.com/mozilla-b2g/gaia/commit/dec958af6e40d213bd9b4a404110567a21222b03

	John Ford [:jhford] CET/CEST Berlin Time
	Comment 28 • 12 years ago

Uplifted commit dec958af6e40d213bd9b4a404110567a21222b03 as:
v1-train: b5e7a068c976d1b62a37005b4577483d3ba9e080

	Jason Smith [:jsmith]
	Comment 29 • 12 years ago

Krupa - On the 4/16 build tomorrow, can you check to see if this patch fixes the issue you hit?

	krupa raj[:krupa] Reporter
	Comment 30 • 12 years ago

(In reply to Jason Smith [:jsmith] from comment #29)
> Krupa - On the 4/16 build tomorrow, can you check to see if this patch fixes
> the issue you hit?

I will be on PTO from 04/16-04/17. I can check on Thursday.