MozMillDropList.prototype.select causes memory leak

RESOLVED FIXED

Status

Testing Graveyard
Mozmill
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: arai, Assigned: arai)

Tracking

Details

(Whiteboard: [mozmill-2.0.4])

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Created attachment 8351693 [details] [diff] [review]
0001-store-menuitems-into-local-variable.patch

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20131216183647



Actual results:

MozMillDropList.prototype.select() stores array of menuitem elements into a global "menuitems" variable.
So it holds reference to them until I call select() again for other element.



Expected results:

"menuitem" elements should be stored into a local variable,
and reference to them should be freed when leaving select() function.
Thank you Tooro for finding this issue, and for the attached patch! This indeed slipped through review when the new code has been landed. I will have a look at the patch in a bit. As a hint you should add a reviewer in the future, so you will get the feedback and the patch can be landed.
Assignee: nobody → arai_a
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8351693 - Flags: review?(hskupin)
Comment on attachment 8351693 [details] [diff] [review]
0001-store-menuitems-into-local-variable.patch

Review of attachment 8351693 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good and makes totally sense. Code is also well aligned. You get my r+. Just one hint for the future, whenever you upload a patch please use a commit message like the following for the patch: 'Bug XYZ - Description. r=hskupin'.
Attachment #8351693 - Flags: review?(hskupin) → review+
Landed the patch as:
https://github.com/mozilla/mozmill/commit/70803a2fdd57971a34b3f436818e0649a00c4365 (master)
https://github.com/mozilla/mozmill/commit/cd3bdd9bcef6232b4798606e211a1e9641d6b8f7 (hotfix-2.0)

Tooru, if you find something else please let us know! Also if you want to help with some other features or bug fixes. We would appreciate your help and also have some challenging tasks.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [mozmill-2.0.4]
(Assignee)

Comment 4

4 years ago
Thank you for letting me know guidelines :)
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.