Closed Bug 882136 Opened 9 years ago Closed 9 years ago

Camera share icon gets stuck in the notification bar; media lock held on tab close


(Firefox for Android Graveyard :: General, defect)

Not set


(firefox24 verified, fennec24+)

Firefox 24
Tracking Status
firefox24 --- verified
fennec 24+ ---


(Reporter: ibarlow, Assigned: wesj)



(Keywords: reproducible, Whiteboard: [getUserMedia][android-gum+])


(1 file, 1 obsolete file)

Steps to reproduce: 
- Pref on webrtc
- Go to
- Enable camera/audio sharing
- Close gum_test tab

expected result
- Camera icon disappears 

actual result
- Camera icon remains in the notification tray and cannot be cleared, neither by swiping, nor closing firefox, nor starting and stopping a new webrtc session. I have to restart my phone to get rid of it.
I can reproduce this on the Alcatel One Touch 8008X (Android 4.1) as well; I closed the tab and the surface preview was still visible with the notification still present.

I suspect the larger issue here is not severing the local media connection on closing a tab!
tracking-fennec: --- → ?
OS: Mac OS X → Android
Hardware: x86 → ARM
Whiteboard: [getUserMedia][android-gum?]
Whiteboard: [getUserMedia][android-gum?] → [getUserMedia][android-gum+]
Duplicate of this bug: 882627
Assignee: nobody → wjohnston
tracking-fennec: ? → 24+
I believe this bug is the symptom of the issue with local media being active on tab close.

This is reproducible on the Nexus 7 (Android 4.2.2) (, share audio and video, close the active tab). Results are what I see in comment #1 (notification stuck and surface preview still visible).
Summary: Camera share icon gets stuck in the notification bar → Camera share icon gets stuck in the notification bar; media lock held on tab close
Blocks: 881875
Keywords: reproducible
Attached patch Patch v1 (obsolete) — Splinter Review
This basically keeps a list of notifications (at least the ones that go through the NotificationHelper) and forces them all closed when the app is destroyed (or the notification is tapped). Notifications that go through other channels shouldn't be affected. Ongoing notifications stay around even if tapped.
Attachment #763913 - Flags: review?(
Attached patch PatchSplinter Review
Removed some logging and comments.
Attachment #763913 - Attachment is obsolete: true
Attachment #763913 - Flags: review?(
Attachment #763916 - Flags: review?(
Comment on attachment 763916 [details] [diff] [review]

Review of attachment 763916 [details] [diff] [review]:

Looks good (with suggested fixes).

::: mobile/android/base/
@@ +1781,5 @@
>          }
>          handleNotification(ACTION_ALERT_CALLBACK, alertName, alertCookie);
> +
> +        if (intent.hasExtra("NotificationHelper_ID")) {
> +            String id = intent.getStringExtra("NotificationHelper_ID");

final String id...

::: mobile/android/base/
@@ +27,4 @@
>  public class NotificationHelper implements GeckoEventListener {
>      private static final String LOGTAG = "GeckoNotificationManager";
> +    private Context mContext;
> +    List<String> mShowing;


@@ +31,4 @@
>      public NotificationHelper(Context context) {
>          mContext = context;
> +        mShowing = new ArrayList<String>();

I'm going to assume that this list will be very short in most cases. Checking 'contains' in a List is not very efficient. If efficiency is needed here, I'd use something like a SparseBooleanArray or similar.

@@ +98,5 @@
>          notificationIntent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
> +        // if this isn't an ongoing notification, add the id to the intent so that we
> +        // can remove the notification from our list of active notifications if its clicked
> +        if (!ongoing) {
> +            notificationIntent.putExtra("NotificationHelper_ID", id);

Turn "NotificationHelper_ID" into a public constant in NotificationHelper and use it here and in BrowserApp.

@@ +118,5 @@
>              id = message.getString("id");
>          } catch (JSONException ex) {
>              Log.i(LOGTAG, "Error parsing", ex);
>              return;
>          }

nit: empty line here.

@@ +127,3 @@
>          NotificationManager manager = (NotificationManager) mContext.getSystemService(Context.NOTIFICATION_SERVICE);
>          manager.cancel(id.hashCode());
> +        if (mShowing.contains(id)) {

remove() is idempotent, no need to check if it contains id before trying.
Attachment #763916 - Flags: review?( → review+
I switch from an ArrayList to a HashSet for the list. This list should never be very big, but it should be more performant. And these IDs should be unique:
"NotificationHelper_ID" ? underscores in strings?
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Keywords: verifyme
Not working for me or one other person in the testday today. The issue cited in comment 0 is still present even with landing of this patch. Filing a followup.
bug 885783 filed as a followup.
Depends on: 885783
Keywords: verifyme
I too can reproduce this well (Nightly 06/21), let's investigate in bug 885783.
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.