1035453 - [B2G][Camera] Pressing home does not function in the lockscreen camera with passcode lock enabled

Status: VERIFIED FIXED

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

Description

[Andrew Stole [:astole]]

Attachment: logcat

After taking a few pictures from the lockscreen with a passcode enabled, pressing the home button does not function. This issue does not occur if there is no passcode enabled and is easier to get to occur if multiple pictures are taken.

Repro Steps:
1) Update a Flame to BuildID: 20140707000200
2) Enable a passcode in the Settings app
3) Lock the screen
4) From the lockscreen, select the camera
5) Take multiple pictures
6) Press the home button

Actual:
Pressing the home button does not return the device to the lockscreen

Expected:
Pressing the home button returns the device to the lockscreen

2.0 Environmental Variables:
Device: Flame 2.0
BuildID: 20140707000200
Gaia: ef67af27dff3130d41a9139d6ae7ed640c34d922
Gecko: f53099796238
Version: 32.0a2
Firmware Version: v122

Repro frequency: 100%
See attached: logcat

Comment 1

[Jason Smith [:jsmith]]

This is missing a QAnalyst-Triage? flag

Comment 2

[Peter Bylenga [:PBylenga]]

Adding qawanted to do the branch checks with a flame device with memory set to 273mb.

Comment 3

[Cody Roesch [:croesch]]

This bug repro's on: Flame 2.1 Master, Flame 2.0

Actual Results: With Mem set to 273, taking a few pictures on the lockscreen camera with passcode enabled, then pressing the home button does not allow the user to leave the camera app.

Environmental Variables:
Device: Flame Master
Build ID: 20140708061447
Gaia: 740faa5d0060fb218b407cf224330654ddf833a5
Gecko: 8bfe3372f848
Version: 33.0a1 (Master)
Firmware Version: v122
-----------------------------------------------
Environmental Variables:
Device: Flame 2.0
Build ID: 20140708071430
Gaia: 444efc47370736a7bc57cc72a1ec00f3cc5f7e92
Gecko: c3683c06f6d8
Version: 32.0a2 (2.0)
Firmware Version: v122

-----------------------------------------------
-----------------------------------------------

This bug does NOT repro on: Flame 1.4

Actual Result: At 273mb, the user can still exit the lockscreen camera with the passcode on.

Environmental Variables:
Device: Flame 1.4
Build ID: 20140708070432
Gaia: 229864ff4ad90899f017341b9e81ed0b53498c66
Gecko: 7e146a1d7542
Version: 30.0 (1.4)
Firmware Version: v122

Comment 4

[Mike Habicher [:mikeh] (high bugzilla latency)]

Confirmed on:
Gonk      v122
Gaia      f71bf6ee534275a44ae3e475e9e2a5dfeebbc0ae
Gecko     https://hg.mozilla.org/integration/b2g-inbound/rev/e2402e271fb6
BuildID   20140709032231
Version   33.0a1
ro.build.version.incremental=108
ro.build.date=Tue Jun 10 19:40:40 CST 2014

Editing the summary due to the following STR:
0. enable lockscreen with passcode, and press power to lock the phone
1. press power to unlock the phone and tap on the camera icon
2. press the Home button

With the above build, I feel the vibrator motor twitch when I press the Home button, but I can't leave either the Camera or Preview screens.

Comment 5

[Hema Koka [:hema]]

camera app does not do anything with the home button. 

Tim/Alive, we need your team's help to see if this is a home button/lockscreen issue?

Comment 6

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

B2g-inbound Regression Window

Last working 
Environmental Variables:
Device: Flame Master
BuildID: 20140703005653
Gaia: 634558f376da67281508c5fcaa91a51c447ad49a
Gecko: 7b49b12aab6a
Version: 33.0a1 (Master) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0



First Broken 
Environmental Variables:
Device: Flame Master
BuildID: 20140703011807
Gaia: 0f27ffb69182a2483a4d0028e3dde50c608fb9d0
Gecko: 2e0c015d861b
Version: 33.0a1 (Master) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0




Last working gaia / First broken gecko - Issue does NOT occur
Gaia: 634558f376da67281508c5fcaa91a51c447ad49a
Gecko: 2e0c015d861b

First broken gaia / Last working gecko - Issue DOES occur
Gaia: 0f27ffb69182a2483a4d0028e3dde50c608fb9d0
Gecko: 7b49b12aab6a

Gaia Pushlog:  https://github.com/mozilla-b2g/gaia/compare/634558f376da67281508c5fcaa91a51c447ad49a...0f27ffb69182a2483a4d0028e3dde50c608fb9d0

Both Bug 1018283 and Bug 1020917 seem likely causes for this issue.

Comment 7

[Joshua Mitchell (Inactive)]

likely broken by bug 1020917, Greg can you take a look?

Comment 8

[Jason Smith [:jsmith]]

(In reply to Joshua Mitchell (Joshua_M) from comment #7)
> likely broken by bug 1020917, Greg can you take a look?

When a single regressing cause is suspected, make sure that bug is linked in the "blocks" field of a bug.

Comment 9

[Joshua Mitchell (Inactive)]

(In reply to Jason Smith [:jsmith] from comment #8)
> (In reply to Joshua Mitchell (Joshua_M) from comment #7)
> > likely broken by bug 1020917, Greg can you take a look?
> 
> When a single regressing cause is suspected, make sure that bug is linked in
> the "blocks" field of a bug.

Noted, however, in this case there were 2 bugs, both of which dealt with lockscreen; The window-reporter listed both and I picked the one I felt was more likely, but was not 100% on my suspicion.

Comment 10

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

Greg, SecureWindowManager should kill the active SecureWindow when press home.
I think the proper solution is let SecureWindowManager catch home event before LockscreenWindowManager.

Comment 11

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

George will help.

Comment 12

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

Comment 13

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8453609 [details] [review]
PR to master

Hi Alive,
could you help me to review it?
We'll not unregister home event when suspendEvents in this commit.

Comment 14

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

Comment on attachment 8453609 [details] [review]
PR to master

See github

Comment 15

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8453609 [details] [review]
PR to master

Hi Greg,

the bug is due to secure_window_manager cannot get home event and do kill app, so I add a home eventListener. However, based on current design, secure_window_manager will unregister all events after getting secure-modeoff, and reregister it when secure-modeon. Then the home event will be occupied by lockscreen. In case of that, I think we don't need to unregister home event in secure_window.

I suggest to remove resume and suspend methods. Please let me know your concern. Thanks.

Comment 16

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8453609 [details] [review]
PR to master

I don't know why you removed all mode-on and mode-off code, rather than just treat the home event as a special case. If there is a event handler register, to remove it when your class instance got suspend is a basic principle for me. Please give the detail reason for to remove the whole code if you have to do that, thanks.

Comment 17

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Alive,

As offline discussed with Greg, there should be no difference to keep or remove suspend events, however, like the reason in comment 16, we should remain event suspender as a basic principle. 

My latest commit has removed all the modeon and modeoff related code. May I have your decision for proper solution  to fix this bug?

Thanks.

Comment 18

[Greg Weng [:snowmantw][:gweng][:λ]]

Well, as I said in the offline discussion, since Alive is the major System maintainer, his opinion and principle is with higher priority than mine. So to follow the conclusion you have discussed is fine. What I addressed is the discussion wasn't on Bugzilla, so there is no obvious reason why the code need to be removed, which as a reviewer I ought to know, or the reviewing would be blocked.

Comment 19

[(Inactive after June) George Duan [:gduan] [:喬智]]

set ni to alive.

Comment 20

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

One of my goal is make window-managers could be inherited by some base-window-manager class so I wanna to reduce the difference. If removing the on/off part doesn't harm then let's go.

Comment 21

[(Inactive after June) George Duan [:gduan] [:喬智]]

Thanks,
merge to master
https://github.com/mozilla-b2g/gaia/commit/ff14c22cf4d1f13903df720b262d917d596ddece

Comment 22

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/c07e2d88ec49691f92c3ad552f1c2b3a6098c76e

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Reverted for Gaia unit test failures.
v2.0: https://github.com/mozilla-b2g/gaia/commit/62fc6e67d31e25809973cefeb3dddb094139945f

https://tbpl.mozilla.org/php/getParsedLog.php?id=43842282&tree=Mozilla-Aurora

Comment 24

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to v2.0

Comment 25

[(Inactive after June) George Duan [:gduan] [:喬智]]

Merge to v2.0
https://github.com/mozilla-b2g/gaia/commit/b60d9535b0d68d59f40b40e8daa5d4b24cfaf740

Comment 26

[Shine(Leave from Mozilla)]

Attachment: video

This issue has been verified successfully on Flame 2.0 and 2.1 (we have verified the two case that set Mem to 273 MB and 319 MB）
See attachment: Verify_1035453.MP4
Reproducing rate: 0/5

Flame2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/ff1100ba2ab8
Build-ID        20141204000228
Version         32.0

Flame 2.1 build:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ebce587d2194
Build-ID        20141203001205
Version         34.0