Closed
Bug 1140131
Opened 10 years ago
Closed 10 years ago
"###!!! ASSERTION: Recursive GetService!" error accessing exthandler service using xpcshell debugger in debug builds
Categories
(DevTools :: Debugger, defect)
DevTools
Debugger
Tracking
(firefox41 fixed)
RESOLVED
FIXED
Firefox 41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: markh, Assigned: benjamin)
Details
Attachments
(4 files, 4 obsolete files)
In a debug build, attempting to run xpcshell tests with --jsdebugger fails - see attachment. Works OK in release builds.
Assignee | ||
Comment 1•10 years ago
|
||
I see this as well, and have a C++ stacktrace attached.
XPConnect ReadSourceFromFilename("resource://gre/modules/Promise.jsm")
calls nsBaseChannel::Open
...-> nsFileChannel::MakeFileInputStream
-> nsExternalHelperAppService::GetTypeFromFile
getservice for @mozilla.org/uriloader/handler-service;1
which is JS, so we end up reentering
ReadSourceFromFilename("file:///home/bsmedberg/mozilla-hg/.../nsHandlerService.js")
which ends up reentering and causing the assert.
We've had this kind of problem consistently over the years where chrome loads which don't need MIME type information still end up triggering the external app service MIME type lookups. Is there a way we can just suppress that lookup for chrome loads?
There is a short-circuit for HasContentTypeHint, which leads to a mContentType check. Is there an obvious place to set that hint from xpconnect?
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(jimb)
Comment 2•10 years ago
|
||
Bug 1016301 would probably fix this problem by using native promises, but it has proved a tough nut to crack.
Comment 3•10 years ago
|
||
I have never looked at this code before, but:
Wouldn't it suffice to just call scriptChannel->SetContentType in ReadSourceFromFilename, before the call to scriptChannel->Open? That's only called from XPCJSSourceHook::load, which presumably knows it's loading JS.
Flags: needinfo?(jimb)
Assignee | ||
Comment 4•10 years ago
|
||
I tried comment 3. The problem remains, though the stack is different and more intelligible. We're recursively getting the addon manager integration service via this native and JS stack.
Assignee | ||
Comment 5•10 years ago
|
||
Jim, is that more helpful for finding the right owner?
Flags: needinfo?(jimb)
Comment 6•10 years ago
|
||
Hrm. A call to LoadModule causes the JS debugger's onNewSource hook to be invoked, as it should. However, then the JS debugger itself attempts to load a service, which triggers the recursion assertion.
If recursive service instantiation is really a fatal error, then the JS debugger must abstain from instantiating any services in its handler functions. Perhaps it could instantiate all the services it will possibly use before trying to debug anything?
As a matter of architecture, in-process debugger code must be very careful not to trigger things in the course of its own execution that disturb the debuggee. Loading services is just one example. This is true whether the debugger is implemented in C++ or JS, although people tend to be more careful about dependencies in C++.
Who owns the debugger code used by xpcshell's --jsdebugger?
Flags: needinfo?(jimb)
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to Jim Blandy :jimb from comment #6)
> Who owns the debugger code used by xpcshell's --jsdebugger?
Good question :) I did the xpcshell-specific implementation, but don't consider myself the owner of it. Although I'm sure the devtools team doesn't want ownership either, so I'm probably the closest approximation.
The following simple patch solves the problem, but I'm not sure whether this is the correct place for it, or whether it should be in the xpcshell actor (testing/xpcshell/dbg-actors.js)? I'm inclined to say it should be part of devtools itself for the reasons mentioned:
(In reply to Jim Blandy :jimb from comment #6)
> As a matter of architecture, in-process debugger code must be very careful
> not to trigger things in the course of its own execution that disturb the
> debuggee. Loading services is just one example. This is true whether the
> debugger is implemented in C++ or JS, although people tend to be more
> careful about dependencies in C++.
Jim, what's your take here? (Or alternatively, can you transfer the NI? to someone else?)
The patch is ugly, but relatively small and effective...
> --- a/toolkit/devtools/server/actors/script.js
> +++ b/toolkit/devtools/server/actors/script.js
> @@ -39,6 +39,14 @@ let TYPED_ARRAY_CLASSES = ["Uint8Array", "Uint8ClampedArray", "Uint16Array",
> // collections, etc.
> let OBJECT_PREVIEW_MAX_ITEMS = 10;
>
> +// See bug 1140131 - we shouldn't instantiate services while we are debugging,
> +// so instantiate the ones we care about now.
> +for (let cid of ["@mozilla.org/addons/integration;1",
> + "@mozilla.org/mime;1",
> + "@mozilla.org/uriloader/handler-service;1"]) {
> + Cc[cid].getService(Ci.nsISupports);
> +}
> +
Flags: needinfo?(jimb)
Comment 8•10 years ago
|
||
I guess something like that is the best fix for now. However, please make the comment stand on its own, rather than citing the bug, as a courtesy to your future readers.
Flags: needinfo?(jimb)
Reporter | ||
Comment 9•10 years ago
|
||
Great, thanks.
Comment 10•10 years ago
|
||
Does this still happen after bug 1149910? We're now using native promises in the place that I believe pertains to this issue.
Reporter | ||
Comment 11•10 years ago
|
||
(In reply to Panos Astithas [:past] from comment #10)
> Does this still happen after bug 1149910? We're now using native promises in
> the place that I believe pertains to this issue.
It happened for me yesterday on a current fx-team build, and it looks like that other bug landed 6 days ago, so I think it does.
Reporter | ||
Comment 12•10 years ago
|
||
(I guess we could also open an xpcom bug - recursive service instantiation shouldn't be a problem for distinct services.)
Comment 13•10 years ago
|
||
Comment on attachment 8598466 [details] [diff] [review]
0008-Bug-1140131-pre-instantiate-all-xpcom-services-used-.patch
Review of attachment 8598466 [details] [diff] [review]:
-----------------------------------------------------------------
It's been so long since I've worked on script.js that I don't feel comfortable making architectural decisions for them any more. I've transferred review to jlongster, who is in a better position to make these decisions.
Attachment #8598466 -
Flags: review?(jimb) → review?(jlong)
Comment 14•10 years ago
|
||
Comment on attachment 8598466 [details] [diff] [review]
0008-Bug-1140131-pre-instantiate-all-xpcom-services-used-.patch
Review of attachment 8598466 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/devtools/server/actors/script.js
@@ +44,5 @@
> +// debug builds (ASSERTION: Recursive GetService!). We solve this by
> +// pre-creating all services we need while debugging.
> +for (let cid of ["@mozilla.org/addons/integration;1",
> + "@mozilla.org/mime;1",
> + "@mozilla.org/uriloader/handler-service;1"]) {
The only services I see instantiated within script.js are these:
@mozilla.org/eventlistenerservice;1
@mozilla.org/chrome/chrome-registry;1
@mozilla.org/network/protocol;1?name=resource
Where did you get that list?
Reporter | ||
Comment 15•10 years ago
|
||
(In reply to James Long (:jlongster) from comment #14)
> Where did you get that list?
I instrumented GetService to print the name of the services being recursively instantiated, add added each one until no more were reported and the assertion didn't fire.
Comment 16•10 years ago
|
||
I believe at least one of them is used by map-uri-to-addon-id.js.
Comment 17•10 years ago
|
||
Oh, so the main problem is modules that are lazily loaded. Looks like some of them are lazily loaded for things like the browser console and addon debugger, which the normal user doesn't need.
In fact, loading these services eagerly probably defeats the purpose of these lazy loaders. The whole point of lazy loading is to optimize startup time so I worry about doing this.
I only have vague concerns though, past am I correct? I'd also like fitzgen's thoughts since he's worked a lot on this.
Updated•10 years ago
|
Flags: needinfo?(nfitzgerald)
Comment 18•10 years ago
|
||
For example, this it the top of script.js:
loader.lazyRequireGetter(this, "mapURIToAddonID", "devtools/server/actors/utils/map-uri-to-addon-id");
That module loads the "@mozilla.org/addons/integration;1" service.
`mapURIToAddonID` is called in the `_mapSourceToAddon` method called from the `SourceActor` constructor. It's only called if the source URI maps to a local file, which I'm assuming happens in the browser toolbox. That module will be loaded when the browser toolbox debugger is used, I think, and seems to replicate the cause of the error here.
But we lazily load for a reason, is loading these services eagerly hurting our startup time?
Comment 19•10 years ago
|
||
First of all, let's not let optimizations stand in the way of correctness.
The goal was to prevent loading a bunch of compartments and bloating memory when the actors are loaded, but the debugger isn't even being used.
As I understand it, the bug here is that we are loading new services while loading a service (which sounds oddly familiar to me... I think I've hit this before...) because we get an onNewScript for the first service and then access some getter that forces a lazily loaded service to be instantiated.
Is there a time earlier than onNewScript that we can load the services, but not so early that it isn't clear whether or not the debugger is even being used? I think so: when we are attaching to the thread actor. I think we can simply force instantiation of our lazily loaded services in ThreadActor.prototype.onAttach with some helper function forceLoadLazyModules or something. Or even just stop lazily loading them, and just do the loading at attach time.
Comment 20•10 years ago
|
||
(In reply to James Long (:jlongster) from comment #18)
> But we lazily load for a reason, is loading these services eagerly hurting
> our startup time?
I believe it was mostly memory bloat, but I wouldn't be surprised if it slowed down debugger server startup time as well.
Flags: needinfo?(nfitzgerald)
Comment 21•10 years ago
|
||
I'd really like to track down the actual module loading these services and force load them instead in the `attach` event if we don't have them yet. Mark, can you think of any way to figure out where they are loaded from? I'm grepping the entire devtools codebase and I don't see "@mozilla.org/uriloader/handler-service;1" at all. I only see the mime service in `browser/devtools/projecteditor/lib/stores/resource.js` which looks unrelated.
"@mozilla.org/addons/integration;1" does seem to be loaded in map-uri-to-addon-id.js. What about the other 2?
Reporter | ||
Comment 22•10 years ago
|
||
(In reply to James Long (:jlongster) from comment #21)
> I don't see
> "@mozilla.org/uriloader/handler-service;1" at all.
That one is subtle - nsFileChannel::MakeFileInputStream() calls into nsExternalHelperAppService::GetTypeFromFile(), which ends up instantiating both the mime and that "handler" service. Bug 1100069 has a little more context on that (that bug fixed warning messages in the console in the same situation) - also found working on xpcshell debugging :)
This patch now defines a const array of services we need to instantiate at the top-level, but only instantiates them in onAttach(). Does this look reasonable?
Attachment #8598466 -
Attachment is obsolete: true
Attachment #8598466 -
Flags: review?(jlong)
Attachment #8599734 -
Flags: feedback?(jlong)
Comment 23•10 years ago
|
||
(In reply to James Long (:jlongster) from comment #17)
> I only have vague concerns though, past am I correct?
I also had the same vague concerns and no better ideas, but I think Nick's idea is a good one.
Now that I've thought about it some more though, I wonder if we could avoid the force-loading altogether, by making map-uri-to-addon-id smarter:
https://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/utils/map-uri-to-addon-id.js#18
This comment indicates that the code isn't supposed to work in xpcshell anyway, so we try, fail, and ignore the error. What if we could just do a check for "is this XPCSshell?" and avoid instantiating the addon manager service and its dependencies during our doomed attempt in that try block?
We already have a need for this in the thread actor:
https://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/script.js#1545
A similar check seems plausible here, although I think the ideal way to deal with this would be to add something like the following to testing/xpcshell/head.js:
https://dxr.mozilla.org/mozilla-central/source/browser/experiments/test/xpcshell/head.js#205
Comment 24•10 years ago
|
||
Panos that sounds great, but there's 2 other services we need to load also. File a bug to clean up map-uri-to-addon-id.js if you want, and hopefully in the future we don't need this patch, but for now I don't see any way around it...
Comment 25•10 years ago
|
||
Comment on attachment 8599734 [details] [diff] [review]
0007-Bug-1140131-pre-instantiate-all-xpcom-services-used-.patch
Review of attachment 8599734 [details] [diff] [review]:
-----------------------------------------------------------------
This is definitely better, but we're going to look at this code in a few months and wonder what it's for... The problem is mainly the primitive GetService check, right? We aren't actually recursively loading services, so we should be fine, but this is necessary because we don't actually really check for circular references, which is what we are trying to do?
Two things: can you link this bug in the top-level comment, and also file a bug to improve the GetService check? I'd love to have plans to remove this code at some point in the future.
Attachment #8599734 -
Flags: feedback?(jlong) → feedback+
Reporter | ||
Comment 26•10 years ago
|
||
(In reply to James Long (:jlongster) from comment #25)
> This is definitely better, but we're going to look at this code in a few
> months and wonder what it's for... The problem is mainly the primitive
> GetService check, right? We aren't actually recursively loading services, so
> we should be fine, but this is necessary because we don't actually really
> check for circular references, which is what we are trying to do?
I was wrong about that - the service manager already has a check for *this* service being recursively initialized rather than a simple *any* service. IOW, we *are* recursively loading individual services.
Looking a little deeper, we don't need to pre-instantiate "@mozilla.org/mime;1" - there's no problem with that one.
"@mozilla.org/uriloader/handler-service;1" is recursively instantiated via the following JS stack:
> 0 isHiddenSource(aSource = [object Source]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js":779]
> 1 TabSources/this.allowSource(source = [object Source]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js":32]
> 2 ThreadActor.prototype._addSource(aSource = [object Source]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js":2039]
> 3 ThreadActor.prototype.onNewScript(aScript = [object Script]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js":2007]
> 4 isHiddenSource(aSource = [object Source]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js":779]
> 5 TabSources/this.allowSource(source = [object Source]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js":32]
> 6 TabSources.prototype.source( = [object Object]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js":113]
> 7 TabSources.prototype.createNonSourceMappedActor(aSource = [object Source]) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js":308]
> 8 TabSources.prototype.createSourceActors/<(actors = null) ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js":351]
> 9 EventLoop.prototype.enter() ["resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js":382]
[IT seems DumpJSStack() only shows 10 entries, or somehow has lost the others]
Another (hacky) way to solve this one is to have isHiddenSource() grow a recursion check, but that's ugly and probably fragile - ISTM anyone invoking the .text getter is at risk.
The addon-manager is recursively instantiated via:
> ".get@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/map-uri-to-addon-id.js:21:35"
> "mapURIToAddonID@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/map-uri-to-addon-id.js:46:10"
> "SourceActor.prototype._mapSourceToAddon@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:2380:21"
> "SourceActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:2293:3"
> "TabSources.prototype.source@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js:154:17"
> "TabSources.prototype.createNonSourceMappedActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js:308:12"
> "ThreadActor.prototype._addSource@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:2059:23"
> "ThreadActor.prototype.onNewScript@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:2017:7"
> ".get@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/map-uri-to-addon-id.js:22:20"
> "mapURIToAddonID@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/map-uri-to-addon-id.js:46:10"
> "SourceActor.prototype._mapSourceToAddon@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:2380:21"
> "SourceActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:2293:3"
> "TabSources.prototype.source@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js:154:17"
> "TabSources.prototype.createNonSourceMappedActor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js:308:12"
> "TabSources.prototype.createSourceActors/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js:351:19"
> "Promise*TabSources.prototype._createSourceMappedActors@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js:324:1"
> "TabSources.prototype.createSourceActors@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/utils/TabSources.js:350:12"
> "ThreadActor.prototype._discoverSources@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:1376:17"
> "ThreadActor.prototype.onSources@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:1381:12"
> "DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1456:15"
> "DebuggerTransport.prototype._onJSONObjectReady/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/transport/transport.js:471:9"
> "makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:82:14"
> "makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:82:14"
> "EventLoop.prototype.enter@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:383:5"
> "ThreadActor.prototype._pushThreadPause@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:589:5"
> "ThreadActor.prototype.onAttach@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/script.js:710:7"
> "DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1456:15"
> "DebuggerTransport.prototype._onJSONObjectReady/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/transport/transport.js:471:9"
> "makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:82:14"
> "makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:82:14"
> "_initDebugging@o:\\src\\mozilla-git\\gecko-dev\\testing\\xpcshell\\head.js:456:5"
> "_execute_test@o:\\src\\mozilla-git\\gecko-dev\\testing\\xpcshell\\head.js:468:7"
So best I can tell, there's not an xpcom bug in either case. As :past mentions though, we could probably avoid requiring the addon manager for xpcshell but that also seems fragile - ISTM that non-xpcshell tests will have the exact same issue if that service isn't instantiated yet (eg, imagine a future where we can debug the browser startup code)
James, given the above, what are you thoughts on the best way to fix this? Stick with the original patch (minus the mime service) with a comment pointing at this bug, or something else?
[Note too that this isn't particularly urgent - debugging xpcshell tests in debug builds isn't that common for me. Also note this is trivial to reproduce - in a debug build, "./mach xpcshell-test --jsdebugger any/test/name.js", connect to it, switch to the debugger tab, and bang!]
Flags: needinfo?(jlong)
Comment 27•10 years ago
|
||
I guess I'm wondering why it doesn't cause problem in release; the assert fails and if we are actually recursively loading modules shouldn't we see some side effect, like the debugger not loading or caught in an infinite loop?
Thanks for digging into the places that trigger this, that will help a lot when coming back to this bug from the comment. I'm find landing the patch to preload those 2 services as long as it has a comment to this bug.
Flags: needinfo?(jlong)
Reporter | ||
Comment 28•10 years ago
|
||
Just the 2 services and the comment tweaked to include this bug number.
Attachment #8599734 -
Attachment is obsolete: true
Attachment #8601201 -
Flags: review?(jlong)
Reporter | ||
Comment 29•10 years ago
|
||
(In reply to James Long (:jlongster) from comment #27)
> I guess I'm wondering why it doesn't cause problem in release; the assert
> fails and if we are actually recursively loading modules shouldn't we see
> some side effect, like the debugger not loading or caught in an infinite
> loop?
In release builds we still fail, but don't abort (ie, the recursive getService() call fails with an error). For the 2 services in question, this failure is benign - the exthandler one just means that the mime-type for the file isn't going to be known (but we don't use it). For the addon one, map-uri-to-addon-id.js will "cache" a null service so isn't going to function correctly - but in this scenario we don't need it anyway.
So yeah, the failure is quite benign. We could *almost* argue the xpcom bug is the fact it aborts in a debug build, but I doubt there'll be much appetite for that change.
Comment 30•10 years ago
|
||
Comment on attachment 8601201 [details] [diff] [review]
0004-Bug-1140131-pre-instantiate-some-xpcom-services-used.patch
Review of attachment 8601201 [details] [diff] [review]:
-----------------------------------------------------------------
thanks!
Attachment #8601201 -
Flags: review?(jlong) → review+
Comment 32•10 years ago
|
||
Reporter | ||
Comment 33•10 years ago
|
||
Same patch, but with the service instantiation reading:
// now's a good time to instantiate the services we need.
if (Cc) { // might not be in a chrome context...
REQUIRED_SERVICES.map(cid => Cc[cid].getService(Ci.nsISupports));
}
A trivial change but I felt it worth running past again.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=87adc566bb9a
Attachment #8601201 -
Attachment is obsolete: true
Attachment #8603175 -
Flags: review?(jlong)
Updated•10 years ago
|
Attachment #8603175 -
Flags: review?(jlong) → review+
Comment 35•10 years ago
|
||
Backed out in https://hg.mozilla.org/integration/fx-team/rev/5177b647d87a
e10s dt does run on try, they are those red ones with the same failures in your try push as the failures on the first landing.
Assignee | ||
Comment 36•10 years ago
|
||
Bug 1140131 - When XPConnect loads scripts (JS components or Cu.import), set the MIME type so that we don't load the exthandler service to guess it, r?bholley
Attachment #8613049 -
Flags: review?(bobbyholley)
Comment 37•10 years ago
|
||
Comment on attachment 8613049 [details]
MozReview Request: Bug 1140131 - When XPConnect loads scripts (JS components or Cu.import), set the MIME type so that we don't load the exthandler service to guess it, r?bholley
https://reviewboard.mozilla.org/r/9703/#review8513
::: js/xpconnect/src/XPCJSRuntime.cpp:3229
(Diff revision 1)
> + scriptChannel->SetContentType(NS_LITERAL_CSTRING("text/plain"));
> +
Please add a comment here that this avoids going through the exthandler service.
Attachment #8613049 -
Flags: review?(bobbyholley)
Comment 38•10 years ago
|
||
Comment on attachment 8613049 [details]
MozReview Request: Bug 1140131 - When XPConnect loads scripts (JS components or Cu.import), set the MIME type so that we don't load the exthandler service to guess it, r?bholley
https://reviewboard.mozilla.org/r/9703/#review8515
Ship It!
Attachment #8613049 -
Flags: review+
Comment 39•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Reporter | ||
Updated•10 years ago
|
Attachment #8603175 -
Attachment is obsolete: true
Reporter | ||
Comment 41•10 years ago
|
||
Thanks Benjamin, that is certainly a cleaner way of managing the exthandler service. However, it still leaves the @mozilla.org/addons/integration;1 service, so I still get the same failure.
FTR, my original patch caused failures on try that I never got to the bottom of, but now that the only problem is the the addons service, I think we can do something like :past suggested in comment 23. Rather than reopening this bug, I'm about to clone it for that problem.
Assignee: markh → benjamin
Summary: "###!!! ASSERTION: Recursive GetService!" error using xpcshell debugger in debug builds → "###!!! ASSERTION: Recursive GetService!" error accessing exthandler service using xpcshell debugger in debug builds
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•