User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.9) Gecko/20061223 Fedora/1.0.7-0.6.fc6 pango-text SeaMonkey/1.0.7 Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.9) Gecko/20061223 Fedora/1.0.7-0.6.fc6 pango-text SeaMonkey/1.0.7 Running the attached testcase results in a null-value for the object-property of the communication object on Firefox 2.0.0.1 while it works in FF 1.5.x, Seamonkey and all xulrunner builds i tried that date before 2006-11-10. Reproducible: Always Steps to Reproduce: 1. Load base.xul from testcase 2. Open dialog by clikcing the button 3. Click accept 4. The textbox will hold the stringified (json) representation of the comObj Actual Results: In FF 2.0.0.1: {"resMode":true,"resData":null,"debug":"12345"} In Seamonkey (see build identifier): {"resMode":true,"resData":{"foo":"bar","blo":1234,"nest":{"blue":"abc"}},"debug":"12345"} Expected Results: FF 2.0.0.1: Identical results as to seamonkey In case it helps, I checkd various versions of xulrunner to trace down the problem to a specific build: It Works up to xulrunner/nightly/2006-11-09-08-mozilla1.8/ while it returns null for resData as of xulrunner/nightly/2006-11-10-08-mozilla1.8/ and later.

This code is stripped down from the version that used to be available on json.org. All i did was remove the comments and the parse-method since for this testcase only the stringify method is needed.

Some more sidenotes - in case the help ;) - This testcase is run in chrome-context - It is interesting to note, that the string value assignment is passed just fine - Making the result-variable global did not have any effect while testing

Need usable testcase; it's not clear what one does with the attached files to see the bug.

Given the regression range in comment 0: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-11-09+08&maxdate=2006-11-10+08&cvsroot=%2Fcvsroot I'd guess at bug 353090 somehow... not sure how, though.

Installable XPI Version of the testcase. After installation go to: 1. Load chrome://comobj/content/base.xul 2. Click on the button - a dialog should appear 3. Click accept in the dialog 4. The Textbox will hold the serialized representation of the comObj-variable

resdata shows as null here, although when I tried to reproduce this (by writing my own testcase) yesterday it seemed to work. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1

So, it seems like your object is there, it's just a confused object. Your json.js is giving you null probably because it does |if(x)|. If I write my own overly-cautious toString method, I can stringify it. However, the object doesn't seem to have |hasOwnProperty|, and it won't auto-stringify. The attached is essentially your test.xul, with a different stringify function.

David: please check out whether or not this is a regression from your fix for bug 353090. If we're getting a mangled object after the dialog has touched it this could be freed memory -- bad times.

So... Having done no debugging but given the info in this bug, I'm guessing that this is indeed a regression from bug 353090. At a guess, we're clearing the proto chain on the window, which includes Object.prototype. So after that Object.prototype.toString doesn't so much exist, for example. Then the json.js code does: switch (typeof x) { case 'object': if (x) { if (typeof x.toString != 'undefined') { // Code here that stringifies and returns } } e('null'); return ''; } Since x.toString is in fact undefined, you get the results you get....

Oh, and just to make it clear, I doubt that there's freed memory involved in any way. ;)

Brendan thinks clearing up the prototype chain should be OK, except for Object.prototype.

But then we can still leak via Object.prototype, no?

Yep.

This is what Brendan suggested. I haven't tested this beyond making sure that the browser still starts up.

Only thought is to make a common subroutine (static helper function) to consolidate the repeated code. /be

I did the consolidation (which was actually rather complicated, and tied in with DOM agnostic) on the trunk. I was hoping not to have to touch the branch too much more.

(In reply to comment #19) > I did the consolidation (which was actually rather complicated, and tied in > with DOM agnostic) on the trunk. I was hoping not to have to touch the branch > too much more. Ok, no problem -- sorry I keep forgetting this is branch-only. /be

I can't tell if this regresses bug 353090 because I'm seeing leaks on http://www.google.com/ig on the 1.8 branch without this patch.

This patch does (on the 1.8 branch) make the steps in comment 7 show more text than they do without it, like they did before bug 353090 landed.

Comment on attachment 253708 [details] [diff] [review] possible 1.8 branch patch r+sr=jst

dbaron: If the patch jst just reviewed is good enough for the 1.8 branch and we want it... please ask for approvals for landing? Thanks!

Comment on attachment 253708 [details] [diff] [review] possible 1.8 branch patch This is conceptually a partial backout of bug 353090. Before bug 353090 went in, we cleared scope on only the global object. Afterwards we cleared scope on it, and then up to the top of the prototype chain. This makes us stop one step before hitting the top of the prototype chain. (Assuming the prototype chain is always of length > 1, i.e., that the global object itself always has a prototype, this is guaranteed to be in between the two patches. I'll double-check this with jst...)

Comment on attachment 253708 [details] [diff] [review] possible 1.8 branch patch The regressing bug 353090 did not land on the 1.8.0 branch as far as I can tell. All things being equal I'd rather not tweak things there if we don't have to: not approved for 1.8.0 branch. Approved to land on the 1.8 branch for 1.8.1.2, a=dveditz for drivers

Ah, oops, I can't keep track of branches. I shouldn't have requested 1.8.0.10 approval.

So, I actually discussed the issue in comment 25 with jst, and we decided it's safer to change the patch so we always JS_ClearScope on the global object's JS object itself, and then walk up the proto chain.

Fix checked in to trunk and MOZILLA_1_8_BRANCH.

Verified fixed on trunk, with a 2007-02-05 build, I get: {"resMode":true,"resData":null,"debug":"12345"} with the test extension, and with a 2007-02-06 build, I get: {"resMode":true,"resData":{"foo":"bar","blo":1234,"nest":{"blue":"abc"}},"debug":"12345"} The latter result is also what I get with the latest branch builds, so also verified that it works for the latest 1.8.1 build.