Last Comment Bug 699528 - Significant perf regressions when Firebug is installed (even inactive) because debug mode is on even if jsd is paused and disables ICs
: Significant perf regressions when Firebug is installed (even inactive) becaus...
Status: VERIFIED FIXED
[qa!]
: regression, verified-beta
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 9 Branch
: All All
: P1 normal (vote)
: mozilla11
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
: Jason Orendorff [:jorendorff]
Mentors:
: 701713 (view as bug list)
Depends on: 699196
Blocks: infer-perf-regress
  Show dependency treegraph
 
Reported: 2011-11-03 12:30 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2012-01-05 10:42 PST (History)
18 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
verified
-
verified


Attachments
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. (5.38 KB, patch)
2011-11-08 14:15 PST, Boris Zbarsky [:bz] (still a bit busy)
sphink: review+
Details | Diff | Splinter Review
This makes Honza's testcase pass (13.33 KB, patch)
2011-11-09 12:58 PST, Boris Zbarsky [:bz] (still a bit busy)
sphink: review+
Details | Diff | Splinter Review
Patch for branches (15.84 KB, patch)
2011-11-16 03:02 PST, Boris Zbarsky [:bz] (still a bit busy)
christian: approval‑mozilla‑aurora+
christian: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2011-11-03 12:30:57 PDT
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?
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2011-11-03 12:31:38 PDT
I wonder if we can do some sort of telemetry on Aurora right now for how much people are in debug mode....
Comment 2 David Mandelin [:dmandelin] 2011-11-03 17:29:21 PDT
Can we realistically fix this in the engine in time for 9? Seems like fixing it in Firebug is the way to go.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2011-11-03 19:08:51 PDT
> 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 Jan Honza Odvarko [:Honza] 2011-11-08 07:04:07 PST
> 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
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2011-11-08 09:23:22 PST
> 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...
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2011-11-08 10:17:06 PST
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?
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-11-08 14:15:43 PST
Created 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.
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2011-11-08 14:20:21 PST
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 Steve Fink [:sfink] [:s:] 2011-11-08 17:17:16 PST
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.
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2011-11-08 18:41:57 PST
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?
Comment 12 Mozilla RelEng Bot 2011-11-08 19:20:25 PST
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 Jan Honza Odvarko [:Honza] 2011-11-09 06:03:27 PST
(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
Comment 14 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 11:11:41 PST
> 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 Steve Fink [:sfink] [:s:] 2011-11-09 11:27:26 PST
(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.)
Comment 16 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 11:36:39 PST
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...
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 11:38:58 PST
> 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.
Comment 18 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 11:40:12 PST
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.
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 11:41:21 PST
Aha, I bet I can cheat.  The internal calls pass null for the out param, which scripted calls will never do.  ;)
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 11:42:17 PST
But probably better to just add an optional argument on the internal version....
Comment 21 Steve Fink [:sfink] [:s:] 2011-11-09 12:36:50 PST
(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.
Comment 22 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 12:58:48 PST
Created attachment 573295 [details] [diff] [review]
This makes Honza's testcase pass
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2011-11-09 13:16:20 PST
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?
Comment 24 Panos Astithas [:past] 2011-11-10 00:31:49 PST
(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 Marco Bonardo [::mak] 2011-11-10 03:22:50 PST
https://hg.mozilla.org/mozilla-central/rev/5636969704cd
Comment 26 Jan Honza Odvarko [:Honza] 2011-11-10 06:11:28 PST
(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 Jan Honza Odvarko [:Honza] 2011-11-10 06:32:48 PST
(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 Jan Honza Odvarko [:Honza] 2011-11-10 06:45:32 PST
(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
Comment 29 Boris Zbarsky [:bz] (still a bit busy) 2011-11-10 07:18:23 PST
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 Jan Honza Odvarko [:Honza] 2011-11-14 05:54:21 PST
I am not seeing a build for Win32 in that directory.
Honza
Comment 31 Steve Fink [:sfink] [:s:] 2011-11-14 09:28:32 PST
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 32 Boris Zbarsky [:bz] (still a bit busy) 2011-11-14 11:09:02 PST
*** Bug 701713 has been marked as a duplicate of this bug. ***
Comment 33 Jan Honza Odvarko [:Honza] 2011-11-15 07:11:33 PST
(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
Comment 34 Boris Zbarsky [:bz] (still a bit busy) 2011-11-15 14:00:51 PST
> 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?
Comment 35 Boris Zbarsky [:bz] (still a bit busy) 2011-11-15 20:49:12 PST
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...
Comment 36 Jan Honza Odvarko [:Honza] 2011-11-16 01:32:11 PST
(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
Comment 37 Boris Zbarsky [:bz] (still a bit busy) 2011-11-16 01:52:58 PST
> Is that correct Fx version (ie does it contain the patch)?

Yes.
Comment 38 Boris Zbarsky [:bz] (still a bit busy) 2011-11-16 03:02:36 PST
Created attachment 574860 [details] [diff] [review]
Patch for branches
Comment 39 christian 2011-11-17 14:36:35 PST
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.
Comment 40 Steve Fink [:sfink] [:s:] 2011-11-17 16:07:15 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/4ea9f0faa83b
Comment 41 Jan Honza Odvarko [:Honza] 2011-11-18 01:13:52 PST
(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
Comment 42 Boris Zbarsky [:bz] (still a bit busy) 2011-11-18 01:25:32 PST
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 Jan Honza Odvarko [:Honza] 2011-11-18 01:29:48 PST
(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 christian 2011-11-22 10:00:59 PST
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.
Comment 45 Steve Fink [:sfink] [:s:] 2011-11-22 14:18:37 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/d706932d8660
Comment 46 Boris Zbarsky [:bz] (still a bit busy) 2011-11-22 14:48:19 PST
Steve, thank you!
Comment 47 Ioana (away) 2011-12-12 09:23:23 PST
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 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-12 09:27:02 PST
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 Ioana (away) 2011-12-13 05:24:45 PST
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.
Comment 50 Jan Honza Odvarko [:Honza] 2011-12-13 07:14:00 PST
(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 Ioana (away) 2011-12-13 07:22:44 PST
(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 Ioana (away) 2012-01-05 08:41:15 PST
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.
Comment 53 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-01-05 10:42:02 PST
Verified on all affected versions, marking [qa!]

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