Closed Bug 406769 Opened 12 years ago Closed 12 years ago
simple code now causes a slow script warning dialog to appear when it didn't before (seen with All In One Gestures extension at start-up)
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b2pre) Gecko/2007120319 Minefield/3.0b2pre ID:2007120319 1. New profile, start firefox. 2. Set extensions.checkUpdateSecurity and extensions.checkCompatibility to false. 3. Install All-in-1 Mouse Gestures extension from https://addons.mozilla.org/en-US/firefox/downloads/file/60/all-in-one_gestures-0.18.0-fx.xpi 4. Close firefox 5. Start firefox Expected: - Firefox starts up unimpeded. Actual: - Firefox appears in the process list for a while, before a dialog appears: A script on this page may be busy, or it may have stopped responding. You can stop the script now, or you can continue to see if the script will complete. Script: chrome://allinonegest/content/allinonegestOverlay.js:73 Works fine: 20071203_1859_firefox-3.0b2pre.en-US.win32 Odd behaviour: 20071203_1916_firefox-3.0b2pre.en-US.win32 Checkins to module PhoenixTinderbox between 2007-12-03 18:59 and 2007-12-03 19:15 : http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1196737140&maxdate=1196738159 Due to bug 376957.
Summary: Extension All In One Gestures now throws a slow script warning dialog → Extension All In One Gestures now throws a slow script warning dialog when firefox starts up
Product: Firefox → Core
QA Contact: general → general
Sounds like All-in-1 will need to update to take into account the (interoperable, being standardized in ES4) change here before they declare themselves compatible -- is this really a Core bug? Recommend INVALID, I think.
Based on comment 1, and the fact that the extension needs to be updated, -'ing. If more info becomes available, and you think this should block. Please re-nom.
Flags: blocking1.9? → blocking1.9-
Why did we conclude that the extension needs updating? Did anyone actually verify that it wasn't a problem with that patch? Steve has pretty conclusive evidence that the landing of the patch for bug 376957 is what caused this, and I don't think we should dismiss it as "the extension needs to be updated" until we know that the cause of the bustage is that it relies on behavior that we've decided to change.
Flags: blocking1.9- → blocking1.9?
I second that this bug should be blocking in that it needs a resolution -- which can be INVALID, too. To me it's not obvious what that extension is doing wrong. Maybe Shaver can elaborate on this.
Shaver? is this a core bug or does the extension need to update...
FWIW I've used All-in-1 Gestures 0.18.0 for something like 18 months on nightly trunk builds with no problems, until this happened.
I don't know if it's a Core bug; I don't know anything about that extension, and it's huge, so without the author's co-operation here or some other test case, I don't know how we'll get to clarity. When I made my comment the only data I had was Steve's from comment #0, indicating that Ai1G wasn't tested to be compatible and wasn't compatible -- is the author one of the people on this bug?
I've just CC'd the author.
I'm Marc Boullet, the author of AiOG extension. I just received the following mail from Mark Tickner: "Hi, I have been running your very nice extension in the beta version of Mozilla Firefox for a while now using the Nightly Tester Tools to override compatibility. The 2007-12-04-04 update seems to have broken something. This is what I get from the Venkman debugger. #0: function (null)() in <chrome://allinonegest/content/allinonegestOverlay.js> line 73 071: var aioSofar = ; aioSofar = 0; 072: for (var ii = 1; ii < aioDist.length -1; ++ii) 073: aioSofar[ii+1] = aioSofar[ii] + (aioDist[ii] - aioDist[ii-1]) * aioRatio[ii]; 074: const aioCursors = ["move", "n-resize", "e-resize"]; 075: var aioScrollCount, aioScrollRate, aioScrollMax, aioASPeriod; You have a really nice extension, so it's a shame something just got broken. Best Regards" The sequence of lines is : const aioDist = [0, 20, 40, 60, 80, 100, 130, 180, 300, 5000]; const aioRatio = [.0, .067, .083, .108, .145, .2, .3, .45, .65, .9]; const aioScrollLoop = [1, 2, 4]; var aioSofar = ; aioSofar = 0; for (var ii = 1; ii < aioDist.length -1; ++ii) aioSofar[ii+1] = aioSofar[ii] + (aioDist[ii] - aioDist[ii-1]) * aioRatio[ii]; Seems there is a problem on the last line but it's quite basic code and I don't see anything wrong with it. Any ideas ?
This is a strange one. I can reproduce the "hang" with this testcase, when loaded locally (gives me the "slow script" dialog). It was reduced from the source of the All-in-One tabs extension from comment 0. Removing a single var declaration from the source causes the problem to not appear. While reducing I noticed that I could replace, for example, |var a="boo";| with |var a; var b;| and still reproduce the problem, but if I changed |var="boo";| to just |var a;|, the problem would disappear, as though the size of the interpreted code in memory mattered. I tried looking for alternatives to the loop at the bottom, but couldn't find any. The array object in the last line also seems to be required.
Assignee: nobody → general
QA Contact: general → general
The testcase seems to work when accessed via Bugzilla directly over https, as well. With a current trunk build, I get a slow script dialog, whereas with a branch build I don't (as expected for such trivial code).
marking blocking until we can figure out exactly what is going on. Igor/Brendan any changes in that time range that might be relevant?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
This can be related to bug 407501.
(In reply to comment #14) > This can be related to bug 407501. In fact I think this is caused by bug 407501 as the GC hazard can cause the behavior from comment 11.
With the patch for 407501 I still got a slow warning for the test case from comment 11. So this is something else.
No longer depends on: 407501
Summary: Extension All In One Gestures now throws a slow script warning dialog when firefox starts up → simple code now causes a slow script warning dialog to appear when it didn't before (seen with All In One Gestures extension at start-up)
The test case that currently goes into infinite loop with js shell.
This is a bug somewhere in the code generator. For the test case from comment 17 the bytecode is: 00000: defvar "a0" ... 00378: defvar "a126" main: 00381: name "a0" 00384: pop ... 00881: name "a125" 00884: pop 00885: one 00886: setgvar "a126" 00889: pop 00890: getgvar "a126" 00893: newinit 3 00895: zero 00896: one 00897: initelem 00898: one 00899: int8 2 00901: initelem 00902: int8 2 00904: int8 3 00906: initelem 00907: endinit 00908: group 00909: getprop "length" 00912: one 00913: sub 00914: lt 00915: ifeq 927 (12) 00918: one 00919: popv 00920: getgvar "a126" 00923: pop 00924: goto 890 (-34) 00927: stop Not that the bytecode at the position 920 is getgvar "a126", which just fetches the value for the golbal a126, not increases it as required in the for loop for the test: for (var a126 = 1; a126 < ([1,2,3]).length -1; ++a126) 1; If I comment out "var a125;" in the test case, then the test terminates and the bytecode becomes: 00000: defvar "a0" ... 00375: defvar "a126" main: 00378: name "a0" 00381: pop ... 00874: name "a124" 00877: pop 00878: one 00879: setgvar "a126" 00882: pop 00883: getgvar "a126" 00886: newinit 3 00888: zero 00889: one 00890: initelem 00891: one 00892: int8 2 00894: initelem 00895: int8 2 00897: int8 3 00899: initelem 00900: endinit 00901: group 00902: getprop "length" 00905: one 00906: sub 00907: lt 00908: ifeq 920 (12) 00911: one 00912: popv 00913: incgvar "a126" 00916: pop 00917: goto 883 (-34) 00920: stop with the proper invgvar at the position 913.
This is regression caused by bug 398609. The changes from bug 376957 just exposed it. As such the patch there is innocent.
In the patch for bug 398609 I have used the wrong ale to extract the constness status of the operation.
It would be nice to fix this in b2 as it exposed a bad regression in the global variable optimizer.
Status: NEW → ASSIGNED
Priority: P2 → P1
I will be away until approximately 20:00 UTC/12:00 PST. So if the patch from comment 20 would get approvals, I could land it only after that time.
I've confirmed that that patch fixes the problem for me. Thanks Igor!
Comment on attachment 292302 [details] [diff] [review] fix v1 We need to fix this for b2, so help landing today may be needed. /be
I am going to sleep (In reply to comment #24) > (From update of attachment 292302 [details] [diff] [review]) > We need to fix this for b2, so help landing today may be needed. I can not land the patch myself today - I am going to sleep soon.
Comment on attachment 292302 [details] [diff] [review] fix v1 yep - we want this for b2..
Attachment #292302 - Flags: approvalM10? → approvalM10+
I landed attachment 292302 [details] [diff] [review] on the trunk: mozilla/js/src/jsemit.c 3.291 I'll trigger a clobber to push out new nightlies.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
OS: Windows 2000 → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M10
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b2pre) Gecko/2007120915 Minefield/3.0b2pre ID:2007120915 AiOG 0.18 now works again --> VERIFIED
Status: RESOLVED → VERIFIED
(Gavin's testcase now works too)
Checking in js1_5/Regress/regress-406769.js; /cvsroot/mozilla/js/tests/js1_5/Regress/regress-406769.js,v <-- regress-406769.js initial revision: 1.1 done
You need to log in before you can comment on or make changes to this bug.