Closed
Bug 699528
Opened 13 years ago
Closed 13 years ago
Significant perf regressions when Firebug is installed (even inactive) because debug mode is on even if jsd is paused and disables ICs
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla11
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: regression, verified-beta, Whiteboard: [qa!])
Attachments
(2 files, 1 obsolete file)
13.33 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
15.84 KB,
patch
|
christian
:
approval-mozilla-aurora+
christian
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
BUILD: Aurora or Nightly build as of today STEPS TO REPRODUCE: 1) Open http://web.mit.edu/bzbarsky/www/mandelbrot-clean.html 2) Observe the runtime 3) Install Firebug 4) Enable the "Console" panel (which I believe all Firebug users do, pretty much). 5) Load the page from step 1 again; observe 6x slowdown. 6) Select the "Deactivate Firebug For This Site" menu item. Firebug UI is gone. 7) Reload the page. Observe that we still have a 4x slowdown. I get the same results if in step 6 I click the Firebug "x" icon. Looking at profiles, the difference between the 4x and the 6x is just the cost of the jsd function hook. But the difference between 1x and 4x seems to be disabled ICs (see bug 699196) and the calls to the ScriptDebugPrologue/Epilogue. But mostly the ICs. For comparison, the same test done against TM+JM shows no slowdown in step 7; simply having the debugger on but paused, with no hooks installed, did not lead us to deoptimize code in TM. Honza says that it's a problem on their end that "Deactivate Firebug For This Site" doesn't turn off the debugger completely. We'll see how that goes. Short of fixing bug 699196, is there anything else we can do here?
Assignee | ||
Comment 1•13 years ago
|
||
I wonder if we can do some sort of telemetry on Aurora right now for how much people are in debug mode....
Assignee | ||
Updated•13 years ago
|
tracking-firefox10:
--- → ?
tracking-firefox9:
--- → ?
Comment 2•13 years ago
|
||
Can we realistically fix this in the engine in time for 9? Seems like fixing it in Firebug is the way to go.
Assignee | ||
Comment 3•13 years ago
|
||
> Can we realistically fix this in the engine in time for 9? I would guess no, but I'm not the expert on this stuff... > Seems like fixing it in Firebug is the way to go. I'm not sure they can fix it either, but let's see what Honza says. Can we get this fixed for 10? Even that seems tight. :(
Comment 4•13 years ago
|
||
> 6) Select the "Deactivate Firebug For This Site" menu item. Firebug UI is gone.
So, when this step is done, Firebug calls jsd.pause(); and removes jsd.scriptHook hook. The other hooks (jsd.debuggerHook, jsd.debugHook, jsd.breakpointHook, jsd.throwHook, jsd.errorHook) remains set.
The reason why Firebug calls pause and not jsd.off when "Deactivate Firebug For This Site" is clicked is that Firebug can be active on another tab.
If you disable the Script panel (using the mini tab menu), which is done for all tabs, than jsd.off is called and the slowdown problem is gone.
So, now I think Firebug actually works as expected (assuming that paused JSD doesn't slow down pages)
The only workaround on the Firebug side that occurs to me is that Firebug could call jsd.off in case there is no other Firebug instance opened (on another tab or in different browser window). But, of course it would be better to fix the jsd.pause method.
Honza
Honza
Assignee | ||
Comment 5•13 years ago
|
||
> is that Firebug can be active on another tab. Can you not keep track of which tabs you're opened in? Also of interest: debug mode is per-compartment. Why are we trying to sync debug mode in all compartments to the global jsd on/off state? > If you disable the Script panel (using the mini tab menu), which is done for all tabs, > than jsd.off is called and The script panel already claims to be disabled. It's the _Console_ panel that's enabled. Which reminds me... why exactly does console enable the jsd again? > assuming that paused JSD doesn't slow down pages Sadly, that assumption has always been wrong with JM. I wish people had taken that more seriously... > But, of course it would be better to fix the jsd.pause method. That's not happening for Firefox 9, basically. So right now for Firefox 9 the options seem to be Firebug playing much nicer (i.e. calling jsd.off() as much as it can) or Firebug being a huge performance hit. Note that Firebug is _already_ a huge performance hit even in Fx8 on any testcase that ends up running mostly under JM. :( And in the latter case I think we will need to very clearly message that or something...
Assignee | ||
Comment 6•13 years ago
|
||
OK, more data. TM is in fact turned off in debug mode as of bug 680428. Debug mode is tied to jsd.on/off, not pause/unpause based on bug 595243. Honza, do you understand why that was done, exactly? I'm going to write up a patch that moves debug mode changes into pause/unpause for Firebug to run some regression tests on. Note that with that change you will have to completely unwind the JS stack after unpausing the debugger to actually get debug mode activated. Is that acceptable?
Assignee | ||
Updated•13 years ago
|
Summary: Turning on TI and turning off TM can lead to significant perf regressions when Firebug is installed (even inactive) → Significant perf regressions when Firebug is installed (even inactive) because debug mode is on even if jsd is paused and disables the JITs
Assignee | ||
Updated•13 years ago
|
Summary: Significant perf regressions when Firebug is installed (even inactive) because debug mode is on even if jsd is paused and disables the JITs → Significant perf regressions when Firebug is installed (even inactive) because debug mode is on even if jsd is paused and disables ICs
Assignee | ||
Comment 7•13 years ago
|
||
Attachment #573002 -
Flags: review?(sphink)
Assignee | ||
Comment 8•13 years ago
|
||
I pushed that patch to try; at some point there should be builds that can be tested. Honza, I'd really appreciate it if you could do that.... Steve, how do you feel about this patch for aurora and beta?
Comment 9•13 years ago
|
||
Comment on attachment 573002 [details] [diff] [review] Make pausing/unpausing JSD try to turn off/on debug mode as needed. new boolean argument to SetDebugModeWhenPossible is needed because if we allow sync-disable of debug mode we seem to crash when loading pages with Firebug active. Review of attachment 573002 [details] [diff] [review]: ----------------------------------------------------------------- Can you add a comment to unpause in the IDL saying that it is now asynchronous? r+ with that, because it accomplishes what it intends, and honestly JSD is on life support and all that matters is that this fixes a serious problem with Firebug. As for whether to land it and on what branches, in my mind it totally depends on whether Firebug can deal with the changed semantics, and for that I'll rely on Honza. I looked at the code, and it seemed safe to me. Specifically, after this patch: - jsd.unPause() is now asynchronous: after you unPause() to pause depth 0, you won't start getting debugger execution callbacks until the stack is fully unwound and no JS scripts are live. There is no way to query whether you're in this state. (jsd.isOn is different; see below.) All of the JSD stuff is still available as it always was, you just won't get the callbacks. - jsd.isOn will be true while there's a pending unPause() and no execution callbacks are firing. This is probably a good thing -- jsd.isOn refers to the JSD machinery only, which can be on even when the JS engine's debugMode is off. So jsd.isOn is false before you call jsd.asyncOn, it is false while that asyncOn is pending, and it is true during the execution of the asyncOn activation callback and afterwards. It will only go to false when jsd.off() is called, regardless of whether there are pause/unPause calls in between. - internally, the logic is a little weird -- when an unPause-triggered reactivation completes, it will internally attempt to call the activation callback. But the callback was nulled out when it was invoked the first time for jsd.AsyncOn, and there's no other way to set it, so it won't do anything. This is not observable by an external API user afaict, it just feels a little fragile. bz, does the above look correct to you? I read through the firebug code some, and everything looked ok from what I could tell. It seems like pause()/unPause() are only used when suspending/resuming firebug for a tab, and in the resume case you'll be exiting the full call stack before you would expect any content JS to run. ::: js/jsd/jsd_xpc.cpp @@ -2556,5 @@ > - > - JSContext *cx; > - rv = cc->GetJSContext (&cx); > - if (NS_FAILED(rv)) return rv; > - Thanks for throwing out the dead code.
Attachment #573002 -
Flags: review?(sphink) → review+
Comment 10•13 years ago
|
||
Builds are in https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bzbarsky@mozilla.com-babad789abef/
Assignee | ||
Comment 11•13 years ago
|
||
Comment 9 seems correct, in my limited understanding of this code. I made the unpause() comment on jsdIDebuggerService.idl say this: * Undo a pause. Once this is called, the debugger won't start * getting execution callbacks until the stack is fully unwound so * that no JS scripts are live. There is no way to query whether * there are such scripts left to unwind at a given point in time. Seem ok?
Assignee: general → bzbarsky
Assignee | ||
Updated•13 years ago
|
Whiteboard: [need Firebug feedback]
Assignee | ||
Updated•13 years ago
|
Priority: -- → P1
Comment 12•13 years ago
|
||
Try run for babad789abef is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=babad789abef Results (out of 216 total builds): success: 198 warnings: 18 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bzbarsky@mozilla.com-babad789abef
Comment 13•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #5) > > is that Firebug can be active on another tab. > Can you not keep track of which tabs you're opened in? > > Also of interest: debug mode is per-compartment. Why are we trying to sync > debug mode in all compartments to the global jsd on/off state? > TM is in fact turned off in debug mode as of bug 680428. > Debug mode is tied to jsd.on/off, not pause/unpause based on bug 595243. > Honza, do you understand why that was done, exactly? No, but in the end of bug 594054 Firebug never used "debugMode" since the original problem (crash) has been fixed. > The script panel already claims to be disabled. It's the _Console_ panel > that's enabled. > Which reminds me... why exactly does console enable the jsd again? The Console panel needs to inject |console| APIs into the page as soon as possible - before any script - that could use it - executes. It's done when the first script compiles, so JSD needs to be enabled. (In reply to Steve Fink [:sfink] from comment #9) > As for whether to land it and on what branches, in my mind it totally > depends on whether Firebug can deal with the changed semantics Thanks! (given that I am the only one maintaining the entire Firebug project now). > I read through the firebug code some, and everything looked ok from what I > could tell. It seems like pause()/unPause() are only used when > suspending/resuming firebug for a tab That's correct (In reply to Mozilla RelEng Bot from comment #12) > Try run for babad789abef is complete. > Builds available at > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bzbarsky@mozilla. > com-babad789abef So, I was testing this build #1 The test case in comment #0 seems to be fixed. #2 I am seeing a lot of unwrapIValue FAILS for [xpconnect wrapped jsdIValue] cause: [Exception... "Illegal value" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: chrome://firebug/content/lib/wrapper.js :: <TOP_LEVEL> :: line 60" data: no] #3 I am also seeing "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [jsdIScript.enableSingleStepInterrupts]" --- I am using this test case that fails: http://getfirebug.com/tests/content/branches/1.9/script/stepping/1179/issue1179.html Single stepping doesn't work, which could be due to #3 Honza
Assignee | ||
Comment 14•13 years ago
|
||
> The Console panel needs to inject |console| APIs into the page as soon as possible - > before any script - that could use it - executes. So it should be using the "inner window created" notification for that, no? That wasn't around back when this hack was created, but it's been around for a good bit now. > It's done when the first script compiles So it's also broken for pages that have no script compiled in them but have someone else call log() on them? Seriously, just change how you hook this up, please. I'll take a look at that testcase you mention. Thanks.
Comment 15•13 years ago
|
||
(In reply to Jan Honza Odvarko from comment #13) > (In reply to Boris Zbarsky (:bz) from comment #5) > > > is that Firebug can be active on another tab. > > Can you not keep track of which tabs you're opened in? > > > > Also of interest: debug mode is per-compartment. Why are we trying to sync > > debug mode in all compartments to the global jsd on/off state? > > > TM is in fact turned off in debug mode as of bug 680428. > > Debug mode is tied to jsd.on/off, not pause/unpause based on bug 595243. > > Honza, do you understand why that was done, exactly? > No, but in the end of bug 594054 Firebug never used "debugMode" since the > original problem (crash) has been fixed. Well, really what happened is that debugMode wasn't exposed through JSD. It's managed internally instead. As for why on/off instead of pause/unpause, it's probably a combination of (1) nobody had a clear picture of both the JSD internals and external users' usage patterns, and (2) it turns out the pause/unpause is heavily used internally and from that it seems intended to be lightweight. In the early, optimistic days of adapting JSD to JM, toggling debugMode was immediately recompiling all scripts in existence. > > I read through the firebug code some, and everything looked ok from what I > > could tell. It seems like pause()/unPause() are only used when > > suspending/resuming firebug for a tab > That's correct After looking further into it, it turns out that the above is correct only from firebug's point of view, but pause/unpause is heavily used internally by JSD as well. For example, every time an execution callback is invoked, JSD is paused just before and unpaused just after. That shouldn't matter, though. Those internal uses will flip gDesiredDebugMode back and forth, but allowSyncDisable will always be false and so it will have no effect. (Though this does mean that perhaps the allowSyncDisable parameter would be desirable even if immediate debugMode->false worked.) > (In reply to Mozilla RelEng Bot from comment #12) > So, I was testing this build > > #1 The test case in comment #0 seems to be fixed. I observed that too. > #2 I am seeing a lot of unwrapIValue FAILS for [xpconnect wrapped jsdIValue] > cause: [Exception... "Illegal value" nsresult: "0x80070057 > (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: > chrome://firebug/content/lib/wrapper.js :: <TOP_LEVEL> :: line 60" data: no] > > #3 I am also seeing "Component returned failure code: 0x80004005 > (NS_ERROR_FAILURE) [jsdIScript.enableSingleStepInterrupts]" I haven't gotten either of these. Do you have exact STR? > I am using this test case that fails: > http://getfirebug.com/tests/content/branches/1.9/script/stepping/1179/ > issue1179.html > > Single stepping doesn't work, which could be due to #3 This test is failing for me too, but without either of the error messages you gave, and the failure I see is that when I do "Step Out" in step 6 of the instructions, I get a single step instead. (So for me, it looks like single stepping works, but step out does not.) If I single-step through the body, I do not stop when returning from clickNested(). This happens with the patch even if I disable the method JIT entirely. (In fact, nothing in that test runs enough times to get JITted, so the method JIT isn't even in the picture.) The problem seems to be that debugMode is getting turned off and staying off at some point (before any of the step buttons is pressed.)
Assignee | ||
Comment 16•13 years ago
|
||
Hmm. Is Firebug catching the exceptions you mention in comment 14? I'm not seeing them, but I _am_ seeing step out not working quite right there...
Assignee | ||
Comment 17•13 years ago
|
||
> Those internal uses will flip gDesiredDebugMode back and forth, but allowSyncDisable will > always be false and so it will have no effect. Yes, that was the idea. ;) > The problem seems to be that debugMode is getting turned off and staying off at some > point Hmm. So stopping at a breakpoint spins a nested event loop; there is _always_ script on the stack there. So if one of the attempts to pause() while stopped at the breakpoint actually succeeds in turning off debug mode, it won't be able to come back on. I wonder whether it would be better to skip the debugmode munging for the internal pause/unpause. Let me try that.
Assignee | ||
Comment 18•13 years ago
|
||
And in fact, jsdService::EnterNestedEventLoop could do just that: it pauses, then calls out into JS, the unpauses.... the former might well trigger an actual debugger off while the latter would not turn it back on.
Assignee | ||
Comment 19•13 years ago
|
||
Aha, I bet I can cheat. The internal calls pass null for the out param, which scripted calls will never do. ;)
Assignee | ||
Comment 20•13 years ago
|
||
But probably better to just add an optional argument on the internal version....
Comment 21•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #18) > And in fact, jsdService::EnterNestedEventLoop could do just that: it pauses, > then calls out into JS, the unpauses.... the former might well trigger an > actual debugger off while the latter would not turn it back on. I added a bunch of printfs, and that's exactly what's happening.
Assignee | ||
Comment 22•13 years ago
|
||
Attachment #573295 -
Flags: review?(sphink)
Assignee | ||
Updated•13 years ago
|
Attachment #573002 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #573295 -
Flags: review?(sphink) → review+
Assignee | ||
Comment 23•13 years ago
|
||
I think this last is safe enough to just go with at least on m-c. Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/5636969704cd Honza, please let me know ASAP if you run into any issues, ok?
Flags: in-testsuite?
Whiteboard: [need Firebug feedback]
Target Milestone: --- → mozilla11
Comment 24•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #14) > > The Console panel needs to inject |console| APIs into the page as soon as possible - > > before any script - that could use it - executes. > > So it should be using the "inner window created" notification for that, no? > That wasn't around back when this hack was created, but it's been around for > a good bit now. Honza, in the Web Console we use nsIDOMGlobalPropertyInitializer currently: http://mxr.mozilla.org/mozilla-central/source/dom/base/ConsoleAPI.js https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDOMGlobalPropertyInitializer But as bz points out, there is also the content-document-global-created notification: https://developer.mozilla.org/en/Observer_Notifications#Documents
Comment 25•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5636969704cd
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 26•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #14) > So it's also broken for pages that have no script compiled in them but have > someone else call log() on them? Seriously, just change how you hook this > up, please. Agree, bug report created in Firebug issue list: http://code.google.com/p/fbug/issues/detail?id=4978 I'll fix it (using "inner window created" notification). Honza
Comment 27•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #23) > I think this last is safe enough to just go with at least on m-c. Pushed > http://hg.mozilla.org/integration/mozilla-inbound/rev/5636969704cd > > Honza, please let me know ASAP if you run into any issues, ok? Is try build available with the patch included? (it's always saving my time) Honza
Comment 28•13 years ago
|
||
(In reply to Steve Fink [:sfink] from comment #15) > > #2 I am seeing a lot of unwrapIValue FAILS for [xpconnect wrapped jsdIValue] > > cause: [Exception... "Illegal value" nsresult: "0x80070057 > > (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: > > chrome://firebug/content/lib/wrapper.js :: <TOP_LEVEL> :: line 60" data: no] > > > > #3 I am also seeing "Component returned failure code: 0x80004005 > > (NS_ERROR_FAILURE) [jsdIScript.enableSingleStepInterrupts]" > > I haven't gotten either of these. Do you have exact STR? Sorry I guess I forgot to mention FBTrace. STR: 1) Install Firebug 2) Install FBTrace http://getfirebug.com/releases/fbtrace/1.9/fbTrace-1.9b2.xpi 3) Check ERRORS in the Options tab (in FBTrace window) 4) Load the same test case: http://getfirebug.com/tests/content/branches/1.9/script/stepping/1179/issue1179.html 5) Follow the steps (click the button) 6) You should see a lot of unwrapIValue FAILS and every step should generate at least one fbs.ScriptInterrupter.enable; EXCEPTION Honza
Assignee | ||
Comment 29•13 years ago
|
||
No try build, sorry. But the builds in http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-11-10-04-08-08-mozilla-inbound/ should have the patch.
Comment 30•13 years ago
|
||
I am not seeing a build for Win32 in that directory. Honza
Comment 31•13 years ago
|
||
The change has long since been merged into mozilla-central, so you can just use a regular nightly. If you're not on the nightly update channel, then for example: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-11-13-03-17-58-mozilla-central/
Comment 33•13 years ago
|
||
(In reply to Steve Fink [:sfink] from comment #31) > The change has long since been merged into mozilla-central, so you can just > use a regular nightly. If you're not on the nightly update channel, then for > example: > http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2011-11-13-03-17-58- > mozilla-central/ Thanks! My STR from comment #28 works now. I have also run Firebug tests and they are only three related tests failing (all related to activation). I guess it's because unpause is now asynchronous. Two questions: 1) jsd.off() is still synchronous? 2) Following code is used in Firebug before jsd.off() is executed: while (jsd.pauseDepth > 0) // unwind completely jsd.unPause(); Will this still work? Honza
Assignee | ||
Comment 34•13 years ago
|
||
> 1) jsd.off() is still synchronous? No changes were made to the behavior of jsd.off(), correct. > while (jsd.pauseDepth > 0) // unwind completely > jsd.unPause(); > Will this still work? I believe that this will behave identically to the old code if immediately followed by jsd.off(). If _not_ followed by jsd.off(), the above snippet would cause debug mode to be turned off once all JS was popped off the stack, which didn't use to happen before. Are the activation test failures you describe a serious problem?
Assignee | ||
Comment 35•13 years ago
|
||
Comment on attachment 573295 [details] [diff] [review] This makes Honza's testcase pass Requesting approval for aurora and beta. I believe that outside of synthetic automated tests this should not cause any Firebug issues...
Attachment #573295 -
Flags: approval-mozilla-beta?
Attachment #573295 -
Flags: approval-mozilla-aurora?
Comment 36•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #34) > Are the activation test failures you describe a serious problem? No, already fixed by Sebastian in Firebug test harness. Firefox used for testing built from: http://hg.mozilla.org/mozilla-central/rev/9ae1d4f44b8b Is that correct Fx version (ie does it contain the patch)? For now I don't see any problems with JSD in Firebug (tests pass) Honza
Assignee | ||
Comment 37•13 years ago
|
||
> Is that correct Fx version (ie does it contain the patch)?
Yes.
Assignee | ||
Comment 38•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #574860 -
Flags: approval-mozilla-beta?
Attachment #574860 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•13 years ago
|
Attachment #573295 -
Flags: approval-mozilla-beta?
Attachment #573295 -
Flags: approval-mozilla-aurora?
Comment 39•13 years ago
|
||
Comment on attachment 574860 [details] [diff] [review] Patch for branches [triage comment] We're going to take this on aurora and see if anything breaks while we search a little bit more for add-ons that may be impacted. Please land on aurora asap and we'll keep the beta approval pending.
Attachment #574860 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 40•13 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/4ea9f0faa83b
Target Milestone: mozilla11 → mozilla10
Assignee | ||
Updated•13 years ago
|
status-firefox10:
--- → fixed
Target Milestone: mozilla10 → mozilla11
Comment 41•13 years ago
|
||
(In reply to Jan Honza Odvarko from comment #26) > (In reply to Boris Zbarsky (:bz) from comment #14) > > So it's also broken for pages that have no script compiled in them but have > > someone else call log() on them? Seriously, just change how you hook this > > up, please. > Agree, bug report created in Firebug issue list: > http://code.google.com/p/fbug/issues/detail?id=4978 I have been working on this issue and realized that another reason why the Console panel is activating JSD is to get correct stack trace for logs. See more details here: http://code.google.com/p/fbug/issues/detail?id=4978#c3 I don't know how to workaround the problem. Honza
Assignee | ||
Comment 42•13 years ago
|
||
If you have to have a stack trace for logs, then you really do need jsd active... but does that work when the JS engine is not in debug mode? I would think it might not.
Comment 43•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #42) > If you have to have a stack trace for logs, then you really do need jsd > active... and especially for error logs. > but does that work when the JS engine is not in debug mode? I > would think it might not. Firebug is using following steps: 1) use debugger; keyword to break the execution 2) get the stack trace from provided frame (coming from JSD) 3) peel off its own frames 4) log the result 5) resume debugger (returning RETURN_CONTINUE) Honza
Comment 44•13 years ago
|
||
Comment on attachment 574860 [details] [diff] [review] Patch for branches [triage comment] Nothing's really blown up on aurora, let's get this in beta today so it makes the build later.
Attachment #574860 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 45•13 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/d706932d8660
status-firefox9:
--- → fixed
Assignee | ||
Comment 46•13 years ago
|
||
Steve, thank you!
Updated•13 years ago
|
Comment 47•13 years ago
|
||
Verified on: Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20100101 Firefox/9.0 Build ID: 20111206234556 The test case from comment 0 has been fixed but the comment 28 test case still doesn't always work as expected (sometimes the results are the expected ones; they are described here: http://getfirebug.com/tests/content/branches/1.9/script/stepping/1179/issue1179.html). What happens most times is that when the user clicks "Step Out" for the first time, it goes to line 22 or it stays where it is (should go to line 14). I also get this error in FBTrace sometimes when clicking "Step Out": Error in hook: [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [jsdIStackFrame.callingFrame]" nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)" location: "JS frame :: resource://firebug/firebug-service.js :: <TOP_LEVEL> :: line 361" data: no] Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [jsdIStackFrame.callingFrame]. The Stack trace for the error is: resource://firebug/firebug-service.js (4105) resource://firebug/firebug-service.js (3886) () http://getfirebug.com/tests/content/branches/1.9/script/stepping/1179/issue1179.html (16) http://getfirebug.com/tests/content/branches/1.9/script/stepping/1179/issue1179.html (1) (). Please let me know if you want me to reopen this bug or log a new one for the reproducing case.
Comment 48•13 years ago
|
||
If you are not getting any noticeable performance regressions and are just seeing those error messages this can be marked verified. I would file those issues you report as bugs against Firebug.
Comment 49•13 years ago
|
||
I also verified this fix on: Mozilla/5.0 (X11; Linux i686; rv:9.0) Gecko/20100101 Firefox/9.0 I have seen no performance regressions on any Firefox 9.0 beta 5 builds. The remaining issue will be reported with the Firebug team.
Keywords: verified-beta
Comment 50•13 years ago
|
||
(In reply to Ioana Budnar [QA] from comment #47) > The test case from comment 0 has been fixed but the comment 28 test case > still doesn't always work as expected (sometimes the results are the > expected ones; they are described here: > http://getfirebug.com/tests/content/branches/1.9/script/stepping/1179/ > issue1179.html). What happens most times is that when the user clicks "Step > Out" for the first time, it goes to line 22 or it stays where it is (should > go to line 14). I am not able to reproduce that behavior (tried several times). Is there anything special I should do? Tested with Nightly http://hg.mozilla.org/mozilla-central/rev/3c321d2c9884 Honza
Comment 51•13 years ago
|
||
(In reply to Jan Honza Odvarko from comment #50) ... > I am not able to reproduce that behavior (tried several times). > Is there anything special I should do? > > Tested with Nightly > http://hg.mozilla.org/mozilla-central/rev/3c321d2c9884 > > Honza I didn't do anything different than what the STR specify. What I did specify in comment 47 though is that that issue only reproduced sometimes (almost always).
Comment 52•12 years ago
|
||
Verified as fixed on: Mozilla/5.0 (Windows NT 6.1; rv:10.0) Gecko/20100101 Firefox/10.0 Mozilla/5.0 (X11; Linux i686; rv:10.0) Gecko/20100101 Firefox/10.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0) Gecko/20100101 Firefox/10.0 Both the test cases from comment 0 and from comment 28 pass on Firefox 10.
Whiteboard: [qa+][qa!:9] → [qa+][qa!:9][qa!:10]
Comment 53•12 years ago
|
||
Verified on all affected versions, marking [qa!]
Whiteboard: [qa+][qa!:9][qa!:10] → [qa!]
You need to log in
before you can comment on or make changes to this bug.
Description
•