896910 - TEST-UNEXPECTED-FAIL | ../../../../mailnews/resources/logHelper.js | Error console says [stackFrame aFolder is not defined]

Status: RESOLVED FIXED

(MailNews Core :: Testing Infrastructure, defect)

Description

[:aceman]

Can be seen in https://tbpl.mozilla.org/php/getParsedLog.php?id=25582134

TEST-INFO | (xpcshell/head.js) | test _async_driver pending (2)
System JS : ERROR resource://gre/modules/dbViewWrapper.js:269
                     ReferenceError: aFolder is not defined


The relevant code is:

247   _folderMoveHelper: function(aOldFolder, aNewFolder) {
248     let oldURI = aOldFolder.URI;
249     let newURI = aNewFolder.URI;
250     // fix up our listener tables.
251     if (oldURI in this._pendingFolderUriToViewWrapperLists) {
252       this._pendingFolderUriToViewWrapperLists[newURI] =
253         this._pendingFolderUriToViewWrapperLists[oldURI];
254       delete this._pendingFolderUriToViewWrapperLists[oldURI];
255     }
256     if (oldURI in this._interestedWrappers) {
257       this._interestedWrappers[newURI] =
258         this._interestedWrappers[oldURI];
259       delete this._interestedWrappers[oldURI];
260     }
261 
262     let wrappers = this._interestedWrappers[newURI];
263     if (wrappers) {
264       // clone the list to avoid confusing mutation by listeners
265       for each (let [, wrapper] in Iterator(wrappers.concat())) {
266         wrapper._folderMoved(aOldFolder, aNewFolder);
267       }
268       // if the folder is deleted, it's not going to get deleted again.
269       delete this._interestedWrappers[aFolder.URI];
270     }

Looks like line 269 should use one of oldURI or newURI, but oldURI is already deleted at like 259... Maybe this is just a leftover line that should be removed altogether.

Comment 1

[:aceman]

Asuth, you seem to be the author of that function, can you please look at what the correct logic should be?

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

I think the situation is that the 'aFolder' logic is derived from the folderDeleted code elsewhere in the file.  I think aFolder in that case wanted to be aOldFolder, but as you point out, we already cleared that entry out of _interestedWrappers.  (I probably was migrating the logic up to the above case and then got distracted and forgot to remove the lower code.)  Yes, I think the line should be removed, as per your analysis.

If you're a valid reviewer for the code and want to delegate review to me for that change, r=asuth.  Otherwise I think you need to find a reviewer :)

Comment 3

[:aceman]

Attachment: patch

Thanks. I am not a reviewer but I think standard8 will accept take your opinion :)

Comment 4

[Mark Banner (:standard8)]

https://hg.mozilla.org/comm-central/rev/bc77857518f2