953398 - MozMillDropList.prototype.select causes memory leak

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Tooru Fujisawa [:arai]]

Attachment: 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.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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'.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 4

[Tooru Fujisawa [:arai]]

Thank you for letting me know guidelines :)