[camera] app.boot can be called twice when switching from gallery to camera

RESOLVED FIXED

Status

Firefox OS
Gaia::Camera
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: djf, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
I found this while trying to diagnose bug 957709.

STR:

1) Launch gallery
2) Click on camera icon in gallery to launch camera (via an activity)
3) Click on the gallery icon in camera to switch back to gallery
4) Click on the camera icon again.

See the following message in logcat:

[JavaScript Error: "this.filmstrip is not a function" {file: "app://camera.gaiamobile.org/js/main.js" line: 1096}]

This happens when app.boot() is called twice.  And app.boot() is called twice because when camera was first launched, it was launched by an activity, so there is a system message handler listening for system messages. But you're assuming that you'll only ever get one. 

With inline activities like pick, we get a new instance of the app for each activity. But for window-disposition activities like the one that gallery uses to launch the camera, an already-running instance of the app is used if it exists.  But it still gets the system message.

I don't know if you can unregister a system message handler. If not, I think you need to ignore messages after the first. Or avoid calling boot if you've already booted or something.

Note that you're not starting the app until the system message is received.  It probably comes very quickly after registering the handler, but it might be worth measuring to make sure you're not slowing down the camera startup by waiting for it.
(Reporter)

Comment 1

4 years ago
Needinfo wilson since he touched the activity.js file last.  I'm not sure who would be best able to fix this.
Flags: needinfo?(wilsonpage)
Created attachment 8361678 [details] [review]
pull-request (master)
Attachment #8361678 - Flags: review?(dflanagan)
Flags: needinfo?(wilsonpage)
(Reporter)

Comment 3

4 years ago
Comment on attachment 8361678 [details] [review]
pull-request (master)

Wilson,

This patch fixes the immediate bug.  It doesn't really address the underlying architectural issue that your Activity abstraction doesn't really do what you need it to do.

You still have a method called check() that is really an addEventListener() for activity handlers.  Except that the activity handler also gets called immediately if there is no pending activity at all.  It is an awkward abstraction. Calling the handler code multiple times seems harmless, though, so not a high priority to clean it up, but it is something I hope you'll put on your list to do.
Attachment #8361678 - Flags: review?(dflanagan) → review+
I understand this is a quick fix and could be better. Is the following spec something we can go by, or have I missed anything?

Activity abstraction spec:

1. Should check for incoming activities and callback when the check is complete.
2. Should parse any activity data found during the check, formatting it so that makes sense to the camera app.
3. Should only run the incoming activity handler once (on app start). Camera app is not currently able to respond to incoming activities after boot, but I don't believe this to be a requirement. If it is we can add this functionality.
I believe this has since been fixed. Wilson: Do you have the bug number off-hand that fixed our Activity issues? Trying to go through and clean up some obsolete Camera bugs.
Flags: needinfo?(wilsonpage)
Fixed by bug 1001530
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(wilsonpage)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.