937887 - App purchase loads a blank screen in the trusted UI due to a JS error

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[krupa raj[:krupa]]

Attachment: logs

gaia 1.3
build identifier: 20131112040207

Steps to reproduce:
1. Start an app purchase on marketplace prod
2. Trusted UI launches

expected behavior:
Trusted UI launches and loads the Persona Sign in screen loads

observed behavior:
Trusted UI launches and loads a blank screen. See attached screenshot

11-12 23:44:38.519 E/GeckoConsole(  108): [JavaScript Error: "frame is undefined" {file: "jar:file:///system/b2g/omni.ja!/components/PaymentGlue.js" line: 130}]

Comment 1

[krupa raj[:krupa]]

Attachment: screenshot

Comment 2

[krupa raj[:krupa]]

app name: Pyrolee costing zl 3.99 with region = Poland

Comment 3

[Sarah Parsons]

This issue started to occur on the Buri 1.2 Build ID: 20130904040205

Gaia   b6c5bf1d24230bfed4a8f680e625fa2175001f82
SourceStamp 7ff96bd19c1c
BuildID 20130904040205
Version 26.0a1


Last working Buri 1.2 Build ID: 20130903040204

Gaia   c6d58088f207829d96e7bb33ddacf990e90752fb
SourceStamp d54e0cce6c17
BuildID 20130903040204
Version 26.0a1

Comment 4

[Jason Smith [:jsmith]]

Moving to koi? as this affects 1.2.

Comment 5

[Preeti Raghunath(:Preeti)]

Block for regression and screen shot looks bad. 

Andrew,

Please review as it looks like a blocker.

Comment 6

[Andrew Overholt [:overholt]]

(In reply to Sarah Parsons from comment #3)
> This issue started to occur on the Buri 1.2 Build ID: 20130904040205
> 
> Gaia   b6c5bf1d24230bfed4a8f680e625fa2175001f82
> SourceStamp 7ff96bd19c1c
> BuildID 20130904040205
> Version 26.0a1
> 
> 
> Last working Buri 1.2 Build ID: 20130903040204
> 
> Gaia   c6d58088f207829d96e7bb33ddacf990e90752fb
> SourceStamp d54e0cce6c17
> BuildID 20130903040204
> Version 26.0a1

Thanks for getting this to something between these builds.  Can we do further bisection to figure out what commit caused the regression?  I'd start with gaia bisection since those builds are much faster.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Andrew Overholt [:overholt] from comment #6)
> (In reply to Sarah Parsons from comment #3)
> > This issue started to occur on the Buri 1.2 Build ID: 20130904040205
> > 
> > Gaia   b6c5bf1d24230bfed4a8f680e625fa2175001f82
> > SourceStamp 7ff96bd19c1c
> > BuildID 20130904040205
> > Version 26.0a1
> > 
> > 
> > Last working Buri 1.2 Build ID: 20130903040204
> > 
> > Gaia   c6d58088f207829d96e7bb33ddacf990e90752fb
> > SourceStamp d54e0cce6c17
> > BuildID 20130903040204
> > Version 26.0a1
> 
> Thanks for getting this to something between these builds.  Can we do
> further bisection to figure out what commit caused the regression?  I'd
> start with gaia bisection since those builds are much faster.

It's likely not a gaia regression - the JS error being fired here is within gecko.

Comment 8

[Andrew Overholt [:overholt]]

(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Andrew Overholt [:overholt] from comment #6)
> > (In reply to Sarah Parsons from comment #3)
> > > This issue started to occur on the Buri 1.2 Build ID: 20130904040205
> > > 
> > > Gaia   b6c5bf1d24230bfed4a8f680e625fa2175001f82
> > > SourceStamp 7ff96bd19c1c
> > > BuildID 20130904040205
> > > Version 26.0a1
> > > 
> > > 
> > > Last working Buri 1.2 Build ID: 20130903040204
> > > 
> > > Gaia   c6d58088f207829d96e7bb33ddacf990e90752fb
> > > SourceStamp d54e0cce6c17
> > > BuildID 20130903040204
> > > Version 26.0a1
> > 
> > Thanks for getting this to something between these builds.  Can we do
> > further bisection to figure out what commit caused the regression?  I'd
> > start with gaia bisection since those builds are much faster.
> 
> It's likely not a gaia regression - the JS error being fired here is within
> gecko.

True but it's an error in the JS, right?  'frame' is undefined?

Comment 9

[Jason Smith [:jsmith]]

Sarah - Can you add gecko commits to the regression range here?

Comment 10

[Andrew Overholt [:overholt]]

I assumed the SourceStamp hashes were gecko (and used them when I tried to eyeball an offending commit).

Comment 11

[Jason Smith [:jsmith]]

(In reply to Andrew Overholt [:overholt] from comment #8)
> > It's likely not a gaia regression - the JS error being fired here is within
> > gecko.
> 
> True but it's an error in the JS, right?  'frame' is undefined?

True - The code in question where the failure is happening hasn't seen changes since nav.pay initially landed.

Comment 12

[Sarah Parsons]

1.2 9/04 Gecko revision="427bdca920169434058eac667f8d01800eb1ca76"

(In reply to Jason Smith [:jsmith] from comment #9)
> Sarah - Can you add gecko commits to the regression range here?

Comment 13

[Jason Smith [:jsmith]]

(In reply to Sarah Parsons from comment #12)
> 1.2 9/04 Gecko revision="427bdca920169434058eac667f8d01800eb1ca76"
> 
> (In reply to Jason Smith [:jsmith] from comment #9)
> > Sarah - Can you add gecko commits to the regression range here?

That's not the right gecko commit. It should be far smaller than that in length.

Anyways, the source stamp is actually showing the regression range here, so we're good here:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d54e0cce6c17&tochange=7ff96bd19c1c

Comment 14

[Andrew Overholt [:overholt]]

Discussing this a bit with others, we could really use some integration tests here.  Kumar, do we have any?

Comment 15

[Jason Smith [:jsmith]]

(In reply to Andrew Overholt [:overholt] from comment #14)
> Discussing this a bit with others, we could really use some integration
> tests here.  Kumar, do we have any?

I believe we have some gaia ui tests covering this.

See https://github.com/mozilla/marketplace-tests-gaia/tree/master/marketplacetests/tests.

Comment 16

[Jason Smith [:jsmith]]

After doing some more digging, this looks more like a Gaia regression. There really isn't anything obvious that jumps out in the gecko commit range as the cause, but there's certainly some suspicious commits in the Gaia commit range involving the window manager.

https://github.com/mozilla-b2g/gaia/commit/1398204704f78d17f5dac814e311ef13d828ac84

https://github.com/mozilla-b2g/gaia/commit/c6d58088f207829d96e7bb33ddacf990e90752fb

I'm going to guess bug 909255 caused this potentially.

Alive - What do you think?

Comment 17

[Jason Smith [:jsmith]]

Also - I don't think any of us right now have the ability to do bisections effectively, so I don't see this happening right now. Naoki is looking into this problem, but I don't see us getting a bisection for this bug anytime soon.

I think the best path forward here for now is to have Alive look into this from the Gaia perspective first to isolate this to what caused this bug. Then, if he concludes it's gecko based, then let's re-evaluate the gecko commit range on what could have caused this.

Comment 18

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

(In reply to Jason Smith [:jsmith] from comment #16)
> After doing some more digging, this looks more like a Gaia regression. There
> really isn't anything obvious that jumps out in the gecko commit range as
> the cause, but there's certainly some suspicious commits in the Gaia commit
> range involving the window manager.
> 
> https://github.com/mozilla-b2g/gaia/commit/
> 1398204704f78d17f5dac814e311ef13d828ac84
> 
> https://github.com/mozilla-b2g/gaia/commit/
> c6d58088f207829d96e7bb33ddacf990e90752fb
> 
> I'm going to guess bug 909255 caused this potentially.
> 
> Alive - What do you think?

Looking.

I don't think bug 909255 has anything to do with this tough.
Bug 909255 is for all window disposition activity request, but trustedUI has nothing to do with that
--- unless the payment is in an app which is invoked by an window disposition activity not as a normal app from homescreen.

Also the "JS error" doesn't seem anything wrong to me from gaia.
The frame here--I guess it's trustedUI frame-- is provided from gecko via window.open and we just paste it the somewhere we'd like it be,
so unless we never pastes it, there's no chance it's undefined.

Comment 19

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

Guys,

I cannot reproduce with latest gaia/master + last week gecko(but I don't remember it's mc or pvtbuild).
I'm using Marktetplace + persona login.

Comment 20

[Jason Smith [:jsmith]]

(In reply to Alive Kuo [:alive][NEEDINFO] from comment #19)
> Guys,
> 
> I cannot reproduce with latest gaia/master + last week gecko(but I don't
> remember it's mc or pvtbuild).
> I'm using Marktetplace + persona login.

You need to try to install a paid app to trigger this bug. It's unrelated to the persona dialog.

See comment 2 for the app you need to try to buy to trigger this bug on marketplace.

Comment 21

[Kumar McMillan [:kumar]]

(In reply to Andrew Overholt [:overholt] from comment #14)
> Discussing this a bit with others, we could really use some integration
> tests here.  Kumar, do we have any?

I don't know what integrations we have for the Trusted UI. stephend, maybe you know? I agree we need tests for this kind of stuff.

Comment 22

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

(In reply to Jason Smith [:jsmith] from comment #20)
> (In reply to Alive Kuo [:alive][NEEDINFO] from comment #19)
> > Guys,
> > 
> > I cannot reproduce with latest gaia/master + last week gecko(but I don't
> > remember it's mc or pvtbuild).
> > I'm using Marktetplace + persona login.
> 
> You need to try to install a paid app to trigger this bug. It's unrelated to
> the persona dialog.
> 
> See comment 2 for the app you need to try to buy to trigger this bug on
> marketplace.

Before investigating again, this comment reflects the fact that it's payment only issue but not a common trustedUI issue.
BTW I cannot see the problem after clicking $0.99 button. Everything is fine.

Comment 23

[Preeti Raghunath(:Preeti)]

Stephend,

Can we please check if this issue is reproducible in-house? Based on comment 22, it is clear there is a question on what the real issue is.

Comment 24

[krupa raj[:krupa]]

I cannot reproduce this anymore on today's inari build.

Comment 25

[Jason Smith [:jsmith]]

Sounds like this is a works for me then.

Comment 26

[krupa raj[:krupa]]

(In reply to krupa raj[:krupa] from comment #24)
> I cannot reproduce this anymore on today's inari build.

NOTE: I used 1.3 inari build from 11/18.

Comment 27

[Jason Smith [:jsmith]]

Putting QA Wanted to confirm if this still reproduces on 1.2.

Comment 28

[Sarah Parsons]

No longer able to reproduce on Buri 1.2 Build ID: 20131118004001

Gaia   7a23f8c53ba97da9c63a7275b36d155b4526a639
SourceStamp 3e8b854a992e
BuildID 20131118004001
Version 26.0

Comment 29

[krupa raj[:krupa]]

Actually, I was just able to reproduce this on 1.3 inari. This time, i hit the error after logging into Persona.

Build identifier: 20131118040206

STR:
1. Start the purchase of IQFitFun on marketplace
2. Log in using Persona

expected behavior:
Webpay screen to enter PIN loads

actual behavior:
A blank marketplace screen loads

Comment 30

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

Lemme move this out of window mgmt component since I don't think it belongs.

Comment 31

[Jason Smith [:jsmith]]

This bug implies paid apps can fail, which needs to block. Resetting blocking flag.

Comment 32

[Preeti Raghunath(:Preeti)]

Fernando,

Can you please take a look as it impacts paid apps.

Comment 33

[Fernando Jiménez Moreno [:ferjm]]

I am not able to reproduce with latest m-i + gaia master.

Comment 34

[Jason Smith [:jsmith]]

I'm going to take a look into this.

Comment 35

[Fernando Jiménez Moreno [:ferjm]]

I was able to get the "E/GeckoConsole(  108): [JavaScript Error: "frame is undefined" {file: "jar:file:///system/b2g/omni.ja!/components/PaymentGlue.js" line: 130}]" error but doesn't seem to be related to this issue.

I am getting this error when manually closing the trusted UI. The reason is because we don't properly remove the event listener for the content event that should carry the frame created by the UI. I'll upload a patch for this in a few minutes.

Comment 36

[Jason Smith [:jsmith]]

Sounds like we have a path forward here, so I'll remove qawanted

Comment 37

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 38

[Fernando Jiménez Moreno [:ferjm]]

Can you try to reproduce the issue with the attached patch applied, please?

Also, in case that you are able reproduce it again, having a logcat with "dom.payment.debug" pref set to true would be helpful.

Thanks!

Comment 39

[Jason Smith [:jsmith]]

(In reply to Fernando Jiménez Moreno [:ferjm] (use needinfo instead of CC, please) from comment #38)
> Can you try to reproduce the issue with the attached patch applied, please?
> 
> Also, in case that you are able reproduce it again, having a logcat with
> "dom.payment.debug" pref set to true would be helpful.
> 
> Thanks!

That's going to be difficult, as we don't have try builds for Buri.

I'll investigate this more generally however.

Comment 40

[[:fabrice] Fabrice Desré]

Comment on attachment 8336133 [details] [diff] [review]
v1

Review of attachment 8336133 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/components/PaymentGlue.js
@@ +83,1 @@
>      });

don't we have a syntax error here with the extra |);| ?

Comment 41

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 42

[[:fabrice] Fabrice Desré]

Comment on attachment 8336395 [details] [diff] [review]
v1

Review of attachment 8336395 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/components/PaymentGlue.js
@@ +78,5 @@
>        } else if (msg.errorMsg) {
>          _error(msg.errorMsg);
>        }
>  
> +      content.removeEventListener("mozContentEvent", self._handleSelection);

don't you also want to do self._handleSelection = null; ? (same question for the other handlers).

@@ +79,5 @@
>          _error(msg.errorMsg);
>        }
>  
> +      content.removeEventListener("mozContentEvent", self._handleSelection);
> +    };

nit: bind(this) _handleSelection() and get rid of self.

Comment 43

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v2

Comment 44

[Fernando Jiménez Moreno [:ferjm]]

https://hg.mozilla.org/integration/b2g-inbound/rev/7ca74fb9f994

Comment 45

[Jason Smith [:jsmith]]

So I can't reproduce this bug on 1.2. I see the PIN screen come up fine on 1.2.

Comment 46

[Jason Smith [:jsmith]]

Can't reproduce on 1.3 either. Pulling nom cause I don't think what's being fixed here is a blocker.

Comment 47

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7ca74fb9f994