Closed Bug 1099500 Opened 5 years ago Closed 5 years ago

Potential memory leak in browser.js due to faulty toast notification handling


(Firefox for Android :: General, defect)

33 Branch
Not set



Firefox 38
Tracking Status
firefox38 --- fixed
fennec + ---


(Reporter: wuhaoshu, Assigned: AndyP, Mentored)



(Whiteboard: [lang=java][lang=js])


(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:30.0) Gecko/20100101 Firefox/30.0

Steps to reproduce:

Check the Firefox code snippets
                    public void onToastHidden(ButtonToast.ReasonHidden reason) {
                        if (reason == ButtonToast.ReasonHidden.TIMEOUT) {
                            GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("Toast:Hidden", buttonId));

    } else if (aTopic == "Toast:Hidden") {
      if (this.toast._callbacks[aData])
        delete this.toast._callbacks[aData];

private void show(Toast t, boolean immediate) {
        // If we're already showing a toast, replace it with the new one by hiding the old one and quickly showing the new one.
        if (mCurrentToast != null) {
            hide(true, ReasonHidden.REPLACED);
            immediate = true;

public boolean onInterceptTouchEvent(View view, MotionEvent event) {
            // Only try to hide the button toast if it's already inflated.
            if (mToast != null) {
                mToast.hide(false, ButtonToast.ReasonHidden.TOUCH_OUTSIDE);

Actual results: notify browser.js to release the callback data, only when the Hidden reason is "ReasonHidden.TIMEOUT", and therefore in case the reasons are "ReasonHidden.TOUCH_OUTSIDE" or "ReasonHidden.REPLACED", callback data will just accumulate all the way.

Expected results:

In case of "ReasonHidden.TOUCH_OUTSIDE" and "ReasonHidden.REPLACED", the callback data should be released as well.
OS: Mac OS X → Android
Hardware: x86 → All
Ever confirmed: true
Blocks: 1019735
tracking-fennec: --- → ?
Mentor: margaret.leibovic
tracking-fennec: ? → +
Whiteboard: [lang=java][lang=js]
Tiecheng are you interested in working on this bug?
Flags: needinfo?(wuhaoshu)
(In reply to Stefan Arentz [:st3fan] from comment #1)
> Tiecheng are you interested in working on this bug?

Sorry, I am more comfortable as a humble bug reporter. I checked the core product source by chance, while developing an extension for Fennec -> , and I would love to contribute my thoughts, and join the discussion. But I think it is a right choice to let the experienced developer to actually address the bug, if it does exist.
Flags: needinfo?(wuhaoshu)
Assignee: nobody → sarentz
Assignee: sarentz → nobody
The patch removes the if clause that prevented the release of callback data on ReasonHidden.TOUCH_OUTSIDE and ReasonHidden.REPLACED.
Attachment #8563491 - Flags: review?(margaret.leibovic)
Comment on attachment 8563491 [details] [diff] [review]

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

Looks good, thanks.

Looking through the code more closely, it looks like we never actually use this ReasonHidden for anything, and I think we should file a follow-up bug to just remove it altogether.
Attachment #8563491 - Flags: review?(margaret.leibovic) → review+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Depends on: 1133499
Depends on: 1133278
The dependency bugs claim this broke super toast interactions (e.g, undo close tab, switch to tab).
I was able to reproduce the problem mentioned in bug #1133278. After unapplying the patch the problem was gone. I'm now trying to find out why exactly this happens.
This fixed the problems mentioned in bug #1133278 and bug #1133499 for me. Could someone else test it as well please?
Flags: needinfo?(margaret.leibovic)
Attachment #8565065 - Flags: review?(margaret.leibovic)
Attachment #8565065 - Attachment filename: bug1099500_PotentialMemoryLeakFix.diff → bug1099500_PotentialMemoryLeakFix.diff -v2
Attachment #8565065 - Attachment description: bug1099500_PotentialMemoryLeakFix.diff → bug1099500_PotentialMemoryLeakFix.diff - v2
Attachment #8565065 - Attachment filename: bug1099500_PotentialMemoryLeakFix.diff -v2 → bug1099500_PotentialMemoryLeakFix.diff
Any idea why the onToastHidden method is called for button clicks? I would have guessed the click handler would be called.
(In reply to Mark Finkle (:mfinkle) from comment #10)
> Any idea why the onToastHidden method is called for button clicks? I would
> have guessed the click handler would be called. hides the toast in its onClickListener using ReasonHidden.CLICKED. 

This seems to be a custom thing because apparently toast can not be clicked by default.
Comment on attachment 8565065 [details] [diff] [review]
bug1099500_PotentialMemoryLeakFix.diff - v2

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

Thanks for being so quick to dig into this!

Can you please file a separate bug for this regression, and attach this patch there to land? As a general rule, we like to file new bugs for regressions, so that it's easier to track where various patches have landed.
Attachment #8565065 - Flags: review?(margaret.leibovic) → review+
Depends on: 1134015
Flags: needinfo?(margaret.leibovic)
You need to log in before you can comment on or make changes to this bug.