1169861 - [Aries][Lockscreen] Holding down the camera button at the lockscreen with a passcode enabled will launch camera below the lockscreen and display an error message.

Status: VERIFIED FIXED

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

Description

[Martin Shuman [:Marty]]

Attachment: logcat-lockscreen-camera.txt

Description:
If the user has a passcode enabled, when they hold down the camera button at the lockscreen, the camera will launch beneath the lockscreen, and the user will be presented with the error message "Camera Unavailable. The camera is in use by another

Repro Steps:
1) Update a Xperia Z3 Compact (B2G) to 20150528210322
2) Enable a passcode.
3) Lock the device and press the power button to bring up the lockscreen
4) Hold down the camera button for several seconds

Actual:
Camera launches underneath the lockscreen, and an error message is displayed.

Expected:
Either nothing happens, or the Camera app launches at the lockscreen, but the device remains locked.

Environmental Variables:
Device: Xperia Z3 Compact (B2G) 3.0 (Full Flash)
Build ID: 20150528210322
Gaia: 18f7c340f970991a10c288310bbfd4d105a1430c
Gecko: b7aed25b94251ea192021885323f15e87fc39321
Version: 41.0a1 (3.0)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Repro frequency: 8/8
See attached: Video (URL), Logcat

Comment 1

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
broken functionality of camera button.

Comment 2

[Doug Sherk (:drs) (inactive)]

Greg, could you take a look at this please?

Comment 3

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

I don't know how you launch the first camera: your finger didn't touch anything on the screen, and the Camera just opened upon LockScreen (at 0:07 of the video). This is not in any design of original and current LockScreen, at least not in the specs I've touched. I can only guess "someone", apparently not included me, implemented the function that looks like be able to launch camera via the new hardware button and via a method I completely have no clue. So I think the best way is to find out whom is the author and NI the one. In this case, I'm also without any knowledge to provide.

Comment 4

[Doug Sherk (:drs) (inactive)]

Wilson, could you take a look at this?

Comment 5

[:gerard-majax]

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #3)
> I don't know how you launch the first camera: your finger didn't touch
> anything on the screen, and the Camera just opened upon LockScreen (at 0:07
> of the video). This is not in any design of original and current LockScreen,
> at least not in the specs I've touched. I can only guess "someone",
> apparently not included me, implemented the function that looks like be able
> to launch camera via the new hardware button and via a method I completely
> have no clue. So I think the best way is to find out whom is the author and
> NI the one. In this case, I'm also without any knowledge to provide.

Isn't it your r+ on bug 1133742 ?

Comment 6

[Wilson Page [:wilsonpage]]

I'm going to defer this to System app. This feature was originally landed as part of bug 1133742.

As a guess I would say the Camera app is being launched twice, the second app instance cannot access the hardware as the first is occupying it.

Comment 7

[Candice Serran (:cserran)]

Rather than just handing this to someone else and delay fixing this. I'd like us to try to come to a solution. And if that means getting multiple people talking simultaneous about this, then please make this happen. This is how bugs stay open for longer periods of time than necessary.

Comment 8

[Johan Lorenzo [:jlorenzo]]

:gerard-majax got a different steps of steps, and I also did. Here's are Alexandre's
1. On the lockscreen, open the camera with the hardware button
2. Press home button
3. Unclock the device and enter the passcode

Here are mines:
1. On the lockscreen, open the camera with the hardware button
2. Press the lockscreen button
3. Press the lockscreen button again to see the lockscreen.
4. Open the camera with either the physical button or the slider.

Actual results
With both set of steps: You arrive back on the camera app and it reads that it's already used. 

From these 2 sets, this looks more like a Camera bug. Moving the bug there.


Build info:
Build ID               20150608202903
Gaia Revision          ea27c4ed5b6083c9e21d233d4804372ac4d5d353
Gaia Date              2015-06-08 03:06:41
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/e10e2e8d8bf2
Gecko Version          41.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150608.201431
Firmware Date          Mon Jun  8 20:14:39 UTC 2015
Bootloader             s1

Comment 9

[Wilson Page [:wilsonpage]]

Attachment: Screenshot 2015-06-09 16.36.25.png

It does in-fact appear that two instances of the Camera app are open at once, confirming my suspicions in comment 6.

Comment 10

[Wilson Page [:wilsonpage]]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #8)
> :gerard-majax got a different steps of steps, and I also did. Here's are
> Alexandre's
> 1. On the lockscreen, open the camera with the hardware button
> 2. Press home button
> 3. Unclock the device and enter the passcode
> 
> Here are mines:
> 1. On the lockscreen, open the camera with the hardware button
> 2. Press the lockscreen button
> 3. Press the lockscreen button again to see the lockscreen.
> 4. Open the camera with either the physical button or the slider.
> 
> Actual results
> With both set of steps: You arrive back on the camera app and it reads that
> it's already used. 
> 
> From these 2 sets, this looks more like a Camera bug. Moving the bug there.
> 
> 
> Build info:
> Build ID               20150608202903
> Gaia Revision          ea27c4ed5b6083c9e21d233d4804372ac4d5d353
> Gaia Date              2015-06-08 03:06:41
> Gecko Revision        
> https://hg.mozilla.org/mozilla-central/rev/e10e2e8d8bf2
> Gecko Version          41.0a1
> Device Name            aries
> Firmware(Release)      4.4.2
> Firmware(Incremental)  eng.worker.20150608.201431
> Firmware Date          Mon Jun  8 20:14:39 UTC 2015
> Bootloader             s1

I can repro with the original steps outline in the description. I'm on yesterday's build.

Comment 11

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

If you take a look at the patch in depth you can see for the lockscreen parts there is only a 'holdcamera' change, and it's definitely good. Moreover, when there are lots of peers and owners I apparently not be expected as the major reviewer for other or whole parts. 

If you have other issues about how we co-working at reviewing System patches like this, you can raise it. Even if you ask me to as the only reviewer of the whole patch of that bug, I would transfer it to Alive or Tim. Unless others (not include me) think I'm more experienced than them.

Why I mentioned that I'm not aware of such issue is because in Bug 1133742 there is even no context of what the bug is for. See your report:

    KEY_CAMERA_SNAPSHOT should be used to be able to start the Camera app at will.
    Wilson, you could also make use of it inside the Camera app to take the picture.

You mentioned nothing about pressing a hardware button to launch the camera at anytime. The bug is also without any UX spec or things to show us about that. For lockscreen.js and lockscreen_test.js, your patch indeed follow the existing logic well, so I gave the r+. But I think the most important is, we don't have any Spark device to test with (in Taipei we don't even have enough numbers to allow us to borrow them every time we have a new review request). So when there are no any video or picture or spec about how to use it, we can only check the code, and review according to the code.

Comment 12

[Wilson Page [:wilsonpage]]

So it seems we're opening the Camera app twice on 'holdcamera' event:

1. https://github.com/mozilla-b2g/gaia/blob/master/apps/system/lockscreen/js/lockscreen.js#L603
2. https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/camera_trigger.js#L20

Comment 13

[:gerard-majax]

Attachment: Capture du 2015-06-09 18:18:57.png

Following my STR, only ONE camera app visible, and bug REPRODUCED.

Comment 14

[Wilson Page [:wilsonpage]]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #8)
> :gerard-majax got a different steps of steps, and I also did. Here's are
> Alexandre's
> 1. On the lockscreen, open the camera with the hardware button
> 2. Press home button
> 3. Unclock the device and enter the passcode
> 
> Here are mines:
> 1. On the lockscreen, open the camera with the hardware button
> 2. Press the lockscreen button
> 3. Press the lockscreen button again to see the lockscreen.
> 4. Open the camera with either the physical button or the slider.
> 
> Actual results
> With both set of steps: You arrive back on the camera app and it reads that
> it's already used. 
> 
> From these 2 sets, this looks more like a Camera bug. Moving the bug there.
> 
> 
> Build info:
> Build ID               20150608202903
> Gaia Revision          ea27c4ed5b6083c9e21d233d4804372ac4d5d353
> Gaia Date              2015-06-08 03:06:41
> Gecko Revision        
> https://hg.mozilla.org/mozilla-central/rev/e10e2e8d8bf2
> Gecko Version          41.0a1
> Device Name            aries
> Firmware(Release)      4.4.2
> Firmware(Incremental)  eng.worker.20150608.201431
> Firmware Date          Mon Jun  8 20:14:39 UTC 2015
> Bootloader             s1

These seem like different steps to reproduce. But all boil down to 'when two Camera apps are open, only one can function'. It is not possible to open more than one Camera from the Homescreen, not sure why it is from the lockscreen.

Comment 15

[Wilson Page [:wilsonpage]]

Commenting out https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/camera_trigger.js#L20-L25 fixes this bug.

Comment 16

[:gerard-majax]

(In reply to Wilson Page [:wilsonpage] from comment #15)
> Commenting out
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> camera_trigger.js#L20-L25 fixes this bug.

Thanks! I've spent quite some time trying to reproduce the issue using the same STR as documented but I have not been able to get anything more than what I got earlier. So I'm trusting your judgement here that we have multiple instances of the Camera app even though it surprises me.

I still think there's something wrong in Camera app itself that is exposed by my STR, since in this case I do see the error message at a moment:
 - Camera app gets focus back
 - There is only ONE instance of the camera

Comment 17

[:gerard-majax]

Attachment: Gaia PR

Meanwhile, this should implement the same kind of logic as commenting :). Alive, thanks for having a look at this.

Comment 18

[:gerard-majax]

Wilson, can you cross check that it fixes ? There should be no surprise given your comment 15 :)

Comment 19

[:gerard-majax]

Comment on attachment 8617475 [details] [review]
Gaia PR

Etienne, I'm switching review to you since Alive is not available right now and this needs to land soon.

Comment 20

[Wilson Page [:wilsonpage]]

(In reply to Alexandre LISSY :gerard-majax from comment #16)
> (In reply to Wilson Page [:wilsonpage] from comment #15)
> > Commenting out
> > https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> > camera_trigger.js#L20-L25 fixes this bug.
> 
> Thanks! I've spent quite some time trying to reproduce the issue using the
> same STR as documented but I have not been able to get anything more than
> what I got earlier. So I'm trusting your judgement here that we have
> multiple instances of the Camera app even though it surprises me.
> 
> I still think there's something wrong in Camera app itself that is exposed
> by my STR, since in this case I do see the error message at a moment:
>  - Camera app gets focus back
>  - There is only ONE instance of the camera

I think you're correct. There are some issues with two Camera app being open at the same time . IIRC dmarcos (NI) did some work on this a while back.

aosmand informed me that bug 1073017 will make sure that Gecko gives the camera hardware to the last app that asked for it, instead of blocking access like it currently does.

If we can't wait for that to land then we'll have to implement some same-origin signaling code in the Camera app to indicate when one app instance wants to steal the camera from another.

Comment 21

[Wilson Page [:wilsonpage]]

(In reply to Alexandre LISSY :gerard-majax from comment #18)
> Wilson, can you cross check that it fixes ? There should be no surprise
> given your comment 15 :)

With this patch applied I CAN'T reproduce any of the STR associated with this bug :)

Comment 22

[Etienne Segonzac (:etienne)]

(In reply to Alexandre LISSY :gerard-majax from comment #16)
>  - There is only ONE instance of the camera

You can have one AppWindow and one SecureWindow probably.

Comment 23

[:gerard-majax]

(In reply to Etienne Segonzac (:etienne) from comment #22)
> (In reply to Alexandre LISSY :gerard-majax from comment #16)
> >  - There is only ONE instance of the camera
> 
> You can have one AppWindow and one SecureWindow probably.

nope, I checked and I really only have one iframe that has src camera

Comment 24

[Etienne Segonzac (:etienne)]

Comment on attachment 8617475 [details] [review]
Gaia PR

The patch is a-ok, r=me

As a follow up we could
- make the |LockScreenPasscodeValidator| expose a "Service state" : |passCodeEnabled|
- when |(Service.query('locked'))| is true
  + if |(Service.query('passCodeEnabled'))| is also true
    -> dispatch a |secure-launchapp| [1] event to launch the "lockscreen camera"

[1] https://github.com/mozilla-b2g/gaia/blob/d63847b857c0626e266a60dccfa4532f5b809f25/apps/system/lockscreen/js/lockscreen.js#L651-658

Comment 25

[:gerard-majax]

(In reply to Etienne Segonzac (:etienne) from comment #24)
> Comment on attachment 8617475 [details] [review]
> Gaia PR
> 
> The patch is a-ok, r=me
> 
> As a follow up we could
> - make the |LockScreenPasscodeValidator| expose a "Service state" :
> |passCodeEnabled|
> - when |(Service.query('locked'))| is true
>   + if |(Service.query('passCodeEnabled'))| is also true
>     -> dispatch a |secure-launchapp| [1] event to launch the "lockscreen
> camera"
> 
> [1]
> https://github.com/mozilla-b2g/gaia/blob/
> d63847b857c0626e266a60dccfa4532f5b809f25/apps/system/lockscreen/js/
> lockscreen.js#L651-658

Sure, filed as bug 1173400

Comment 26

[:gerard-majax]

Rebased, commit title updated for the reviewer change. I'll land once green.

Comment 27

[:gerard-majax]

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

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Reverted for Gij failures.
https://treeherder.mozilla.org/logviewer.html#?job_id=2132415&repo=b2g-inbound

Master: https://github.com/mozilla-b2g/gaia/commit/d2f31eb85837aae6eca04d022d1f5b2023bc778c

Comment 29

[:gerard-majax]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #28)
> Reverted for Gij failures.
> https://treeherder.mozilla.org/logviewer.html#?job_id=2132415&repo=b2g-
> inbound
> 
> Master:
> https://github.com/mozilla-b2g/gaia/commit/
> d2f31eb85837aae6eca04d022d1f5b2023bc778c

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=c197046b16eda6e74c6a47a3bb0d402d43280ada

That was 100% green ...

Comment 30

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #29)
> (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #28)
> > Reverted for Gij failures.
> > https://treeherder.mozilla.org/logviewer.html#?job_id=2132415&repo=b2g-
> > inbound
> > 
> > Master:
> > https://github.com/mozilla-b2g/gaia/commit/
> > d2f31eb85837aae6eca04d022d1f5b2023bc778c
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=gaia&revision=c197046b16eda6e74c6a47a3bb0d402d43280ada
> 
> That was 100% green ...

Gij16 retriggered multiple times, still green ...

Comment 31

[:gerard-majax]

> https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=b32e345dbc0c

Much green on Gij10 after several retriggers, failures looks unrelated to my changes.

Comment 32

[:gerard-majax]

> https://treeherder.mozilla.org/#/jobs?repo=try&revision=63ee1de85964

Try including the commit https://github.com/lissyx/gaia/commit/3a2c1d79b264f6066ba21e9125029f92cdc0f34f which adds inhibiting of the resend feature in the test reported as failing

Comment 33

[:gerard-majax]

Looking at b2g-inbound, I see Gij 10 starting to fail in https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&filter-searchStr=Gij&fromchange=ef1f5518567f with https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=91c6f410b278

Comment 34

[:gerard-majax]

Attachment: Gaia PR

Carrying r+ from :etienne

Comment 35

[:gerard-majax]

That's green on https://treeherder.mozilla.org/#/jobs?repo=try&revision=63ee1de85964

Comment 36

[:gerard-majax]

And PR is green too: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=3a2c1d79b264f6066ba21e9125029f92cdc0f34f

Comment 37

[:gerard-majax]

https://github.com/mozilla-b2g/gaia/commit/68269e7b6510930eb2f644f69d27d456c1bdec75

Comment 38

[Martin Shuman [:Marty]]

This issue is verified fixed on the latest Spark 2.5 build.
Holding the camera button at the lockscreen will properly launch the camera without unlocking the phone.

Environmental Variables:
Device: Aries 2.5
Build ID: 20150804120650
Gaia: 67c38af8347f93ddc005a53f427d651b744b55c1
Gecko: 5cf4d2f7f2f2
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0