992379 - [B2G][Tarako][Everything.me] The user is unable to add a 'custom' smart collection to the home screen if it was been killed once.

Status: RESOLVED FIXED

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

Description

[demerick]

Attachment: tarako_smart_logcat.txt

Description:
If the user attempts to add a 'custom' smart collection to the home screen nothing will happen.

Repro Steps:
1) Update a Tarako to BuildID: 20140404004001
2) Long press on the home screen where this is no icon
3) Tap 'Add Smart Collections'
4) Tap 'Custom...'

Actual:
The user is brought back to the home screen

Expected:
The user should be able to add a custom smart collection

1.3T Environmental Variables:
Device: Tarako 1.3T MOZ
BuildID: 20140404004001
Gaia: acd18bbd94ebfa534e252a24a75a0617e4b5d5ae
Gecko: 91a1b54da4a6
Version: 28.1
Firmware Version: sp8810

Repro frequency: 100%
See attached: logcat.txt
Video URL: https://www.youtube.com/watch?v=JXRJMlUAQXc&

Comment 1

[demerick]

This issue does not occur on the Buri 1.3 MOZ RIL

1.3 Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140401004003
Gaia: 24f562fce468fc948ac9e6185e002c23350cb9ee
Gecko: 0adf24a785f2
Version: 28.0
Firmware Version: v1.2-device.cfg

The user is able to add a 'custom' smart collection to the home screen.

Comment 2

[Joe Cheng [:jcheng] (please needinfo)]

1.3T+ for broken feature
ni? Gregor

Comment 4

[Gregor Wagner [:gwagner]]

Ran, Amir, any idea whats going on here?

Comment 5

[Ran Ben Aharon [:ranbena] EverythingMe]

The log doesn't show any e.me javascript errors. Unfortunately, we don't have the Tarako device yet so testing is a problem. Amir, can you list possible causes?

Comment 6

[Amir Nissim (:amirn)]

Possibly a duplicate of bug 963673

can anyone confirm that BuildID: 20140404004001 includes commit:
https://github.com/mozilla-b2g/gaia/commit/a15974a80d6b93a23054caaa5c8641c5e317db51

Comment 7

[Jason Smith [:jsmith]]

(In reply to Amir Nissim (Everything.me) from comment #6)
> Possibly a duplicate of bug 963673
> 
> can anyone confirm that BuildID: 20140404004001 includes commit:
> https://github.com/mozilla-b2g/gaia/commit/
> a15974a80d6b93a23054caaa5c8641c5e317db51

It's in the build:

http://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/bb425f8e2361

Comment 8

[Amir Nissim (:amirn)]

Thanks Jason.

I still suspect it is a duplicate since the STR and attached videos are exactly the same in both bugs.

Perhaps Cristian (who fixed bug 963673) will have more information.

Comment 9

[Jason Smith [:jsmith]]

(In reply to Amir Nissim (Everything.me) from comment #8)
> Thanks Jason.
> 
> I still suspect it is a duplicate since the STR and attached videos are
> exactly the same in both bugs.
> 
> Perhaps Cristian (who fixed bug 963673) will have more information.

I don't think that's true - if that was, then the patch there would have fixed the problem on 1.3T. So there's likely a contextual additional fix here needed on 1.3T. If that's the case, I'd rather keep a separate bug open for this & not dupe.

Comment 10

[Cristian Rodriguez (:crdlc)]

Please Amir, could you test if the flow reaches our window.confirm in ev.me code? If the answer is "yes", we have to move it directly to system component

(In reply to Amir Nissim (Everything.me) from comment #8)
> Thanks Jason.
> 
> I still suspect it is a duplicate since the STR and attached videos are
> exactly the same in both bugs.
> 
> Perhaps Cristian (who fixed bug 963673) will have more information.

Comment 11

[Amir Nissim (:amirn)]

(In reply to Cristian Rodriguez (:crdlc) from comment #10)
> Please Amir, could you test if the flow reaches our window.confirm in ev.me
> code? If the answer is "yes", we have to move it directly to system component
> 
> (In reply to Amir Nissim (Everything.me) from comment #8)
> > Thanks Jason.
> > 
> > I still suspect it is a duplicate since the STR and attached videos are
> > exactly the same in both bugs.
> > 
> > Perhaps Cristian (who fixed bug 963673) will have more information.

Unfortunately we don't have any Tarako devices so I can't test it on device. Do you have one Cristian?

Comment 12

[Cristian Rodriguez (:crdlc)]

jeje me neither. Jason, we would appreciate a lot if someone will take a look at this issue? Amir and me don't have a Tarako device so we cannot reproduce it in our environments.

I think that the first thing to test here is:

"Is the window.confirm performed from ev.me dispatched by system?"

Comment 13

[Joe Cheng [:jcheng] (please needinfo)]

ni? ehung

Comment 14

[Jan Jongboom [:janjongboom] (inactive)]

FYI, Tried on v1.3t gaia on a Keon, and doesn't repro there.

Comment 15

[Jason Smith [:jsmith]]

Gregor - Is there someone on your team who can investigate this that has a Tarako device?

Comment 16

[Gregor Wagner [:gwagner]]

(In reply to Jason Smith [:jsmith] from comment #15)
> Gregor - Is there someone on your team who can investigate this that has a
> Tarako device?

We still have to distribute the phones to the right remote people. It will take a few days.

Comment 17

[Evelyn Hung]

(In reply to Cristian Rodriguez (:crdlc) from comment #12)
> jeje me neither. Jason, we would appreciate a lot if someone will take a
> look at this issue? Amir and me don't have a Tarako device so we cannot
> reproduce it in our environments.
> 
> I think that the first thing to test here is:
> 
> "Is the window.confirm performed from ev.me dispatched by system?"

Were you saying this window.confirm ?
https://github.com/mozilla-b2g/gaia/blob/v1.3t/apps/homescreen/everything.me/js/Brain.js#L736

I tested on a normal build without the bug here, didn't see this window.confirm is executed. So I feel I'm tracing the wrong code.

Comment 18

[Amir Nissim (:amirn)]

It's the window.prompt here:
https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/everything.me/modules/CollectionsSuggest/CollectionsSuggest.js#L54

Comment 19

[Evelyn Hung]

(In reply to Amir Nissim (Everything.me) from comment #18)
> It's the window.prompt here:
> https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/everything.
> me/modules/CollectionsSuggest/CollectionsSuggest.js#L54

I sometimes can't reproduce this issue especially when I flash a new homescreen version to the device. Will try on the latest build.

BTW, at least I'm sure that homescreen app doesn't get killed because I saw its process is always there with the same PID.

Comment 20

[[:fabrice] Fabrice Desré]

We fail in BrowserElementPromptService.jsm : prompt() is called, but not this._browserElementChild.showModalPrompt() so I suspect that _browserElementChild is dead in some way. Of course I can't repro anymore since I added more debug info...

Comment 21

[Evelyn Hung]

(In reply to Fabrice Desré [:fabrice] from comment #20)
> We fail in BrowserElementPromptService.jsm : prompt() is called, but not
> this._browserElementChild.showModalPrompt() so I suspect that
> _browserElementChild is dead in some way. Of course I can't repro anymore
> since I added more debug info...

Yes, I confirm that System app didn't receive "mozbrowsershowmodalprompt" event. I found there is a way to easier reproduce it - After flash your build, try to make it in memory pressure so homescreen might be killed once. I usually open Settings -> Email -> Camera, then I can heavy enough. So maybe it's related to homescreen has been killed once?

Comment 22

[Evelyn Hung]

(In reply to Evelyn Hung [:evelyn] from comment #21)
> (In reply to Fabrice Desré [:fabrice] from comment #20)
> > We fail in BrowserElementPromptService.jsm : prompt() is called, but not
> > this._browserElementChild.showModalPrompt() so I suspect that
> > _browserElementChild is dead in some way. Of course I can't repro anymore
> > since I added more debug info...
> 
> Yes, I confirm that System app didn't receive "mozbrowsershowmodalprompt"
> event. I found there is a way to easier reproduce it - After flash your
> build, try to make it in memory pressure so homescreen might be killed once.
> I usually open Settings -> Email -> Camera, then I can heavy enough. So
> maybe it's related to homescreen has been killed once?

Confirm that if homescreen is killed and relaunched, the issue happens.

Comment 23

[Amir Nissim (:amirn)]

Thanks Evelyn.

Is it possible the bug applies to other devices as well but hasn't been discovered as they have more memory? (homecreen is less likely to be killed)

I tried the following STR on an unuagi but could not reproduce:
1. adb shell
2. kill <homescreen PID>
3. long tap > add collections > custom > prompt appears.

Comment 24

[Evelyn Hung]

(In reply to Amir Nissim (Everything.me) from comment #23)
> Thanks Evelyn.
> 
> Is it possible the bug applies to other devices as well but hasn't been
> discovered as they have more memory? (homecreen is less likely to be killed)
> 
> I tried the following STR on an unuagi but could not reproduce:
> 1. adb shell
> 2. kill <homescreen PID>
> 3. long tap > add collections > custom > prompt appears.

Maybe, but I didn't try other devices nor other branches. Not sure which branch you were using, but the issue happens on v1.3t, which did a lot of optimization on both gecko and gaia for running on a low memory devices (only 128M). Maybe you can build a 1.3t build for your unagi. :)

Comment 25

[Joe Cheng [:jcheng] (please needinfo)]

ni? Fabrice, not sure if you will be able to take this bug? Thanks

Comment 26

[[:fabrice] Fabrice Desré]

Yep, taking as I can now reproduce reliably.

Comment 27

[[:fabrice] Fabrice Desré]

Attachment: restart-homescreen.patch

And the winner is... the system app! We were creating the AppModalDialog that attaches the mozbrowserprompt event only when creating the HomescreenWindow, but not when restarting after a crash.

Comment 28

[[:fabrice] Fabrice Desré]

One last thing: I haven't checked if this bug also exists in gaia master.

Comment 29

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

Comment on attachment 8404390 [details] [diff] [review]
restart-homescreen.patch

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

Ya, this is what master have right now(install sub component in render function). Thanks tarako to verify it!
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L539

Comment 30

[[:fabrice] Fabrice Desré]

https://github.com/mozilla-b2g/gaia/commit/1e558f8af0016e5348f545543c90aa0b5e5d1f11

Comment 32

[Montoya Clemmons (MClemmons)]

Following STR from Comment 0, user is able to add a Custom Smart Collection on latest 1.3T build. 

Environmental Variables:
Device: Tarako 1.3T MOZ
BuildID: 20140428014001
Gaia: 8895b180ed636069473703d0e7b73086989601ce
Gecko: 7caf4b5abfce
Version: 28.1
Firmware Version: sp6821a_gonk4.0_user.pac