Last Comment Bug 355161 - Function.prototype disappears after several seconds of browser's work. (chrome)
: Function.prototype disappears after several seconds of browser's work. (chrome)
Status: RESOLVED FIXED
[Fx 2.0.0.1]
: fixed1.8.1.1, regression
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 1.8 Branch
: x86 Windows XP
: -- normal with 7 votes (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
: Jason Orendorff [:jorendorff]
Mentors:
: 357947 358289 359253 360267 361179 (view as bug list)
Depends on: 358755
Blocks: 343417
  Show dependency treegraph
 
Reported: 2006-10-02 14:41 PDT by Yan
Modified: 2007-07-31 11:04 PDT (History)
17 users (show)
mbeltzner: blocking1.8.1-
dveditz: blocking1.8.1.1+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch based on jst's investigation and suggestion (1.53 KB, patch)
2006-10-03 22:18 PDT, Brendan Eich [:brendan]
jst: review-
Details | Diff | Splinter Review
Not what we want, but fixes the bug. (3.16 KB, patch)
2006-10-25 15:45 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Splinter Review
Alternate approach. (2.70 KB, patch)
2006-10-26 18:21 PDT, Johnny Stenback (:jst, jst@mozilla.com)
mrbkap: review+
brendan: superreview+
dveditz: approval1.8.1.1+
Details | Diff | Splinter Review
Trunk version of the above. (5.41 KB, patch)
2006-10-27 17:30 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Splinter Review

Description Yan 2006-10-02 14:41:14 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061001 BonEcho/2.0

Test case:

chrome.manifest:
overlay	chrome://browser/content/browser.xul chrome://test/content/overlay.xul

chrome://test/content/overlay.xul:
<?xml version="1.0"?>
<overlay id="testOverlay" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
<script type="application/x-javascript">
<![CDATA[

Function.prototype.f=function(){};
Function.prototype.s='string';

function dumpF(s){
	dump('\n'+s+':Function.prototype.f='+(typeof Function.prototype.f));
}
function dumpS(s){
	dump('\n'+s+':Function.prototype.s='+(typeof Function.prototype.s));
}

dumpF('without timeout');
dumpS('without timeout');

setTimeout("dumpF('0ms')",0);
setTimeout("dumpS('0ms')",0);

setTimeout("dumpF('1000ms')",1000);
setTimeout("dumpS('1000ms')",1000);

setTimeout("dumpF('5000ms')",5000);
setTimeout("dumpS('5000ms')",5000);

setTimeout("dumpF('10000ms')",10000);
setTimeout("dumpS('10000ms')",10000);
]]>
</script>

</overlay>

After several seconds "typeof Function.prototype.f" becomes "undefined" instead of being "function".


Reproducible: Always

Steps to Reproduce:
1. Apply overlay mentioned above to chrome://browser/content/browser.xul.
2. Set browser.dom.window.dump.enabled=true in about:config.

(You can download and install a test extension which does items 1 and 2:
http://xsms.nm.ru/temp/gecko/function_prototype/testxpi.xpi)

3. Run Firefox with "-console" flag.
4. Look into console for results


Actual Results:  
without timeout:Function.prototype.f=function
without timeout:Function.prototype.s=string
0ms:Function.prototype.f=function
0ms:Function.prototype.s=string
1000ms:Function.prototype.f=function
1000ms:Function.prototype.s=string
5000ms:Function.prototype.f=undefined
5000ms:Function.prototype.s=undefined
10000ms:Function.prototype.f=undefined
10000ms:Function.prototype.s=undefined

Expected Results:  
without timeout:Function.prototype.f=function
without timeout:Function.prototype.s=string
0ms:Function.prototype.f=function
0ms:Function.prototype.s=string
1000ms:Function.prototype.f=function
1000ms:Function.prototype.s=string
5000ms:Function.prototype.f=function
5000ms:Function.prototype.s=string
10000ms:Function.prototype.f=function
10000ms:Function.prototype.s=string

This happens only with Firefox 2.0 nightly builds.
There is no such problem in Firefox 1.5.0.x.
Comment 1 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-10-02 17:43:03 PDT
This is a regression from bug 343417, tested via local backout.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/safebrowsing/content/sb-loader.js&rev=1.10&mark=76,77#75
 is what causes Function.prototype.f to be cleared - that component loads http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/url-classifier/content/js/lang.js&rev=1.2#272 , which plays with Function.prototype.

The timeouts are only needed because that component is instantiated on a timeout from the browser window load.
Comment 2 Mike Beltzner [:beltzner, not reading bugmail] 2006-10-03 10:10:29 PDT
Bug 343417 was landed on branch over 2 months ago, and this testcase is the first report of this bug. That makes me wonder how badly (if at all) it will affect users if we release with it.

Brendan/jst: gavin tells me you're investigating. Please let us know what you think the deleterious effects of this bug might be, and the risk involved in fixing it. Backing out bug 343417 seems like the cure might be worse than the disease.
Comment 3 Brendan Eich [:brendan] 2006-10-03 12:59:23 PDT
(In reply to comment #2)
> Brendan/jst: gavin tells me you're investigating. Please let us know what you
> think the deleterious effects of this bug might be, and the risk involved in
> fixing it. Backing out bug 343417 seems like the cure might be worse than the
> disease.

It looks related to overlays, but last night jst and I saw stuff debugging that was moderately alarming.  Need to hear from jst today about what he learned.

If only XUL overlays provoke the bug, perhaps we can wait for 1.8.1.1.  We need to understand what's going on first to be sure.  The alarming stuff has to-do with split window interactions with the JS standard environment initialization.

/be
Comment 4 Brendan Eich [:brendan] 2006-10-03 22:18:01 PDT
Created attachment 241145 [details] [diff] [review]
patch based on jst's investigation and suggestion

How does this test out?

/be
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-10-03 23:15:12 PDT
(In reply to comment #4)
> Created an attachment (id=241145) [edit]
> patch based on jst's investigation and suggestion
> 
> How does this test out?

I tried testing this, but applying the patch and rebuilding in js/src resulted in a browser that would close immediately after being launched (it doesn't seem to crash).
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-03 23:54:40 PDT
Comment on attachment 241145 [details] [diff] [review]
patch based on jst's investigation and suggestion

Unfortunately this doesn't do it. For one, we end up getting here for globals with no inner, and even when I changed that to just use the context's global it still doesn't fix the problem. Still digging (had hardly any time for this tonight) :(
Comment 7 Mike Beltzner [:beltzner, not reading bugmail] 2006-10-04 10:10:57 PDT
(In reply to comment #3)
> was moderately alarming.  Need to hear from jst today about what he learned.
> If only XUL overlays provoke the bug, perhaps we can wait for 1.8.1.1.  We 

I'm going to assume that course of action until we hear differently. We're triaging every day, so just renominate if this turns out to be a showstopper.
Comment 8 Wladimir Palant 2006-10-08 04:55:16 PDT
(In reply to comment #2)
> Bug 343417 was landed on branch over 2 months ago, and this testcase is the
> first report of this bug.

Not really, the same bug affected the Custom Buttons extensions (http://adblockplus.org/development-builds/adding-filter-subscriptions-with-two-clicks#c000054). Unfortunately I didn't have time to investigate this issue propertly when I got this report, and Custom Buttons was easily fixed by avoiding using function prototypes.
Comment 9 Luke Matkins 2006-10-25 04:43:59 PDT
This may be related to bug 357947, in which properties added to Object and Function are disappearing in windows opened with window.open. 
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-25 15:45:47 PDT
Created attachment 243533 [details] [diff] [review]
Not what we want, but fixes the bug.
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-26 18:21:11 PDT
Created attachment 243737 [details] [diff] [review]
Alternate approach.

Need to run, so this patch will need to explain itself for now. Mainly for mrbkap to review and test out, but should be what we want AFAICT.
Comment 12 Wladimir Palant 2006-10-26 20:54:24 PDT
Bug 358289 seems to be referring to the same issue (prototype.js library produces the error "(function(fragment){}).bind is not a function" with the testcase).
Comment 13 Blake Kaplan (:mrbkap) 2006-10-26 23:59:22 PDT
Comment on attachment 243737 [details] [diff] [review]
Alternate approach.

Testing this is going to be hard for me, but I like the patch.
Comment 14 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-27 11:59:58 PDT
Comment on attachment 243737 [details] [diff] [review]
Alternate approach.

Cool, thanks for reviewing mrbkap. As far as testing goes, I've checked and this does fix the testcase in this bug and also the testcase in bug 357947. Wladimir, any chance you could revert your changes to the Custom Buttons extension and see if this patch fixes that as well?

As for what this patch does, the primary fix is to make js_FindClassObject innerize the object before looking up the class. But for that to work I had to tweak how the innerObject hook works in nsWindowSH. With this change it returns the outer if the outer is frozen, and with this patch an outer window is frozen on creation and remains frozen until the first document is loaded in it. That way we don't forward anything to the inner or create dummy inner objects while we're initializing the outer, and after that we do what we've always done. The bulk of this change is tweaking the freeze/thaw code to make it work as described above, the rest is the fixes in the innerObject hook and js_FindClassObject().
Comment 15 Wladimir Palant 2006-10-27 13:06:45 PDT
Sorry, recompiling Firefox is a little of a problem for me right now, you will have to test yourself. I reverted my changes, you can download the extension that still has this problem from http://adblockplus.org/trash/custombuttons-buggy.xpi. After you install put this URL into the location bar:
custombutton://Options%5D%5Bdata%3Aimage/png%3Bbase64%2CiVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAADHUlEQVR4Xl3DS2hcVRzA4d+55869M5NMppOEyctoxlgnsRgxRrSKpaQ+arG4kkJEBQmu1YUaoXQluNEuRUQ3xShiKdpiKSFpNRAosbQaY3yUGqOpcZI0Zh7JzJx7zt9FyaYffKrp0Gl2iBMkERsSJwOqWJsUZwtUTb/zvTnxlHH1CKxFRNjhcRPKurtVLcpHTelPtkcGP47yrZ/adHiy9kT+gsskjlKLwDlu5QHgZLA20HHa9GcnzGB7j7svS/Tknkc5/MDTHNyTckN3jDntHUN7RxC6AVA3q9TBr8Hqt0uH9r/DcAKKhl3f/EXjPxuoQKE9H9uZohQTNjHocz99oTYqI4JYAA/nEGe79aUNqEDDzDqt26Xr8Q7/VNjiz/jNmIZqmdvDgGyuC5eK97rt7dDV67iawRNR+01f+4vykAdFIblV/zXeknjcC/0jKu49Fe9KPRbetWvKyzhykaFpqG/AZpvHURxAgSfKv6fe05t0fe1gFFaH6WhFF1wpNIkgWcbJRS/QzyV2Zy4ke2K0PdIbY/Tws9Ld8R4mavPFRpGaM8ga0AultnR7tlp7NZ30jzZkkkirRu0Obuh08IavmE4uSag2I2S9soyi7Clnvg+u//CtXpgrsGIwQZx/W5vHYg2xsZiWprjEaCn4JLbUbNKq2XpFIcv/Qbm0LNaGqmH4FBJFOGvfre478CadLWAhJTVySTPVmdFnUil/Lkj4+WtV9frlRe6sGuDyPExPj6r43s/AWd92tX1n9g3vxSpIAEVQAolQaIxT3q6pxkoVHEAOmP4Nzk48o8KHxyGyzTaXnYieHx7k6iY0K+jMwBWgAvhADIgAU4W1eZiZO8lG8RUVPHiCmxi2udZxVotnqFptR+5/CT+vWAL6gXuBq8AVA1+e+Jut4iC+v+ohAiLgZEr/vDyg1iqjFDZfVucWPuc2oMPAR18t8OHkCnkDNxagXjuO1qsAHjsUSBAriAeiEda31vjjF1icLbP45wtM/v4Wx87Oc37mNax5H6UA8LmFWIsoBcWt43wwPUU9MsSDS9joRy5eO09ML6E9dvwPdX9rZvPeL7sAAAAASUVORK5CYII%3D%5D%5BopenPreferences%28%29%3B%5D%5B

You will get an Options button in your Customize dialog, add it to a toolbar and restart Firefox. The current Firefox version shows the error mentioned above on startup.
Comment 16 Brendan Eich [:brendan] 2006-10-27 16:12:04 PDT
Comment on attachment 243737 [details] [diff] [review]
Alternate approach.

Cool, I especially like the removal of the ad-hoc Freeze/Thaw around the JS_NewArrayObject -- any more like that?  Prolly not.

As jst and I were discussing in person, the followup bug for this one should ask why a scope chain ends in an outer window object, such that js_FindClassObject needs to innerize.  The premise of the split window work was that no scope chain ends in an outer window.

/be
Comment 17 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-27 16:45:32 PDT
Wladimir, unless I missed something obvious that works both with and without this patch, so I'm not sure what to make of that. I'm planning to land this change real soon now, so maybe you can test it out once it's in (trunk) and see if you can prove that problem fixed as well.
Comment 18 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-27 17:29:16 PDT
(In reply to comment #16)
> (From update of attachment 243737 [details] [diff] [review] [edit])
> Cool, I especially like the removal of the ad-hoc Freeze/Thaw around the
> JS_NewArrayObject -- any more like that?  Prolly not.

I didn't find any other ones like that. And in fact on the trunk that one doesn't exist any more either, which lead me and mrbkap to thinking that the bug this fixed has regressed on the trunk. But not so any more now that this is in on the trunk. I'll attach the trunk version of this patch too...
Comment 19 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-27 17:30:25 PDT
Created attachment 243861 [details] [diff] [review]
Trunk version of the above.

This one's in on the trunk now.
Comment 20 Wladimir Palant 2006-10-27 18:25:02 PDT
Johnny, I don't see this issue with CustomButtons on trunk either, it only occurs with Firefox 2.
Comment 21 Blake Kaplan (:mrbkap) 2006-10-27 21:34:28 PDT
(In reply to comment #16)
> As jst and I were discussing in person, the followup bug for this one should
> ask why a scope chain ends in an outer window object

There is no scope chain in this case -- that's why we're using the context's globalObject which is, by definition, the outer window (since the outer window owns the context).
Comment 22 Brendan Eich [:brendan] 2006-10-27 21:43:21 PDT
(In reply to comment #21)
> (In reply to comment #16)
> > As jst and I were discussing in person, the followup bug for this one should
> > ask why a scope chain ends in an outer window object
> 
> There is no scope chain in this case -- that's why we're using the context's
> globalObject which is, by definition, the outer window (since the outer window
> owns the context).

Oh, right.  In that case, the OBJ_INNER_OBJECT should be moved up into the else clause, and then then clause should assert that the last object is not an outer.

/be
Comment 23 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-30 16:16:10 PST
(In reply to comment #20)
> Johnny, I don't see this issue with CustomButtons on trunk either, it only
> occurs with Firefox 2.

Wladimir, I did all my testing with the 1.8 branch, aka Firefox 2.
Comment 24 Wladimir Palant 2006-10-30 18:10:47 PST
I can reproduce issue with CustomButtons in both Firefox 2.0 and current 1.8.1 branch nightly. I'll test it again when you land the patch.
Comment 25 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-31 13:40:37 PST
*** Bug 357947 has been marked as a duplicate of this bug. ***
Comment 26 Johnny Stenback (:jst, jst@mozilla.com) 2006-10-31 15:54:31 PST
(In reply to comment #22)
> Oh, right.  In that case, the OBJ_INNER_OBJECT should be moved up into the else
> clause, and then then clause should assert that the last object is not an
> outer.

That does fix the testcase in bug 357947, but not the testcase in this bug. I have yet to sit down and figure out exactly what's causing us to use an outer as a scope in the testcase in this bug...
Comment 27 Bob Clary [:bc:] 2006-11-02 13:34:38 PST
*** Bug 359253 has been marked as a duplicate of this bug. ***
Comment 28 Mike Bulman 2006-11-02 13:41:17 PST
So what is the status of this making it to a release?
Comment 29 Brendan Eich [:brendan] 2006-11-02 16:36:27 PST
(In reply to comment #28)
> So what is the status of this making it to a release?

Look at the nomination flags, the blocking1.8.1.1? in particular.  That should turn into blocking1.8.1.1+ very soon.  The Mozilla and Gecko 1.8.1.1 version lines up with Firefox 2.0.0.1, the first patch release.

jst, hope you don't mind my giving you this, since you patched.  Can it be marked FIXED now?

/be
Comment 30 Daniel Veditz [:dveditz] 2006-11-08 10:42:27 PST
Comment on attachment 243737 [details] [diff] [review]
Alternate approach.

approved for 1.8 branch, a=dveditz for drivers
Comment 31 Daniel Veditz [:dveditz] 2006-11-08 10:43:12 PST
This patch was checked into the trunk, assuming it supposed to be FIXED.
Comment 32 Johnny Stenback (:jst, jst@mozilla.com) 2006-11-08 17:59:30 PST
Fix landed on the 1.8 branch.
Comment 33 Wladimir Palant 2006-11-10 04:08:49 PST
FYI: According to http://bugzilla.mozdev.org/show_bug.cgi?id=15700 this affects web pages as well.
Comment 34 Bob Clary [:bc:] 2006-11-10 10:00:51 PST
*** Bug 360267 has been marked as a duplicate of this bug. ***
Comment 35 Wladimir Palant 2006-11-15 15:27:35 PST
Verified in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061115 BonEcho/2.0 - the problematic CustomButtons version from above works, test cases in bug 357947 and http://bugzilla.mozdev.org/show_bug.cgi?id=15700 don't show any more problems either.
Comment 36 Wladimir Palant 2006-11-15 15:36:03 PST
*** Bug 358289 has been marked as a duplicate of this bug. ***
Comment 37 Boris Zbarsky [:bz] (still a bit busy) 2006-11-16 00:17:12 PST
This patch caused bug 358755 (which is basically a regression of bug 348990 and bug 323641).
Comment 38 Brendan Eich [:brendan] 2007-07-31 11:04:55 PDT
*** Bug 361179 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.