996514 - [Dolphin][tarako] two "<div id="homescreen"...></div> in System app after running monkey test

Status: RESOLVED FIXED

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

Description

[Peipei Cheng (needinfo if you need my action)]

Spreadtrum still see this issue in recent build.


+++ This bug was initially created as a clone of Bug #990365 +++

see bug 988110 comment 28. After running monkey test, there are two "<div id="homescreen"...></div>". The second one is empty, and is on top of the first one.

Comment 1

[Peipei Cheng (needinfo if you need my action)]

Attachment: log of two div after monkey test

Comment 2

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

is this a bug of Bug 995886? thanks

Comment 3

[Tzu-Lin Huang [:dwi2][:tzhuang]]

No, they are different

Comment 4

[Tzu-Lin Huang [:dwi2][:tzhuang]]

From the main.log in https://bugzilla.mozilla.org/attachment.cgi?id=8406572 in bug 990365

I can't see anything weird in the log from gaia side.
But I saw that homescreen app get killed very frequently at foreground. I think we need people familiar with low memory killer to look at it.

04-15 05:35:23.870 19179 19179 I log     : <4>0[24967.112038] lowmem_shrink select 19139 (Homescreen), adj 2, size 6279, to kill 
04-15 05:35:42.040 19250 19250 I log     : <4>0[24985.196984] lowmem_shrink select 19215 (Homescreen), adj 2, size 5791, to kill 
04-15 05:36:50.000 19422 19422 I log     : <4>0[25053.224021] lowmem_shrink select 19405 (Homescreen), adj 2, size 5876, to kill 
04-15 05:37:36.120 19540 19540 I log     : <4>0[25099.300546] lowmem_shrink select 19500 (Homescreen), adj 2, size 5240, to kill 
04-15 05:37:43.920 19560 19560 I log     : <4>0[25107.160745] lowmem_shrink select 19529 (Homescreen), adj 2, size 4932, to kill 
04-15 05:37:50.380 19579 19579 I log     : <4>0[25113.573517] lowmem_shrink select 19550 (Homescreen), adj 2, size 5760, to kill 
04-15 05:37:59.210 19624 19624 I log     : <4>0[25122.416767] lowmem_shrink select 19607 (Homescreen), adj 2, size 5716, to kill 
04-15 05:38:04.750 19642 19642 I log     : <4>0[25127.968079] lowmem_shrink select 19627 (Homescreen), adj 2, size 5842, to kill 
04-15 05:38:07.550 19657 19657 I log     : <4>0[25130.800457] lowmem_shrink select 19643 (Homescreen), adj 2, size 5411, to kill 
04-15 05:38:31.970 19783 19783 I log     : <4>0[25155.079761] lowmem_shrink select 19766 (Homescreen), adj 2, size 5493, to kill 
04-15 05:38:37.940 19828 19828 I log     : <4>0[25161.088031] lowmem_shrink select 19784 (Homescreen), adj 2, size 6132, to kill 
04-15 05:38:46.250 19853 19853 I log     : <4>0[25169.486317] lowmem_shrink select 19820 (Homescreen), adj 2, size 5226, to kill 
04-15 05:39:01.590 19890 19890 I log     : <4>0[25184.823333] lowmem_shrink select 19862 (Homescreen), adj 2, size 5274, to kill 
04-15 05:39:25.530 20013 20013 I log     : <4>0[25208.710725] lowmem_shrink select 19971 (Homescreen), adj 2, size 5640, to kill 
04-15 05:39:31.360 20042 20042 I log     : <4>0[25214.576166] lowmem_shrink select 20005 (Homescreen), adj 2, size 5859, to kill 
04-15 05:39:35.460 20079 20079 I log     : <4>0[25218.701202] lowmem_shrink select 20043 (Homescreen), adj 2, size 6141, to kill 
04-15 05:39:46.060 20108 20108 I log     : <4>0[25229.069911] lowmem_shrink select 20080 (Homescreen), adj 2, size 5148, to kill 
04-15 05:39:57.760 20137 20137 I log     : <4>0[25240.329566] lowmem_shrink select 20124 (Homescreen), adj 2, size 5905, to kill 
04-15 05:40:01.630 20167 20167 I log     : <4>0[25244.868283] lowmem_shrink select 20140 (Homescreen), adj 2, size 4753, to kill 
04-15 05:40:17.820 20240 20240 I log     : <4>0[25261.058033] lowmem_shrink select 20206 (Homescreen), adj 2, size 6090, to kill 
04-15 05:40:38.710 20345 20345 I log     : <4>0[25281.964554] lowmem_shrink select 20311 (Homescreen), adj 2, size 6071, to kill 
04-15 05:40:41.500 20359 20359 I log     : <4>0[25284.726364] lowmem_shrink select 20346 (Homescreen), adj 2, size 5281, to kill

Comment 5

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I think there's a possibility that it starts mishandling code at a certain low memory point and this is why we may be seeing things like this.

Marking as OOM, memory, perf.

Comment 6

[James Zhang (Spreadtrum)]

It'll block our monkey test.

Comment 7

[bhavana bajaj [:bajaj]]

(In reply to James Zhang from comment #6)
> It'll block our monkey test.

How about do you hit this ? Have you been seeing this issue recently, on a recent build ?

Comment 8

[bhavana bajaj [:bajaj]]

(In reply to bhavana bajaj [:bajaj] from comment #7)
> (In reply to James Zhang from comment #6)
> > It'll block our monkey test.
> 
> How about do you hit this ? Have you been seeing this issue recently, on a
> recent build ?
whoops, I meant How often**

Comment 9

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

ni? styang to discuss with partner

Comment 11

[Jason Smith [:jsmith]]

Carrying over steps-wanted from the dupe.

This has been reproduced manually multiple times, but we haven't nailed down the exact STR to reproduce it.

Comment 12

[yang.zhao]

(In reply to Jason Smith [:jsmith] from comment #11)
> Carrying over steps-wanted from the dupe.
> 
> This has been reproduced manually multiple times, but we haven't nailed down
> the exact STR to reproduce it.

No,996514 is reproduced after running monkey test.995886 is reproduced manually multiple times.They are different,you could ask Tzu-Lin for details,and both of them don't have STR.

Comment 13

[Steven Yang [:styang]]

minus till we have clear STR.

Comment 14

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment 4 looks like possibly a different bug. Does that need to be split out?

Comment 15

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Also removing memory because the LMK issue in comment 4 isn't obviously related to the div issue reported here.

Comment 16

[thomas tsai]

Please upload the log to analyze, if this issue happens again.

Comment 17

[yang.zhao]

(In reply to thomas tsai from comment #16)
> Please upload the log to analyze, if this issue happens again.

Haven't happened for now.

Comment 18

[yang.zhao]

Attachment: Screenshot from 2014-05-03 10:23:00.png

app-manager screenshot

Comment 19

[yang.zhao]

Attachment: two_homescreen.zip

log

Comment 20

[James Zhang (Spreadtrum)]

Attachment: homescreen_window_bug.patch

add log and try to fix it.

Comment 21

[Evelyn Hung]

Comment on attachment 8416989 [details] [diff] [review]
homescreen_window_bug.patch

This looks like a workaround, we still don't figure out why there are two homescreen div. Per my understanding, we call this.kill() before entering this setTimeout, so |this.element| should not exists. So why is it still there? Can you explain your patch?

@Alive, does this patch give you any thought that people can try out?

Comment 22

[James Zhang (Spreadtrum)]

With this patch, run 7x12hours monkey, we don't meet this issue.

And we can find the log.
05-04 20:23:46.670   606   606 E GeckoConsole: Content JS LOG at app://system.gaiamobile.org/js/homescreen_window.js:141 in hw_restart/<: bug#293965 occurs.

Comment 23

[James Zhang (Spreadtrum)]

1.3t+, thanks.

Comment 24

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8416989 [details] [diff] [review]
homescreen_window_bug.patch

Looks good.

Comment 25

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Let me create a patch from that since we need to remove the console.log() line and preferably write an unit test.

Comment 26

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

My guess: HomescreenWindow is ensured before the timer in restart() executes so render() is called twice.

Comment 27

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

The better solution might be to check this.element existence in render().

Comment 28

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Alive Kuo [:alive][NEEDINFO!][OOO 4/30 - 5/4] from comment #26)
> My guess: HomescreenWindow is ensured before the timer in restart() executes
> so render() is called twice.

Yes. And after reading the code I realize the same check already exist in app_window.js _render() in master branch, so we don't need a patch in master.

We need to check v1.4 though.

Comment 29

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:v1.3t PR#18923

Comment 30

[Tim Guan-tin Chien [:timdream] (please needinfo)]

git blame showed the check was added in bug 907013, when app_window.js was created and window.js was removed.

Comment 31

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Not sure if this is the case, but what's the result of this?
Do two homescreen pages overlap each other? ( bug 995886 )?

Comment 32

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #31)
> Not sure if this is the case, but what's the result of this?
> Do two homescreen pages overlap each other? ( bug 995886 )?

If both frames are correctly launched and populated, yes, that might be a symptom of the same bug. Let's fix this first and see if we could dup that here.

Comment 33

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

Comment on attachment 8417162 [details] [review]
mozilla-b2g:v1.3t PR#18923

One another old window manager victim.

Comment 34

[Tim Guan-tin Chien [:timdream] (please needinfo)]

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

Comment 35

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Correction,

1.3t: https://github.com/mozilla-b2g/gaia/commit/22f4b9c49578767f99bb15f514dbdccec24aa12b

Comment 36

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #32)
> (In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from
> comment #31)
> > Not sure if this is the case, but what's the result of this?
> > Do two homescreen pages overlap each other? ( bug 995886 )?
> 
> If both frames are correctly launched and populated, yes, that might be a
> symptom of the same bug. Let's fix this first and see if we could dup that
> here.

According to partner, their WIP patch did not fix bug 995886.

Comment 37

[Julien Wajsberg [:julienw]]

(In reply to Alive Kuo [:alive][NEEDINFO!][OOO 4/30 - 5/4] from comment #33)
> Comment on attachment 8417162 [details] [review]
> mozilla-b2g:v1.3t PR#18923
> 
> One another old window manager victim.

Your PR was not green before merging: https://travis-ci.org/mozilla-b2g/gaia/builds/24424991
And it stayed red after merging too: https://travis-ci.org/mozilla-b2g/gaia/builds/24435070

So I'm reverting the v1.3t patch, sorry :(

v1.3t: 48372ff88c712468a77a6f14a97f9978ee56eade

Comment 38

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

It turns out that the patch caused bug 1005953 to occur.  Basically a huge delay in going to the homescreen after the FTU.  It turns out if you let the phone sleep and wake the phone it will show the homescreen in that bug.

Julien's backout was necessary to not see bug 1005953.  I asked for a new build from releng.

Comment 39

[Julien Wajsberg [:julienw]]

Note that I haven't reverted the master patch (and that's why I didn't reopen the bug in the first place, Joe). Naoki, do you think we should back out the master patch?

Comment 40

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Julien, not sure the answer to that question.  I haven't tried master as of yet...

Comment 41

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Julien Wajsberg [:julienw] (away until 5th May) from comment #39)
> Note that I haven't reverted the master patch (and that's why I didn't
> reopen the bug in the first place, Joe). Naoki, do you think we should back
> out the master patch?

These isn't a master patch. comment 34 was a typo.

I prepared this patch from feedback from our partner. If this patch need more work the partner have to take it instead.

Comment 42

[Tim Guan-tin Chien [:timdream] (please needinfo)]

After discussion with Kai-zen and Evelyn, I think we should land a patch similar to attachment 8416989 [details] [diff] [review].

Comment 43

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:v1.3t PR#18965

Comment 44

[Jesse]

(In reply to Alive Kuo [:alive][NEEDINFO!][OOO 4/30 - 5/4] from comment #27)
> The better solution might be to check this.element existence in render().

Attachment 8416989 [details] [diff] is really a fix, We may not want to call render or open function either if some other operations have called render and open functions already.

Comment 45

[James Zhang (Spreadtrum)]

Please use my patch which provided by Jesse.Ji, we have verified it by 7x24 hours monkey test.

Comment 46

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

Comment on attachment 8417781 [details] [review]
mozilla-b2g:v1.3t PR#18965

Could we have this in master as well?

Comment 47

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:master PR#18966

Comment 48

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8417836 [details] [review]
mozilla-b2g:master PR#18966

Patch to master.

Comment 49

[Tim Guan-tin Chien [:timdream] (please needinfo)]

1.3t: https://github.com/mozilla-b2g/gaia/commit/01b4910f2fb48b39686cab3e35166750a4d474ff

Comment 50

[Tim Guan-tin Chien [:timdream] (please needinfo)]

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

Comment 51

[Ivan Tsay (:ITsay)]

Hi Tim,

Is v1.4 affected by this bug? Thanks.

Comment 52

[Tim Guan-tin Chien [:timdream] (please needinfo)]

It "look like" we have never been able to reproduce this bug manually. So I would say no.

Comment 53

[Keven Kuo [:kkuo]]

Hi! Tim,

But this patch was in master already.

--
Keven

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #52)
> It "look like" we have never been able to reproduce this bug manually. So I
> would say no.

Comment 54

[Peipei Cheng (needinfo if you need my action)]

partner also want this fix in dolphin.

Comment 55

[Ivan Tsay (:ITsay)]

Hi Tim,

Can we uplift the master patch to v1.4 directly? Or do we need to create another v1.4 patch? This is partner blocker and we need it in v1.4 for Dolphin.

Comment 56

[Ivan Tsay (:ITsay)]

Comment on attachment 8417836 [details] [review]
mozilla-b2g:master PR#18966

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Found from partner's test
[User impact] if declined: Partner blocker on Dolphin
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: No

Comment 57

[Ivan Tsay (:ITsay)]

(In reply to Ivan Tsay (:ITsay) from comment #55)
> Hi Tim,
> 
> Can we uplift the master patch to v1.4 directly? Or do we need to create
> another v1.4 patch? This is partner blocker and we need it in v1.4 for
> Dolphin.

Per discussion with Tim, let's try to uplift the master patch to v1.4.

Comment 58

[Jason Smith [:jsmith]]

Comment on attachment 8417836 [details] [review]
mozilla-b2g:master PR#18966

A patch that causes a smoketest regression once & gets backed out is not low risk. Minusing this as this is not safe to take into 1.4 at this point.

Comment 59

[Ivan Tsay (:ITsay)]

Hi Tim,

please help to take a look if the patch needs to be rebase to fit v1.4. Thanks.

Comment 60

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:v1.4 PR#20050

Comment 61

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8434670 [details] [review]
mozilla-b2g:v1.4 PR#20050

Rebased, Ivan, could you verify this is what the partner want?

Comment 62

[Jason Smith [:jsmith]]

Comment on attachment 8434670 [details] [review]
mozilla-b2g:v1.4 PR#20050

Please see comment 58.

Comment 63

[Ivan Tsay (:ITsay)]

Let's try this re-based patch in local v1.4 with Flame to ensure the patch won't cause any smoke test regression. Partner needs this patch to prepare their next v1.4 build for the test.

ni? Al Tsai for the help.

Comment 64

[Al Tsai [:atsai]]

Tested with PVT build with the PR#20050.

All green in smoke test. I also played around for a while and didn't encounter any blocker. It should be low risk to merge the build.

=== PVT Base image ===
Gaia      1f238df7ac68a73a4ceb27391a204c800f38ab1c
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/a7b7f1b579cc
BuildID   20140604160203
Version   30.0
ro.build.version.incremental=96
ro.build.date=Fri May 23 11:17:40 CST 2014

Comment 65

[Ivan Tsay (:ITsay)]

Comment on attachment 8434670 [details] [review]
mozilla-b2g:v1.4 PR#20050

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Found from partner's test
[User impact] if declined: two homescreen appears, ugly UI. it is also the partner blocker on Dolphin
[Testing completed]: Yes (verified v1.4 with the patch on Flame)
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: No

Based on comment 64, there is low risk for merging this patch

Comment 66

[Jan Jongboom [:janjongboom] (inactive)]

Hi Ivan, this was approval-gaia-v1.4?'ed a week ago. When can we see movement here? Partner request from this side :-)

Comment 67

[James Zhang (Spreadtrum)]

We meet the same issue on dolphin v1.4, please land this patch and we will use monkey test to verify.

Comment 68

[Ivan Tsay (:ITsay)]

Hi Preeti,

Please help to approve this request so that we can move on to uplift the patch to v1.4. Thanks!

Comment 70

[Preeti Raghunath(:Preeti)]

Comment on attachment 8434670 [details] [review]
mozilla-b2g:v1.4 PR#20050

To take on Dolphin branch

Comment 71

[Wayne Chang [:wchang]]

Comment on attachment 8434670 [details] [review]
mozilla-b2g:v1.4 PR#20050

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 72

[Preeti Raghunath(:Preeti)]

Wayne,

Please fill in the form.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 73

[Wayne Chang [:wchang]]

oops here you go (from comment 65) :

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Found from partner's test
[User impact] if declined: two homescreen appears, ugly UI. it is also the partner blocker on Dolphin
[Testing completed]: Yes (verified v1.4 with the patch on Flame)
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: No

Thanks!

Comment 74

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/185b195b0965e6b764948d548d38bf7677f291e8