Last Comment Bug 483672 - Permission denied for <http://localhost:7080> to call method UnnamedClass.handleEvent@XPCSafeJSObjectWrapper.cpp:445.0
: Permission denied for <http://localhost:7080> to call method UnnamedClass.han...
Status: RESOLVED FIXED
[firebug-p1][need for 1.9.2]
: fixed1.9.1, regression
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: 1.9.2 Branch
: x86 Windows XP
: P1 blocker with 11 votes (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
:
Mentors:
Depends on:
Blocks: 435025 453978 493074
  Show dependency treegraph
 
Reported: 2009-03-16 14:38 PDT by John J. Barton
Modified: 2010-04-07 06:12 PDT (History)
24 users (show)
mbeltzner: blocking1.9.1+
mbeltzner: blocking1.9.0.19-
mbeltzner: wanted1.9.0.x-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1+
?
-
unaffected
-
unaffected


Attachments
Test Case (1.49 KB, application/x-xpinstall)
2009-03-18 05:32 PDT, Jan Honza Odvarko [:Honza]
no flags Details
patch v1 (5.77 KB, patch)
2009-04-02 18:51 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Updated comment (5.85 KB, patch)
2009-04-03 17:31 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Oops (5.86 KB, patch)
2009-04-06 16:46 PDT, Blake Kaplan (:mrbkap)
bzbarsky: review+
jst: superreview+
Details | Diff | Splinter Review
Fix for that (8.60 KB, patch)
2009-04-28 16:25 PDT, Blake Kaplan (:mrbkap)
jst: review+
bzbarsky: superreview+
Details | Diff | Splinter Review
For checkin (14.72 KB, patch)
2009-05-12 18:05 PDT, Blake Kaplan (:mrbkap)
mrbkap: review+
mrbkap: superreview+
Details | Diff | Splinter Review
For 1.9.1. (9.88 KB, patch)
2009-05-22 14:41 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Splinter Review

Description John J. Barton 2009-03-16 14:38:10 PDT
I get this error message:

Permission denied for <http://localhost:7080> to call method UnnamedClass.handleEvent@XPCSafeJSObjectWrapper.cpp:445.0

The line number is incorrect, the method name makes no sense.

I believe the call is in here:

function onHTTPSpyReadyStateChange(spy, event)
{
    try
    {
        spy.context.onReadySpy = spy; // maybe the handler will eval(), we want the URL.
        if (spy.onreadystatechange)
            spy.onreadystatechange.handleEvent(event);
    }
    catch (exc)
    {
        if (FBTrace.DBG_ERROR)
            FBTrace.sysout("spy.onHTTPSpyReadyStateChange: EXCEPTION", exc);
    }
    finally
    {
        delete spy.context.onReadySpy;
    }

    if (spy.xhrRequest.readyState == 4)
        onHTTPSpyLoad(spy);
}
Comment 1 John J. Barton 2009-03-16 14:45:22 PDT
Either the error is spurious, in which case this problem blocks Firebug or the error is real and the message is inadequate.  I marked the bug BLOCKS for both cases for now.
Comment 2 John J. Barton 2009-03-16 15:12:37 PDT
This is examples/exampleNetTest.js running on FF 3.1b3. It never gets to testDone.

I think that what is happening is the readyStateChange event is firing and spy.js onHTTPSpyReadyStateChange() is being called by Firefox, but it is incorrectly being run under the web page principle. The call stack has only  
chrome://firebug/content/spy.js (512)
chrome://firebug/content/spy.js (334)

So my first guess is that this is a bug in 3.1, not a new security feature.
Comment 3 Nochum Sossonko [:Natch] 2009-03-16 15:52:27 PDT
I've seen error messages similar to that, and in the console the linkified view-source url points to http://www.{some-.cpp-file}.com, with some-.cpp-file being whatever the exception passes through. It's very interesting that errors are reported to the console with the uri being some source cpp file...
Comment 4 Boris Zbarsky [:bz] (TPAC) 2009-03-16 18:18:38 PDT
Isn't this a dup of the "Chrome now gets XPCNativeWrapper for XHR" thing?
Comment 5 Rob Campbell [:rc] (:robcee) 2009-03-16 19:04:28 PDT
Not sure about the severity of this, marking sensitive.
Comment 6 Boris Zbarsky [:bz] (TPAC) 2009-03-16 19:06:17 PDT
fwiw, I don't think this is a security issue.
Comment 7 John J. Barton 2009-03-16 21:09:37 PDT
If, contrary to my assumption, the web principal is correct, then Firebug 1.3 exposes Firefox 3.0 users. I don't know.
Comment 8 Rob Campbell [:rc] (:robcee) 2009-03-17 05:23:37 PDT
exposes in what way though? Based on this it looks like something (the web principal?) is preventing an event's listener from running. This would feel more securityish if we were running arbitrary javascript in an unprotected way.

Seeking further consults from the SG gurus.
Comment 9 Rob Campbell [:rc] (:robcee) 2009-03-17 05:29:16 PDT
I'm looking for the test case mentioned above, and can't find it in http://code.google.com/p/fbug/source/browse/#svn/examples/firebug1.4.
Comment 10 Boris Zbarsky [:bz] (TPAC) 2009-03-17 06:19:18 PDT
For what it's worth, the bug I was thinking of was bug 471395.
Comment 11 Boris Zbarsky [:bz] (TPAC) 2009-03-17 06:20:11 PDT
So presumably not a duplicate, but caused by the same thing.

Peter, mind looking into this?
Comment 12 Jan Honza Odvarko [:Honza] 2009-03-18 05:32:45 PDT
Created attachment 367994 [details]
Test Case

I am attaching a simple test extension that can be used to reproduce the problem.

Steps to reproduce:

1) Install the extension and enable dump() so, you can see the final exception in the system console.

2) Start Firefox 3.1b3 and load http://www.janodvarko.cz/firebug/tests/601/Issue601.htm page

3) Press the "POST" button on the page (it generates a XHR)

4) See the system console there is following exception.
XHRTest: EXCEPTION Error: Permission denied for <http://www.janodvarko.cz> to ca
ll method UnnamedClass.handleEvent


The test-case is replacing onreadystatechange property of the XHR object by  own function. The original handler is called within this function using handleEvent. 

The exception is fired when the original handler is called so, the handler implemented on the page is never executed. You can see that the green text (on the test page) saying "{response must be displayed here}" is never replaced with the actual response.

This works as expected in Firefox 3.0.7
Comment 13 Jan Honza Odvarko [:Honza] 2009-03-18 05:36:10 PDT
(In reply to comment #10)
> For what it's worth, the bug I was thinking of was bug 471395.

It's not dup, since the problem in #471395 is that the exception was fired when the onreadystatechange property was just accessed. 
This has been fixed and works now.

In this case, the exception is fired when the onreadystatechange property is used to call the original handler.

Honza
Comment 14 Boris Zbarsky [:bz] (TPAC) 2009-03-18 06:44:38 PDT
Taking the liberty to move this to the right product.  Can someone check whether this also broke with bug 457022?  I expect it did...
Comment 15 John J. Barton 2009-03-18 08:18:34 PDT
(In reply to comment #9)
> I'm looking for the test case mentioned above, and can't find it in
> http://code.google.com/p/fbug/source/browse/#svn/examples/firebug1.4.

Sorry, I was thinking relative to the test directory:
http://code.google.com/p/fbug/source/browse/tests/content/examples/exampleNetTest.html
Comment 16 Mike Beltzner [:beltzner, not reading bugmail] 2009-03-18 08:58:06 PDT
Blocks Firebug, blocks product on resolution.
Comment 17 Nochum Sossonko [:Natch] 2009-03-21 21:06:15 PDT
(In reply to comment #14)
> whether this also broke with bug 457022?  I expect it did...

Not that bug, checked 08-11-13 and 08-11-14 builds, both of which did not reproduce the bug (following str from comment 12). Still looking for a regression range.
Comment 18 Nochum Sossonko [:Natch] 2009-03-21 21:24:06 PDT
Regression range:

works: 09-02-24
fails: 09-02-25
pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=69c86f3acc5a&tochange=8eba35e62d92
->Bug 471395?
Comment 19 Boris Zbarsky [:bz] (TPAC) 2009-03-23 12:04:31 PDT
That's not a useful range, sadly.  See comment 13.  The testcase as written is just meaningless before bug 471395 is fixed; it throws before we get to the code we're trying to test here.
Comment 20 Boris Zbarsky [:bz] (TPAC) 2009-03-23 12:05:16 PDT
Which suggests to me that we might need a better testcase, and that the issue is still bug 457022 originally.
Comment 21 Rob Campbell [:rc] (:robcee) 2009-03-24 11:42:47 PDT
honza, is there anything you can do to improve this testcase? Boris, anything you'd like to see in it in particular?
Comment 22 Boris Zbarsky [:bz] (TPAC) 2009-03-24 11:49:54 PDT
The only reason to try improving the testcase is if we think there's a relevant regression range to be found.

Frankly, I think what this bug needs at this point is just debugging.
Comment 23 Nochum Sossonko [:Natch] 2009-03-24 12:00:52 PDT
This testcase completes successfully (the date appears in green) prior to the regression range date, so I'm not sure what I'm doing wrong. Granted, I see the errors in the console, however the str in comment 12 are _not_ reproducible until the regression date...
Comment 24 Jan Honza Odvarko [:Honza] 2009-03-25 04:41:23 PDT
> The testcase as written is just meaningless before bug 471395 is fixed; it 
> throws before we get to the code we're trying to test here.
True. Unfortunately I don't know how to provide better test case that isn't dependent on 471395. The onreadystatechange property must be accessed before the original handler is called.
Comment 25 Jan Honza Odvarko [:Honza] 2009-03-25 04:49:56 PDT
(In reply to comment #23)
> This testcase completes successfully (the date appears in green) prior to the
> regression range date, so I'm not sure what I'm doing wrong. Granted, I see the
> errors in the console, however the str in comment 12 are _not_ reproducible
> until the regression date...
I am able to reproduce the problem consistently with: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3
How can I help here?
Honza
Comment 26 Rob Campbell [:rc] (:robcee) 2009-03-25 05:58:03 PDT
(In reply to comment #25)
> I am able to reproduce the problem consistently with: Mozilla/5.0 (Windows; U;
> Windows NT 6.0; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3
> How can I help here?
> Honza

I think as Boris suggests in c#22, we just need to get someone who understands this part of the code stepping through the test case in a debugger. Any volunteers? I will totally reward successful patches with coffee and pie (or a beverage and dessert of your choice) at the AllHands.
Comment 27 Blake Kaplan (:mrbkap) 2009-03-25 08:25:37 PDT
For what it's worth, I'm trying to get to this bug -- it's on my todo list behind a couple of other things though.
Comment 28 Rob Campbell [:rc] (:robcee) 2009-03-25 09:09:50 PDT
ok Blake. Thanks for the update.
Comment 29 Blake Kaplan (:mrbkap) 2009-03-31 18:08:07 PDT
So, I looked into this today, and it's very tricky. The short of it is that |someXHRobject.onreadystatechange| is a double-wrapped JS object (that is, it's an XPCWrappedNative around an XPCWrappedJS). Because of this, calling the event handler through the double wrapped JSObject ends up in XPCWrappedNative::CallMethod. This does a security check to make sure we're allowed to call the method.

Now, because we're calling this through XPCSafeJSObjectWrapper, the subject principal is the web page. This *shouldn't* be a problem because the target object here is from the web page, however, we run into these lines in nsScriptSecurityManager.cpp:
        if (!aCallContext || classInfoData.IsDOMClass())
            securityLevel.level = SCRIPT_SECURITY_SAME_ORIGIN_ACCESS;
        else
            securityLevel.level = SCRIPT_SECURITY_NO_ACCESS;
in this case, we're called from CallMethod, we we have a context, and there is no classinfo, since we're asking if we have access to touch a regular Function. So we set securityLevel.level to be noAccess and throw.

This problem is actually visible from content without any XPCSafeJSObjectWrapper involvement: the testcase:

var xhr = new XMLHttpRequest();
xhr.onreadystatechange = function(){};
alert(xhr.onreadystatechange);

throws:

JavaScript error: http://localhost/~mrbkap/temp.html, line 7: Permission denied for <http://localhost> to create wrapper for object of class UnnamedClass

for the same underlying reason (we're creating the XPCWrappedNative around the Function in that case).

Fixing this for Firefox 3.5 now is scary. jst and I talked about assuming same-origin for double-wrapped objects in the security manager, but that's a *really* scary change.

Boris, any additional thoughts?
Comment 30 Boris Zbarsky [:bz] (TPAC) 2009-03-31 19:03:44 PDT
>var xhr = new XMLHttpRequest();
>xhr.onreadystatechange = function(){};
>alert(xhr.onreadystatechange);

Did this never work?  Or did we regress it with one of our changes?

Could we give classinfo to double-wrapped functions?
Comment 31 John J. Barton 2009-03-31 20:48:21 PDT
In 3.0.8 the code gives
Permission denied to create wrapper for object of class UnnamedClass
from the alert() call.  I'm not sure what this proves since Firebug spy works in 3.0.8 but not in 3.55.

(attempting to suppress urge to insert comment about the error message...partial failure)
Comment 32 Blake Kaplan (:mrbkap) 2009-04-01 11:14:55 PDT
(In reply to comment #30)
> Did this never work?  Or did we regress it with one of our changes?

This appears to have never worked. I tested back to Firefox 1.0.8 before giving up.

> Could we give classinfo to double-wrapped functions?

Yeah, that might work... Alternatively, we could implement nsISecurityCheckedComponent for double-wrapped, non-chrome JS objects and return "sameOrigin" for everything.
Comment 33 Blake Kaplan (:mrbkap) 2009-04-01 11:57:53 PDT
(In reply to comment #31)
> I'm not sure what this proves since Firebug spy works
> in 3.0.8 but not in 3.55.

This particular bug is fallout from the wrapper changes. However, fixing it correctly requires understanding of what broke what, exactly. The reason that wrappers broke this is that before, we were running into the "chrome can do anything" codepath and SJOWs prevent that.
Comment 34 Boris Zbarsky [:bz] (TPAC) 2009-04-01 12:14:09 PDT
Hmm.  nsISecurityCheckedComponent might be simpler to do...
Comment 35 Blake Kaplan (:mrbkap) 2009-04-02 18:51:13 PDT
Created attachment 370765 [details] [diff] [review]
patch v1

So, this works except for the testcase in comment 29. The comment in the patch in SameOriginCheckeComponent::CanCreateWrapper says why.

What do you think?
Comment 36 Boris Zbarsky [:bz] (TPAC) 2009-04-02 19:11:02 PDT
Please use NS_strdup?

> If it isn't a double-wrapped object, then it definitely will
> +    // not have classinfo (and therefore won't be a DOM object). 

That should be "is a double-wrapped object", no?
Comment 37 Boris Zbarsky [:bz] (TPAC) 2009-04-02 19:11:57 PDT
Or do you mean that our JSObject is not a double-wrapped object?  I really can't tell what the double-wrapped part of this is trying to say.
Comment 38 Blake Kaplan (:mrbkap) 2009-04-03 17:31:43 PDT
Created attachment 370997 [details] [diff] [review]
Updated comment

Boris, does this make more sense?
Comment 39 Boris Zbarsky [:bz] (TPAC) 2009-04-03 19:35:33 PDT
Yeah, that comment is a lot clearer.  Still need NS_strdup.
Comment 40 Blake Kaplan (:mrbkap) 2009-04-06 16:46:18 PDT
Created attachment 371344 [details] [diff] [review]
Oops

Sorry, didn't see that in your first comment.
Comment 41 Johnny Stenback (:jst, jst@mozilla.com) 2009-04-09 17:58:54 PDT
Comment on attachment 371344 [details] [diff] [review]
Oops

This is somewhat scary, but let's get it in for beta4...
Comment 42 Blake Kaplan (:mrbkap) 2009-04-09 18:01:01 PDT
moz_bug_r_a4, can you poke at this to see if we're opening a gaping hole here?
Comment 43 moz_bug_r_a4 2009-04-11 04:11:49 PDT
I applied that patch and tested.  That patch does not fix this bug.  It seems
that CanCallMethod() returning "sameOrigin" does not work due to the same
reason as CanCreateWrapper().

http://mxr.mozilla.org/mozilla-central/source/caps/src/nsScriptSecurityManager.cpp#794
794             case nsIXPCSecurityManager::ACCESS_CALL_METHOD:
795                 checkedComponent->CanCallMethod(objIID,
796                                                 JSValIDToString(cx, aProperty),
797                                                 getter_Copies(objectSecurityLevel));
798             }
799         }
800     }
801     rv = CheckXPCPermissions(aObj, objectSecurityLevel);
Comment 44 John J. Barton 2009-04-14 11:44:29 PDT
I don't know of any workaround for this problem, other than disabling the Firebug features that trip on it.
Comment 45 Rob Campbell [:rc] (:robcee) 2009-04-14 12:10:58 PDT
(In reply to comment #44)
> I don't know of any workaround for this problem, other than disabling the
> Firebug features that trip on it.

that is the work-around.
Comment 46 John J. Barton 2009-04-14 14:08:22 PDT
But the purpose of disabling the feature is to allow us to find other FF3.5 problems. We need this bug to block 1.9.1.
Comment 47 Rob Campbell [:rc] (:robcee) 2009-04-14 15:39:03 PDT
(In reply to comment #46)
> But the purpose of disabling the feature is to allow us to find other FF3.5
> problems. We need this bug to block 1.9.1.

We're disabling the feature so we can find other bugs in FF3.5, yes. Also this allows us to be not completely broken on FF3.5. Not having a version of Firebug that works on 3.5 is not an option, even if that means disabling a few nice-to-have features like XHR responses in the Console.

This bug's still blocking 1.9.1, it's just not going to block beta 4. There is still a possibility that we won't have a fix for 3.5 final, so we have to keep that in mind and come up with fixes in firebug side if necessary.
Comment 48 Mark D. 2009-04-14 16:31:49 PDT
As a web developer, and a user of Firefox and Firebug, not having XHR responses anymore is a critical regression from past versions of Firefox. Fixing this issue is important for us to continue to develop modern JS websites in Firefox.

Developers that use firefox 3.5, firebug, and have the console tab enabled will suddenly have their ajax calls return nothing. No answer, no error message nothing; hopefully most of them will be able to trace it back here, and realize sooner than later that it's not a problem in their code, but a problem in firefox.
Comment 49 Rob Campbell [:rc] (:robcee) 2009-04-14 17:05:36 PDT
we will still have XHR responses in the Net panel.
Comment 50 Blake Kaplan (:mrbkap) 2009-04-14 20:39:35 PDT
I was also able to work around this by adding .wrappedJSObject to a couple of places in the example plugin. It wasn't pretty, but it seemed to work. I've lost exactly what I did, but I'll look into it again in a couple of days for you to try out.
Comment 51 Rob Campbell [:rc] (:robcee) 2009-04-15 07:19:35 PDT
thanks Blake. Keep us posted. Hopefully we can come up with something that isn't hideous and doesn't force us to drop features.
Comment 52 Mike Beltzner [:beltzner, not reading bugmail] 2009-04-15 07:22:18 PDT
So what's our resolution for B4? The goal is to have Firebug be compatible - should we be taking this off the P1 blocker list and asking FB to employ the workaround?
Comment 53 Rob Campbell [:rc] (:robcee) 2009-04-15 07:25:08 PDT
beltzner: My interpretation and hope for what we were going to do to maintain compatibility and still allow b4 to ship was to employ a workaround in Firebug for the time-being. We'd like this to remain on the blocker list for 1.9.1 final.
Comment 54 Mike Beltzner [:beltzner, not reading bugmail] 2009-04-15 13:11:00 PDT
--> P2 in order to block final release.
Comment 55 John J. Barton 2009-04-22 20:38:28 PDT
The weekly summary says for this bug:
— Won’t block on a fix here, Firebug workaround on hand instead of taking this fix. 

What we have is a disabled feature that many users expect and use.
Comment 56 Leon Sorokin 2009-04-25 20:20:13 PDT
+1 for blocking. I use console XHR daily :(
Comment 57 Blake Kaplan (:mrbkap) 2009-04-28 16:25:26 PDT
Created attachment 374960 [details] [diff] [review]
Fix for that

This builds on top of attachment 371344 [details] [diff] [review] -- it makes caps understand "sameOrigin" from nsISecurityCheckedComponent. See also the comments in nsScriptSecurityManager.h.
Comment 58 Boris Zbarsky [:bz] (TPAC) 2009-04-29 01:37:44 PDT
Comment on attachment 374960 [details] [diff] [review]
Fix for that

I'd just do s/subject has greater than or equal privileges to the object/subject subsumes the object/

Other than that, looks good.
Comment 59 Johnny Stenback (:jst, jst@mozilla.com) 2009-05-05 17:11:32 PDT
Comment on attachment 374960 [details] [diff] [review]
Fix for that

- In nsScriptSecurityManager::CheckXPCPermissions():

+            if (!aJSObject)
+            {
+                nsCOMPtr<nsIXPConnectWrappedJS> xpcwrappedjs =
+                    do_QueryInterface(aObj);
+                if (xpcwrappedjs)
+                {
+                    rv = xpcwrappedjs->GetJSObject(&aJSObject);
+                    NS_ENSURE_SUCCESS(rv, rv);
+                }
+            }
+
+            if (!aSubjectPrincipal)
+            {
+                // No subject principal passed in. Compute it.
+                aSubjectPrincipal = GetSubjectPrincipal(cx, &rv);
+                NS_ENSURE_SUCCESS(rv, rv);
+            }
+            if (aSubjectPrincipal)
+            {
+                nsIPrincipal* objectPrincipal = doGetObjectPrincipal(aJSObject);

If aJSObject comes in as null here, and the QI of aObj to nsIXPConnectWrappedJS fails, aJSObject will still be null here. A null check here seems like the right thing to do.

r=jst with that.
Comment 60 Blake Kaplan (:mrbkap) 2009-05-12 18:05:49 PDT
Created attachment 377057 [details] [diff] [review]
For checkin
Comment 61 info 2009-05-13 00:10:03 PDT
if (!aSubjectPrincipal)

6 lines below:
if (aSubjectPrincipal)

Does this kind of language don't understand the }else{ ?
Comment 62 Peter Van der Beken [:peterv] 2009-05-13 01:28:19 PDT
(In reply to comment #61)
> Does this kind of language don't understand the }else{ ?

            if (!aSubjectPrincipal)
                aSubjectPrincipal = GetSubjectPrincipal(cx, &rv);
            else

is not the same as

            if (!aSubjectPrincipal)
                aSubjectPrincipal = GetSubjectPrincipal(cx, &rv);
            if (aSubjectPrincipal)
Comment 63 info 2009-05-13 02:12:21 PDT
ah sorry, didn't see that 'aSubjectPrincipal' could be set in that first if statement.
Comment 64 Nochum Sossonko [:Natch] 2009-05-13 15:10:49 PDT
dev-doc-needed? (it's not clear to me if this introduces a new public [scriptable?] api of some sort...)
Comment 65 Blake Kaplan (:mrbkap) 2009-05-13 15:13:28 PDT
http://hg.mozilla.org/mozilla-central/rev/42832a65a579
Comment 66 Mike Beltzner [:beltzner, not reading bugmail] 2009-05-14 03:18:23 PDT
Regression: DHTML NoChrome increase 1.07% on Vista Firefox:
http://graphs-new.mozilla.org/graph.html#tests=[{%22machine%22:70,%22test%22:19,%22branch%22:1}]&sel=1242168480,1242341280

Previous results: 740.265 from build 20090513144241 of revision 9c62b2169f76 at 2009-05-13 14:42:00 on qm-pvista-trunk04 run # 0

New results: 748.176 from build 20090513154844 of revision 42832a65a579 at 2009-05-13 15:48:00 on qm-pvista-trunk04 run # 0

Suspected checkin range: from revision 9c62b2169f76 to revision 42832a65a579:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9c62b2169f76&tochange=42832a65a579

.. implicates this checkin.
Comment 67 Blake Kaplan (:mrbkap) 2009-05-14 15:12:42 PDT
I filed bug 493074 to track the performance regression. I'll try to get the patch in that bug in today to see if it helps, if not. I'll back the whole thing out.
Comment 68 Damon Sicore (:damons) 2009-05-21 18:21:17 PDT
Blake, since you aren't working on COW now, this one should be your priority.
Comment 69 Damon Sicore (:damons) 2009-05-21 18:22:45 PDT
To be clear, we should be doing whatever we can to find a fix to the perf regression.
Comment 70 Johnny Stenback (:jst, jst@mozilla.com) 2009-05-21 20:33:37 PDT
attachment 371344 [details] [diff] [review] backed out for mrbkap to see if that's the part that's causing the performance regression.

http://hg.mozilla.org/mozilla-central/rev/ed1f93938bf5
Comment 71 Johnny Stenback (:jst, jst@mozilla.com) 2009-05-21 20:34:48 PDT
And reopening, to make sure this doesn't get forgotten...
Comment 72 Damon Sicore (:damons) 2009-05-22 09:00:56 PDT
Is this in the whiteboard:

[firebug can work around temporarily, no longer blocks, but need for 1.9.1]

Still true?
Comment 73 John J. Barton 2009-05-22 09:15:50 PDT
We've disable the feature in Firebug to prevent the error message, allowing us to test the rest of Firebug on FF3.5.  We need this bug fixed so we can restore this important feature of Firebug.
Comment 74 Mike Shaver (:shaver -- probably not reading bugmail closely) 2009-05-22 09:29:11 PDT
Yes, we will fix this before shipping 3.5.
Comment 75 Rob Campbell [:rc] (:robcee) 2009-05-22 09:29:46 PDT
what they said.
Comment 76 Johnny Stenback (:jst, jst@mozilla.com) 2009-05-22 14:41:42 PDT
Created attachment 379247 [details] [diff] [review]
For 1.9.1.

This is a combination of the fix in this bug and the followup fix in bug 483672, which is what's been on the trunk the longest.
Comment 77 Johnny Stenback (:jst, jst@mozilla.com) 2009-05-22 15:42:03 PDT
Fix checked in for 1.9.1. Per discussion on irc with shaver and beltzner, we decided to take the small perf hit on the trunk rather than risking taking any new attempts at fixing it there as well.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a1b78429e99d
Comment 78 Johnny Stenback (:jst, jst@mozilla.com) 2009-05-22 16:01:48 PDT
Backed out part relanded, marking fixed again.

http://hg.mozilla.org/mozilla-central/rev/ba3b363f2ba1
Comment 79 Jan Honza Odvarko [:Honza] 2009-05-26 07:52:23 PDT
I have tested this with Firebug using following test case:
http://www.janodvarko.cz/firebug/tests/1586/Issue1586.htm

* It fails with Firefox 3.5pre (Permission denied for  to call method
UnnamedClass.handleEvent, chrome://firebug/content/spy.js (514))
* But passes with Firefox 3.6a1pre

Does that mean that it isn't fixed in 3.5pre, but will be in 3.5?
According to the https://bugzilla.mozilla.org/show_bug.cgi?id=483672#c74

Honza
Comment 80 Boris Zbarsky [:bz] (TPAC) 2009-05-26 08:01:08 PDT
This should be working with current 1.9.1 nightlies...  (anything after 2009-05-22).  If it's not, please remove the "fixed1.9.1" keyword from this bug?
Comment 81 Jan Honza Odvarko [:Honza] 2009-05-26 09:10:46 PDT
True, tested with 3.5pre (Gecko/20090526) and works.
Thanks!

Honza
Comment 82 Rob Campbell [:rc] (:robcee) 2009-05-28 13:42:07 PDT
also tested and verified working.
Comment 83 Jan Honza Odvarko [:Honza] 2010-03-22 07:04:56 PDT
I am reopening this one.

The test (from comment #79) stops working with 3.7a4pre
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a4pre) Gecko/20100322 Minefield/3.7a4pre (.NET CLR 3.5.30729)

Here is the test:
http://www.janodvarko.cz/firebug/tests/1586/Issue1586.htm
Follow instructions on the page.

Tested in a new profile with Firebug 1.6Xa8 installed
http://getfirebug.com/releases/firebug/1.6X/firebug-1.6X.0a8.xpi

Throws following exception:

onHTTPSpyReadyStateChange: EXCEPTION [Exception... "Security Manager vetoed action arg 0 [nsIDOMEventListener.handleEvent]"  nsresult: "0x80570027 (NS_ERROR_XPC_SECURITY_MANAGER_VETO)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no]

Related to the Firebug code mentioned in comment #0

The same test works when I test with Fx 3.6.3pre
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.3pre) Gecko/20100321 Namoroka/3.6.3pre (.NET CLR 3.5.30729)

I think this could be also related to bug 519901

Related Firebug issue:
http://code.google.com/p/fbug/issues/detail?id=2927

Is this known issue? Should I report a new bug for this?

Honza
Comment 84 Jan Honza Odvarko [:Honza] 2010-03-25 09:40:32 PDT
I am marking as blocking since this breaks Firebug 1.6 compatibility with Fx 3.7.
Honza
Comment 85 Jan Honza Odvarko [:Honza] 2010-04-02 04:42:03 PDT
I am setting some other flags to get more attention to this bug. According to Firebug's test harness, this is so far the only problem with Firebug 1.6 + Firefox 3.7a4pre combo.

As mentioned in comment #83 you can use following test page:
http://www.janodvarko.cz/firebug/tests/1586/Issue1586.htm

..to see the problem - and the exception in Firebug tracing console if ERRORS options is checked (note that you need 'X' build of Firebug, can be downloaded here: http://getfirebug.com/releases/firebug/1.6X/)

Tested with:
- Firefox 3.7a4pre, Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a4pre) Gecko/20100401 Minefield/3.7a4pre (.NET CLR 3.5.30729)
- Firebug 1.6a8

Could somebody please take a look at this?
I'll provide further info how to reproduce if needed, just ask me.

Thanks!
Honza





Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a4pre) Gecko/20100401 Minefield/3.7a4pre (.NET CLR 3.5.30729)
Comment 86 Reed Loden [:reed] (use needinfo?) 2010-04-02 04:50:49 PDT
(In reply to comment #85)
> I am setting some other flags to get more attention to this bug.

None of the changes you made would be noticed by drivers. If you want attention, you need to use the blocking1.9.x flags, which I have requested for you.
Comment 87 Jan Honza Odvarko [:Honza] 2010-04-02 05:08:16 PDT
Thanks Reed!
Honza
Comment 88 Daniel Veditz [:dveditz] 2010-04-02 13:41:22 PDT
Please don't abuse the flags by requesting blocking for branches this doesn't even apply to (comment 83). Different people triage the trunk, but the flags really don't mean much until later in the process. if you need it track people down.

(In reply to comment #83)
> I am reopening this one.

I don't think that's particularly helpful in this case. The patches here were checked in and still work for the earlier versions. If the symptoms came back it's probably a different problem (or the same problem in a new place?). Please file a separate bug for the trunk, something like "bug xxxx regressed bug xxxx" if you like.
Comment 89 Jan Honza Odvarko [:Honza] 2010-04-07 06:12:52 PDT
I see, I have reported:
https://bugzilla.mozilla.org/show_bug.cgi?id=557791

Daniel, could you please take a look at the new bug and check whether all flags are correctly set so it reaches proper people?

This issue is blocking Firebug 1.6 for Firefox 3.7

I am changing status of this one back to FIXED.

Thanks!
Honza

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