Last Comment Bug 335058 - script timeout too small in firefox
: script timeout too small in firefox
Status: RESOLVED FIXED
: dev-doc-complete, fixed1.8.0.7, fixed1.8.1
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Windows XP
: -- enhancement with 1 vote (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Hixie (not reading bugmail)
: Andrew Overholt [:overholt]
Mentors:
: 320409 330293 335060 342734 345493 349087 (view as bug list)
Depends on:
Blocks: 346527
  Show dependency treegraph
 
Reported: 2006-04-22 00:26 PDT by Ratan Hudda
Modified: 2011-01-21 11:36 PST (History)
25 users (show)
mbeltzner: blocking1.8.1+
dveditz: blocking1.8.0.4-
dveditz: blocking1.8.0.7-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Set different time limits for chrome and content, and bump content limit to 10 seconds. (4.17 KB, patch)
2006-07-25 17:59 PDT, Johnny Stenback (:jst, jst@mozilla.com)
bzbarsky: review+
brendan: superreview+
dbaron: approval1.8.1+
Details | Diff | Splinter Review
Patch that was checked in. (4.17 KB, patch)
2006-07-27 15:38 PDT, Johnny Stenback (:jst, jst@mozilla.com)
jst: review+
jst: superreview+
Details | Diff | Splinter Review
Branch patch that was checked in. (7.10 KB, patch)
2006-07-28 16:33 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Splinter Review
FWIW, this should fix the 1.8 bustage... (1.07 KB, patch)
2006-07-30 05:37 PDT, Mats Palmgren (:mats)
brendan: review+
Details | Diff | Splinter Review
1.8.0 branch patch (just the pref change) (623 bytes, patch)
2006-08-17 19:04 PDT, Daniel Veditz [:dveditz]
brendan: review+
dveditz: approval1.8.0.7+
Details | Diff | Splinter Review

Description Ratan Hudda 2006-04-22 00:26:43 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060409 Firefox/1.5.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060409 Firefox/1.5.0.2

We have been seeing "Warning: Unresponsive Script" dialog box a lot in yahoo mail beta and it happens a lot when some cpu intensive application is running on users box and they tries to check mail.

The value of dom.max_script_run_time in FF 1.5.0.2 is 5 second. It seems to be too low compare to what IE offers. Even though user can change it in about:config but it is too hard to tell millions of users to do so.

To compare it with IE:
IE timesout after executing 5,000,000 statements on current page.
http://support.microsoft.com/?kbid=175500

Wouldn't it be a good idea to set dom.max_script_run_time to 60 sec or
something like that in FF by default (I mean something reasonably high)? The
value of 5 sec seems to be too low.

Some of the bugs already filed on it are:
https://bugzilla.mozilla.org/show_bug.cgi?id=250841
https://bugzilla.mozilla.org/show_bug.cgi?id=330293
https://bugzilla.mozilla.org/show_bug.cgi?id=308205
https://bugzilla.mozilla.org/show_bug.cgi?id=320409
https://bugzilla.mozilla.org/show_bug.cgi?id=320020
https://bugzilla.mozilla.org/show_bug.cgi?id=320409



Reproducible: Sometimes
Comment 1 Ratan Hudda 2006-04-22 00:34:48 PDT
Some what related bug: 
https://bugzilla.mozilla.org/show_bug.cgi?id=311994
Comment 2 Ryan Flint [:rflint] (ping via IRC for reviews) 2006-04-22 00:36:36 PDT
*** Bug 335060 has been marked as a duplicate of this bug. ***
Comment 3 chris hofmann 2006-04-26 08:04:03 PDT
we bumped this up in minimo due to slower processors on mobile devices, and it seems to be working well there.  might be worth considering for firefox 2.0 as well due to more use of intensive Ajax apps on the web.
Comment 4 Daniel Veditz [:dveditz] 2006-04-26 11:40:23 PDT
mrbkap, brendan: what do you think?
Comment 5 Bob Clary [:bc:] 2006-04-26 12:05:01 PDT
I have mine set to 1800 and don't see problems on public web sites. Of course that is too high, but 60 seems reasonable.
Comment 6 Brendan Eich [:brendan] 2006-04-26 13:09:16 PDT
IE is not comparable, because as far as I know, it uses COM apartment threading to avoid problems hanging the whole app due to a runaway or rogue script.

Locking up our UI for 60 seconds, without the Stop button even being live, is a bad idea.  Even 10 seconds is long.

This bug's reporter has heard it before: for cross-browser best results, break up long computations via setTimeout.  This sucks in known ways for the few and elite hackers who write such long-running JS.  It does not suck for the millions of end users, and they're the audience to optimize for, given the current browsers.

Things will get better as we evolve JS and our execution model a bit, so don't get me wrong: we should be and are working on making things better for JS hackers. But I do not believe jacking up the limit, given the whole app locks up with no UI responsiveness, is the right answer.

/be
Comment 7 Ratan Hudda 2006-04-26 13:42:54 PDT
Brendan,
The problem with the time based script timeout is that if users' computer is running some CPU intensive task at the same time as Firefox then they will see the "unresponsive script dialog" too often if they are browsing around. We have done optimizations using setTimeout to avoid it and even though we see it at times when CPU intensive tasks like virus scan, backup is running on windows box.

If time based script timeout is hard to fix then can we move FF script timeout to IE modal i.e. timeout based on number of statements executed and that way we can avoid FF script timeout dependency on other tasks in the system.

Correct me if I知 wrong on the dependency of CPU intensive task and firefox as that is what we have observed.

Thanks for your response.
Ratan
Yahoo Mail Team
Comment 8 Brendan Eich [:brendan] 2006-04-26 14:30:45 PDT
(In reply to comment #7)
> Brendan,
> The problem with the time based script timeout is that if users' computer is
> running some CPU intensive task at the same time as Firefox then they will see
> the "unresponsive script dialog" too often if they are browsing around. We have
> done optimizations using setTimeout to avoid it and even though we see it at
> times when CPU intensive tasks like virus scan, backup is running on windows
> box.

Sounds like the OS is starving Firefox, but let's say that happens and we have to work around it.

> If time based script timeout is hard to fix then can we move FF script timeout
> to IE modal i.e. timeout based on number of statements executed and that way we
> can avoid FF script timeout dependency on other tasks in the system.

We don't count statements.  We used to monitor backward branches and returns in the bytecoded JS VM, but that was fixed using wall-clock time, because counting backward branches does not really count time at all, and the UI is locked up the whole while.  That is, you could have 256K backward branches that run in < 1sec, or take > 60sec, depending on what goes on between the branches.

> Correct me if I知 wrong on the dependency of CPU intensive task and firefox as
> that is what we have observed.

How much computation are you doing between setTimeouts?  If it's still a lot, any way to use a smaller quantum?

/be
Comment 9 Ratan Hudda 2006-04-26 15:28:58 PDT
>How much computation are you doing between setTimeouts?  If it's still a lot,
>any way to use a smaller quantum?

We have done the changes already to accommodate FF. Even though we continue to see the dialog and keep getting user complains. If users have to change the script timeout value to avoid "unresponsive script dialog" to get better web surfing experience then I think it defeats the purpose of keeping it too low.

I was wandering if it is possible to:
1. Log something in JavaScript console about the script that is causing this problem? This will help engineers.
2. In 'unresponsive script dialog', display users a preference to change this value. This will help web surfing users and they can make their own decision.
3. Worst case, allow javascript to hear this event from FF. This way we can display users an appropriate messsage as how to fix it.
    e.g.
    window.onunresponsiveScript = userAlertAboutPref;
4. Worst case, allow javascript to change this value for a browser session   ( for that domain or all domain in that session)
     window.max_script_run_time = 20000; //20 second, should be greater than default

Thanks
Ratan

Comment 10 Daniel Veditz [:dveditz] 2006-04-27 11:52:50 PDT
Too late to test the implications and fall-out in 1.5.0.4, unblocking for this release.
Comment 11 Jo Hermans 2006-06-26 06:56:49 PDT
*** Bug 342734 has been marked as a duplicate of this bug. ***
Comment 12 Jo Hermans 2006-06-26 07:03:03 PDT
*** Bug 330293 has been marked as a duplicate of this bug. ***
Comment 13 Patrick Castleton 2006-07-07 18:38:50 PDT
Just my 2 cents.   The timeout happens only when Firefox is opened.  I have no problems once the program is loaded.   I believe that's when Firefox checks for updates, and here lies what I think could be the problem:  the dns lookup for mozilla takes about 20 seconds to be found. I counted the time connecting to the web site several times. So could that be what is really timing out.
Comment 14 Ratan Hudda 2006-07-19 19:04:06 PDT
comment #13
Timeouts happens even after Firefox is opened as I have seen it myself and have user reports of seeing it in Yahoo Mail Beta.
Let me know if you guys need more info. Any plan to fix it in the next release as it was taken out of 1.5.0.4 release.
Comment 15 Anders Conbere 2006-07-21 11:21:43 PDT
*** Bug 345493 has been marked as a duplicate of this bug. ***
Comment 16 Brendan Eich [:brendan] 2006-07-21 11:25:46 PDT
The script watchdog barking at our own lengthy extension-manager JS is a problem, I agree.  Separate bug, arguably.  That is, we could disable the watchdog during EM startup, and still leave this bug for later.

Cc'ing Rob Strong and Mike Connor.

/be
Comment 17 Robert Strong [:rstrong] (use needinfo to contact me) 2006-07-21 11:40:31 PDT
Does this actually happen with the extension-manager JS? I haven't seen it in quite some time and have tested operations with 100+ extensions being installed, etc. without experiencing this.
Comment 18 Brendan Eich [:brendan] 2006-07-21 11:46:35 PDT
(In reply to comment #17)
> Does this actually happen with the extension-manager JS? I haven't seen it in
> quite some time and have tested operations with 100+ extensions being
> installed, etc. without experiencing this.

Depends on OS and what all is running, competing for time slices and VM.  It's possible to give Firefox too little real time and have the watchdog bark.  Unless we think we'll save someone from an iloop but in EM or similar, we should disable the watchdog during startup, I think.

/be
Comment 19 Florin Andrei 2006-07-21 11:57:03 PDT
The default max_script_run_time is definitely too low. As you can see in bug #345493 it's triggered by simply reading a Slashdot page such as:

http://yro.slashdot.org/article.pl?sid=06/07/21/0448225

I've had it happen on other legit websites too but I forgot to report the problem.

Vital stats: Firefox-1.5.0.4, Linux Fedora Core 5, P4 2.4GHz - so this system is clearly not too slow. CPU idle is typically around 95...99% except when Firefox starts chewing that page.
Comment 20 David Baron :dbaron: ⌚️UTC-8 2006-07-21 12:02:43 PDT
Could we leave it 5 seconds for content and make it larger (30 or 60 seconds) for chrome JS?
Comment 21 Brendan Eich [:brendan] 2006-07-21 12:17:57 PDT
Ideas, easy ones for 1.8.1 star'ed:

- Bump to 10 seconds for all, or (see below) for content (*)
- Split chrome and content as dbaron proposed (*)
- Use process-virtual time, not real time (?)
- Put up some modal UI that's polled from the branch callback often enough to seem responsive
- Poll the Stop button

/be
Comment 22 Boris Zbarsky [:bz] (still a bit busy) 2006-07-21 13:24:20 PDT
> it's triggered by simply reading a Slashdot page

Er... in a vanilla no-extensions install?  I see no slow script dialog on that page, with a MUCH slower machine (p3-733).
Comment 23 Johnny Stenback (:jst, jst@mozilla.com) 2006-07-25 17:59:39 PDT
Created attachment 230677 [details] [diff] [review]
Set different time limits for chrome and content, and bump content limit to 10 seconds.

This makes us differentiate between chrome and content code (at the top of the JS stack) and gives us different prefs to controll the two limits. I set the default limit for chrome to 20, which is really high but hey, chrome code shouldn't iloop  (or call content code that could iloop) to begin with. I don't know that these are the defaults we want, but I picked those numbers as defaults to have a starting point at least.
Comment 24 Boris Zbarsky [:bz] (still a bit busy) 2006-07-25 20:20:20 PDT
Comment on attachment 230677 [details] [diff] [review]
Set different time limits for chrome and content, and bump content limit to 10 seconds.

r=bzbarsky, but I think we should leave the non-chrome pref at 5 seconds unless that 5 is being a _really_ major problem.  10 seconds is a _very_ long time for which to freeze up the browser....
Comment 25 Brendan Eich [:brendan] 2006-07-26 06:17:15 PDT
(In reply to comment #24)
> (From update of attachment 230677 [details] [diff] [review] [edit])
> r=bzbarsky, but I think we should leave the non-chrome pref at 5 seconds unless
> that 5 is being a _really_ major problem.  10 seconds is a _very_ long time for
> which to freeze up the browser....

Problem reports from the field suggest our use of real time is a bug.  If we used process-virtual time, we would be better off.  Given this flaw, even 10 seconds on an overloaded system, with Firefox deprioritized for VM reasons, may not be enough wall time.  Not sure we can get process-virtual time easily on all platforms.  Cc'ing wtc, hope he is not too busy still and can give some advice.  Thanks,

/be
Comment 26 Brendan Eich [:brendan] 2006-07-26 06:43:07 PDT
Comment on attachment 230677 [details] [diff] [review]
Set different time limits for chrome and content, and bump content limit to 10 seconds.

The patch is fine given the real-time design.  Not sure we can do better for Fx2.

/be
Comment 27 Brendan Eich [:brendan] 2006-07-26 06:53:51 PDT
Comment on attachment 230677 [details] [diff] [review]
Set different time limits for chrome and content, and bump content limit to 10 seconds.

>-    sMaxScriptRunTime = LL_INIT(0x40000000, 0);
>+    t = LL_INIT(0x40000000, 0);
>+  } else {
>+    t = time * PR_USEC_PER_SEC;
>+  }

One nit: given the righteous removal of LL_* elsewhere, do we still need LL_INIT, or would 0x4000000000000000LL not work on all platforms?

/be
Comment 28 Wan-Teh Chang 2006-07-26 10:04:15 PDT
Brendan,

I'm not sure if we can get process-virtual time on all platforms.
I believe we can get process-virtual time on Unix (including Mac
OS X and Linux) and Windows.

The LL_INIT macro is actually intended for initializers. But that
limitation only applies when PRInt64/PRUint64 is a struct.  So
it is fine now to use LL_INIT anywhere to construct a PRInt64/PRUint64
constant, even though the "INIT" in the name looks strange.  Older
versions of MSVC (such as 6.0) use i64/ui64 as the suffix for
PRInt64/PRUint64 constants.  I believe they are the only compilers
that don't support the LL/ULL suffix.
Comment 29 Johnny Stenback (:jst, jst@mozilla.com) 2006-07-27 15:38:10 PDT
Created attachment 231009 [details] [diff] [review]
Patch that was checked in.

Same as above but with LL_INIT removed.
Comment 30 Johnny Stenback (:jst, jst@mozilla.com) 2006-07-27 15:39:20 PDT
Comment on attachment 231009 [details] [diff] [review]
Patch that was checked in.

This was checked in on the trunk, but I'll leave the bug open in case we want to explore alternative fixes for the trunk. Requesting 1.8.1 approval for this change.
Comment 31 Wan-Teh Chang 2006-07-27 15:45:36 PDT
Comment on attachment 231009 [details] [diff] [review]
Patch that was checked in.

Johnny,

>-    sMaxScriptRunTime = LL_INIT(0x40000000, 0);
>+    t = 0x4000000000000000LL;

Have you tested this code under MSVC 6.0?  It doesn't
support the long long type and the LL suffix.  You need
to use the __int64 type and the i64 suffix with MSVC 6.0.
Comment 32 Johnny Stenback (:jst, jst@mozilla.com) 2006-07-27 16:18:29 PDT
(In reply to comment #31)
> Have you tested this code under MSVC 6.0?  It doesn't
> support the long long type and the LL suffix.  You need
> to use the __int64 type and the i64 suffix with MSVC 6.0.

No, I have not, but Mozilla doesn't build with MSVC 6.0 any more for other reasons either, so I don't think we really care any more. If we do, I'm happy to revert that change tho.
Comment 33 Blake Kaplan (:mrbkap) 2006-07-27 16:22:20 PDT
(In reply to comment #32)
> No, I have not, but Mozilla doesn't build with MSVC 6.0 any more for other
> reasons either

If this patch is going in on the 1.8 branch, I believe that we still use MSVC 6.0 there.
Comment 34 Johnny Stenback (:jst, jst@mozilla.com) 2006-07-27 17:04:41 PDT
0x400..00LL turned tinderbox red too, so I had to undo that change on the trunk as well.
Comment 35 David Baron :dbaron: ⌚️UTC-8 2006-07-28 10:54:22 PDT
Comment on attachment 230677 [details] [diff] [review]
Set different time limits for chrome and content, and bump content limit to 10 seconds.

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH
Comment 36 Ratan Hudda 2006-07-28 14:39:30 PDT
FYI
we are getting more reports of script timeout (unresponsive script) on Yahoo Mail Beta and I was wandering if anything got changed in FF 1.5.0.5 that can make it worse in new release.
I will keep an eye on it and update the thread again.
Comment 37 Johnny Stenback (:jst, jst@mozilla.com) 2006-07-28 16:33:36 PDT
Created attachment 231177 [details] [diff] [review]
Branch patch that was checked in.
Comment 38 Johnny Stenback (:jst, jst@mozilla.com) 2006-07-28 16:35:27 PDT
Fixed on trunk and 1.8 branch.
Comment 39 Doug Halamay 2006-07-28 19:43:41 PDT
Doing my regular "nightly" build with MSVC7.1, I get the following build error on the branch with the patch that was checked in:

-nologo -W3 -Gy -FdnsJSEnvironment.pdb  -DNDEBUG -DTRIMMED -O2p -Ob2 -GATF7 -GL -arch:SSE2 -MD            -DX_DISPLAY_MISSING=1 -DMOZILLA_VERSION=\"1.8.1b1\" -DMOZILLA_VERSION_U=1.8.1b1 -DHAVE_SNPRINTF=1 -D_WINDOWS=1 -D_WIN32=1 -DWIN32=1 -DXP_WIN=1 -DXP_WIN32=1 -DHW_THREADS=1 -DWINVER=0x400 -D_WIN32_WINNT=0x400 -DSTDC_HEADERS=1 -DWIN32_LEAN_AND_MEAN=1 -DNO_X11=1 -D_X86_=1 -DD_INO=d_ino -DMOZ_DEFAULT_TOOLKIT=\"windows\" -DMOZ_PHOENIX=1 -DMOZ_BUILD_APP=browser -DMOZ_XUL_APP=1 -DMOZ_ENABLE_CANVAS=1 -DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_XTF=1 -DMOZ_MATHML=1 -DMOZ_SVG=1 -DMOZ_SVG_RENDERER_CAIRO=1 -DMOZ_UPDATE_CHANNEL=default -DMOZ_FEEDS=1 -DMOZ_STORAGE=1 -DMOZ_SAFE_BROWSING=1 -DMOZ_URL_CLASSIFIER=1 -DMOZ_LOGGING=1 -DMOZ_USER_DIR=\"Mozilla\" -DHAVE_UINT64_T=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_MORK=1 -DMOZ_DLL_SUFFIX=\".dll\" -DJS_THREADSAFE=1 -DMOZILLA_1_8_BRANCH=1 -DMOZILLA_LOCALE_VERSION=\"1.8b5\" -DMOZILLA_REGION_VERSION=\"1.8b5\" -DMOZILLA_SKIN_VERSION=\"1.8\"  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /cygdrive/i/Mozilla/mozilla/dom/src/base/nsJSEnvironment.cpp
nsJSEnvironment.cpp
i:/Mozilla\mozilla\dom\src\base\nsJSEnvironment.cpp(516) : error C2039: 'JS_IsSystemObject' : is not a member of 'operator``global namespace'''
i:/Mozilla\mozilla\dom\src\base\nsJSEnvironment.cpp(516) : error C3861: 'JS_IsSystemObject': identifier not found, even with argument-dependent lookup
make[5]: *** [nsJSEnvironment.obj] Error 2
make[5]: Leaving directory `/cygdrive/i/mozilla/objdir/dom/src/base'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/cygdrive/i/mozilla/objdir/dom/src'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/cygdrive/i/mozilla/objdir/dom'
make[2]: *** [tier_9] Error 2
make[2]: Leaving directory `/cygdrive/i/mozilla/objdir'
make[1]: *** [default] Error 2
make[1]: Leaving directory `/cygdrive/i/mozilla/objdir'
make: *** [build] Error 2
Compile finished at 
The current time is: 18:54:46.48
 Build failed 

Any help?
Comment 40 Mats Palmgren (:mats) 2006-07-29 03:43:33 PDT
I suspect that this checkin also broke the 1.8 Camino builds:
http://tinderbox.mozilla.org/Camino/
Comment 41 Nick Kreeger 2006-07-29 22:58:12 PDT
Comment on attachment 231177 [details] [diff] [review]
Branch patch that was checked in.

This patch has caused bustage in Camino for the 1.8 Branch.
Comment 42 Nick Kreeger 2006-07-29 23:51:38 PDT
It seems that removing the "ac_add_options --disable-jsd" option from the camino mozconfig fixes the bustage. Perhaps there is a better dependency work around (maybe an |#ifdef|)?
Comment 43 Nick Kreeger 2006-07-29 23:51:45 PDT
It seems that removing the "ac_add_options --disable-jsd" option from the camino mozconfig fixes the bustage. Perhaps there is a better dependency work around (maybe an |#ifdef|)?
Comment 44 Nick Kreeger 2006-07-29 23:52:25 PDT
Oops sorry mouse caught the radio button.
Comment 45 Mats Palmgren (:mats) 2006-07-30 05:37:32 PDT
Created attachment 231295 [details] [diff] [review]
FWIW, this should fix the 1.8 bustage...

FWIW, this is the change I did to fix my 1.8 branch debug build on Linux...
Comment 46 Nick Kreeger 2006-07-30 11:11:48 PDT
(In reply to comment #45)
> Created an attachment (id=231295) [edit]
> FWIW, this should fix the 1.8 bustage...
> 
> FWIW, this is the change I did to fix my 1.8 branch debug build on Linux...
> 

This patch fixes the Camino 1.8 branch bustage as well.
Comment 47 Doug Halamay 2006-07-30 14:09:38 PDT
(In reply to comment #45)
> Created an attachment (id=231295) [edit]
> FWIW, this should fix the 1.8 bustage...
> 
> FWIW, this is the change I did to fix my 1.8 branch debug build on Linux...
> 

I can also confirm that this fixes the problem on my local build (MSVC7.1) on WinXP.
Comment 48 Stuart Morgan 2006-07-30 15:27:38 PDT
I'm not familiar with this code, but if the function in question is now being used for things that aren't related to JS debugging, then it probably needs to be moved to a non-JS-debugging header, rather than bringing the debug header into a core JS file.
Comment 49 Adam Guthrie 2006-07-30 15:37:01 PDT
Mats, you should probably ask for review and get this patch in the tree...
Comment 50 Brendan Eich [:brendan] 2006-07-30 15:38:57 PDT
(In reply to comment #48)
> I'm not familiar with this code, but if the function in question is now being
> used for things that aren't related to JS debugging,

That's not relevant -- JS_IsSystemObject was always part of jsdbgapi.h.

The bug that bit Camino was a latent one, a dependency on nested header includes. Since nsJSEnvironment.cpp directly calls JS_IsSystemObject, it must directly include "jsdbgapi.h".

> then it probably needs to
> be moved to a non-JS-debugging header, rather than bringing the debug header
> into a core JS file.

nsJSEnvironment.cpp is a core DOM file, rather -- but the main point is that "dbg" or not, JS_IsSystemObject and other reflection or non-main-API exports have been and will continue to be provided via jsdbgapi.h.  That's not the bug, since that organization, however distasteful, has not changed.  What changed was the direct dependency on JS_IsSystemObject from nsJSEnvironment.cpp being added without the necessary #include.

I'll + the bustage fix -- mats, please check in ASAP.

/be
Comment 51 Boris Zbarsky [:bz] (still a bit busy) 2006-07-30 21:29:05 PDT
Removing the fixed keyword so we don't lose track of that patch...
Comment 52 Adam Guthrie 2006-07-31 00:43:41 PDT
Comment on attachment 231295 [details] [diff] [review]
FWIW, this should fix the 1.8 bustage...

Looks like dougt just checked this into the 1.8 branch (without approval!).
Comment 53 Florin Andrei 2006-08-01 10:32:24 PDT
It would be nice if the timeout prompt had a checkbox saying "don't ask again during this session" or something like that.
It can be _very_ annoying to click on the pop-up 30 times just to complete the rendering of a page. I'd like the browser to just remember my last decision and apply it automatically.
Comment 54 Tony Mechelynck [:tonymec] 2006-08-04 18:25:24 PDT
(In reply to comment #53)
> It would be nice if the timeout prompt had a checkbox saying "don't ask again
> during this session" or something like that.
> It can be _very_ annoying to click on the pop-up 30 times just to complete the
> rendering of a page. I'd like the browser to just remember my last decision and
> apply it automatically.
> 

See bug 347365 for a different-but-complementary approach to solving this same problem.
Comment 55 Tony Mechelynck [:tonymec] 2006-08-04 18:29:47 PDT
(In reply to comment #0)
[...]
> Some of the bugs already filed on it are:
> https://bugzilla.mozilla.org/show_bug.cgi?id=250841
> https://bugzilla.mozilla.org/show_bug.cgi?id=330293
> https://bugzilla.mozilla.org/show_bug.cgi?id=308205
> https://bugzilla.mozilla.org/show_bug.cgi?id=320409
> https://bugzilla.mozilla.org/show_bug.cgi?id=320020
> https://bugzilla.mozilla.org/show_bug.cgi?id=320409
[...]

After reading the descriptions, it seems to me that most of the above are dupes of each other.
Comment 56 Mike Schroepfer 2006-08-15 18:30:34 PDT
Is this safe enough for the 1.8.0.x branch? 
Comment 57 Tony Mechelynck [:tonymec] 2006-08-15 19:04:12 PDT
(In reply to comment #56)
> Is this safe enough for the 1.8.0.x branch? 
> 

I don't know how to define "safe enough", but I do know that there have been requests by Fx/Tb 1.5.0.x users (me among them) to make the preference easier to find, because that same popup was so annoying. See for instance bug 347365 (Fx) and bug 347476 (Tb), which are not dupes of this one but relate to the same topic. I don't like bugspamming any more than anyone else, I just hope that some fix will be found for the 1.8.0.x/1.5.0.x branch before it becomes obsolete. Keep up the good work, you guys. :-)
Comment 58 Daniel Veditz [:dveditz] 2006-08-16 14:29:28 PDT
We don't want this code change in 1.5.0.x, but if you want to increase the default pref (only) to 10 to accomodate popular sites that seems safe enough and we'd consider approving that.

Would that be good enough?
Comment 59 Florin Andrei 2006-08-16 14:41:05 PDT
(In reply to comment #58)
> We don't want this code change in 1.5.0.x, but if you want to increase the
> default pref (only) to 10 to accomodate popular sites that seems safe enough
> and we'd consider approving that.
> 
> Would that be good enough?

Anything's better than the current situation.
Comment 60 Ratan Hudda 2006-08-16 15:50:37 PDT
Default max_script_run_time increase in 1.8.0.x/1.5.0.x branch will be appreciated as I saw it twice today on Firefox/1.5.0.6

I'm assuming FF 2.0 (1.8.1.x branch) will set it to following default as fix seems to be marked as 1.8.1 blocker.
dom.max_chrome_script_run_time = 20 sec;
dom.max_script_run_time =  10 sec

I can see the warning dialog appearing everywhere if I throttle cpu on my box using 
slow or winThrottle
Comment 61 Ratan Hudda 2006-08-17 16:10:02 PDT
I have verified in nightly build that (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060817 BonEcho/2.0b1) chrome script and content script run time are set to new values:
dom.max_chrome_script_run_time = 20 sec;
dom.max_script_run_time =  10 sec

if possible then please increase content script timeout to 10 sec in FF 1.5.0.7.
Comment 62 Daniel Veditz [:dveditz] 2006-08-17 19:04:22 PDT
Created attachment 234346 [details] [diff] [review]
1.8.0 branch patch (just the pref change)

Brendan: are you OK with just the time-limit change for the 1.8.0 branch?
Comment 63 Brendan Eich [:brendan] 2006-08-17 21:28:08 PDT
Comment on attachment 234346 [details] [diff] [review]
1.8.0 branch patch (just the pref change)

Sure, in practice it's not going to cause hardship to wait another 5 seconds, and it sounds like it will help in some hard cases.

/be
Comment 64 Jo Hermans 2006-08-18 04:33:20 PDT
*** Bug 349087 has been marked as a duplicate of this bug. ***
Comment 65 Daniel Veditz [:dveditz] 2006-08-18 11:37:19 PDT
Comment on attachment 234346 [details] [diff] [review]
1.8.0 branch patch (just the pref change)

approved for 1.8.0 branch, a=dveditz for drivers
Comment 66 Johnny Stenback (:jst, jst@mozilla.com) 2006-08-18 15:12:11 PDT
Pref-only change landed on the 1.8.0 branch.
Comment 67 Jo Hermans 2007-09-30 04:18:49 PDT
*** Bug 320409 has been marked as a duplicate of this bug. ***
Comment 68 Henrik Skupin (:whimboo) 2011-01-20 11:08:37 PST
There is no documentation on MDN yet for the introduced 'dom.max_chrome_script_run_time' pref by this bug.

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