The point of the extensions/ directory was to put optional things there. If you attempt to build the browser with that directory missing, it will crash. If nsIWalletService.h & nsICookieService.h are moved into another directory (say mozilla/include/), the browser builds fine without the extensions directory. So, the question is...where do we move these header files? They are needed to provide the interface to Mozilla but the directories that they are currently in are not needed.

I nominate netwerk/base/public or netwerk/protocol/http/public for nsICookieService.idl. The wallet interface should probably live somewhere like layout/html/forms/public, but I'm not as sure about that.

Actually, this is the first I've heard that the browser should be able to build with the extensions directory missing. Is this really a requirement? In any case, I'm deferring this to Brendan since he was the original advocate of creating an extensions when we were looking for a home for wallet.

I can only repeat what cls wrote: "The point of the extensions/ directory was to put optional things there." Extension and optional have pretty clear and, in most software, related meanings. For example, all Emacs versions I know of can be built and run without the many extensions that might be bundled as source and even binary (.fasl, whatever) optional added parts (which is to say, as extensions). This should be fixed, in particular it seems to me that wallet hookup from layout code should use a public API purveyed from layout. I'm not the person to fix this, though. Steve, now that I've restated the purpose of extensions, will you fix it? If not, please bounce to dp. /be

This has nothing to do with layout -- the dependency with layout was removed a long time ago. I just investigated and the cause of this dependency is the fact that wallet and cookie are initialized in the webshell. So this is really a duplicate of bug 14889. *** This bug has been marked as a duplicate of 14889 ***

Even if the initialization of the cookie & wallet components is moved from webshell to somewhere under necko (according to <a href="http://bugzilla.mozilla.org/show_bug.cgi?id=17120">bug 17120</a>), the header files will still need to be moved.

That's not the case. The latest thinking on bug 14889 is the following (quoting from a message from Brendan): "have the app, at startup, add a tiny observer for the Cookie: header, but don't link in or otherwise spend cycles on getting the Cookie service and initializing it. Do all that from the observer when it actually sees the first header. The observer that gets the Cookie service and calls into it can be a tiny part of the browser app." So this tiny observer is a built-in part of the browser and will not be in the extensions file. It then accesses the cookie dll through the xpcom interface. So the header files for the cookie dll will remain in the extensions directory and will not cause any problems. Wallet will be handled in a similar manner.

Ok, I'm confused. Isn't nsICookieService.h needed to define the cookie interface for the tiny observer? If so, isn't it (part of the main browser) still going to need to look under extensions/ for that header? Or do you plan to redefine those interfaces in the tiny observer?

I guess all this still needs more thought. So I'll keep this bug open and distinct from 14889 just to make sure both issues get addressed.

Reassigning to dp since this he can fix this at the same time that he fixes bug 14889.

I dont think we will fix this goodness before shipping Netscape 6

it really depends on who you talk to, dp. I'll see if i can help at all to make extensions really *mean* extensions.

mass re-assign of all bugs where i was listed as the qa contact

C'mon, guys, this is just getting worse. Now you can't build libimg without a dependency on the cookie ``extension'', because it has a hard-coded dependency on nsICookieService and such. (Since if.cpp 3.46, March 19, morse@netscape.com, no reviewer listed (!).) (To say nothing of the fact that images have nothing to do with cookies, but I'll file another bug about the interface-scope-creep issue later.) It's one thing to say ``I don't have time to go back and fix the existing brokenness'' but it's another entirely to just keep piling the brokenness on. What gives?

For the record, the way to fix this was moving cookie (not cookie manager, the UI) into necko. We have concensus on doing that with the necko group.

Are you going to move the non-cookie-related API nonsense for images in there as well? Seems like that stuff should really be in a separate interface, seeing as how image loading and cookie disposition are pretty much unrelated. Also, will the cookie policy stuff no longer be pluggable? There is interest in some circles WRT replacing the default Mozilla cookie policy with something more flexible, or integrated with other pieces of software, so I'd be a little saddened to hear that we were just going to pile this stuff into necko, rather than doing the legwork originally implied by the extensions/ location. (Also, will we still have to build wallet to get the UI for managing the cookie policy? That seems like another case of conflation that is better undone than relocated.)

I dont know much about the image manager. It is a recent addition. Anyway, eventhough we are going to move the cookie code into necko, it will be triggered by cookie notify using categories. So an external cookie module can implement its own cookie notify and replace the existing CID under the http-startup category and they will be in business. Moving it wont impose any new restrains. It will save us from one more dll. ok ?

Embedding needs imagelib, but it certainly doesn't need cookies. Nominating for nsbeta3, and cc'ing valeski.

Only embedding, no help for seamonkey.

Putting brackets in - from nsbeta3- to [nsbeta3-] to get jud's query to work

Fuck it. I'm going to whack extensions/ so hard it'll spin. (Though I won't go out of my way to break them, I don't especially care if wallet or cookie manager build and work when I'm done: people who care about making them interface with Mozilla have had ample time to clean this mess up. I just care that we don't need the extensions/ stuff to build Mozilla, because that's the contract of the directory, and lots of people want to build without this code for a variety of reasons. I guess someone who really cares about the current wallet and cookie management code can fix that stuff in the MFuture.)

When I say ``whack extensions/'', I really mean ``whack all the things with bullshit dependencies on extensions/, and which are not themselves extensions''. Sorry for the confusion.

As one step towards this, I have some changes to abstract out our security/crypto interface. What I'm trying to do is provide interfaces for ssl and crypto functions in a way not specific to PSM. We can then export these headers from some location other than extensions/psm-glue (I'm using security/base), thus not making any part of the tree explicitly dependent on PSM. Since patch seems to have problems patching Makefile.in's in multiple directories, I've split this up into: extensions.psm-glue.patch extensions.psm-glue.public.patch extensions.psm-glue.src.patch security.patch mozilla.patch (which has the rest of the changes) security-base.tar.gz (contents of security/base) I'm rolling all of these into a tarball which I'll attach next.

(note: bugzilla doesn't indicate it very well, but the above attachment should be saved as a .tar.gz file)

Bob, could someone on your team comment (soon!) on bryner's approach? I like it, a lot, and I'd like to get it in soon so that we can remove dependencies on PSM. Right now, I'm working on refactoring the wallet interfaces into ``form management'' and ``password management''. The former will live in layout/public, and the latter in netwerk/public. That'll let us disentangle the build-time wallet dependency from the layout and networking code, for starters. M18 is going to be a close thing, but it's not like anyone actually cares about this. =/

See bug http://bugzilla.mozilla.org/show_bug.cgi?id=46872 Everything else should have been abstracted enough to replace cartman.

What do you mean by ``replace cartman''? There are references to nsIPSMSocketInfo in the core networking code, which comes from extensions/psm-glue. I'm actually not as worried about being able to replace cartman as I am about being able to build the system without anything that purports to be an ``extension''. (Though bryner's patch makes the replacing of cartman pretty clean, too, for those who would be motivated to do so.)

bryner: very slick clean-up, r,a=brendan@mozilla.org Module owners should give a timely r=, or this patch will go in without their explicit review. It fixes an ownership lapse, namely: making non-extension code depend on extensions/psm-glue. It's not the end of the world, so owners should not get all defensive; but it is a mistake, and bryner's patch rectifies it as far as I can tell. Same spiel for wallet and cookie dependencies from core/embeddable/non-extension parts of the codebase. /be

Unless there are objections from module owners, I'm going to check this in tomorrow evening (Saturday).

Adding a couple random psm people to CC-- Do you guys have any objection to glue code going in under mozilla/security/base?

shaver's patch looks good to me. r=valeski

The above patch adds nsIPasswordManager and such to netwerk/base/public, because we use passwords for accessing network content, and I believe that netwerk should define and publish the interface through which it is controllable/extendable/pluggable, rather than having a compile-time dependency on wallet/. I have not yet adapted wallet to the new interface; I'll do that, if at all, when all the other, similar changes are in place and I can do one big update.

I've got a bunch of new patches coming (mailnews, xpfe, imglib, etc.), and some questions to go along with the phase after that: * OJI guys: does the OJI plugin directly call the cookie service, or does it use the PluginHost's cookie interface? If you tell me it's the latter, I'll be very happy * Ben and the appcore police: the only remaining dependency on wallet that's in the core code is in the nsBrowserInstance appcore. Can I get a hand from someone to whack that thing? Should I just tear out the wallet stuff, and have the wallet UI retrofitted to use overlays for menu item addition, and modern XPConnect techniques to talk to the service itself? The latter is very tempting indeed. * Brendan: are you still interested in adapting the cookie manager to use my politically-correct nsIContentPolicy hooks? My patches trim out libimg's cookie manager calls (though somewhat superficially), so they'll be needed if we want image-filtering to survive.

We'll want image filtering to survive, hooked up via a proper XPCOM interface and not a #include. Let me know when I should apply the latest consolidated patch and get busy. /be

At the moment, OJI has no dependencies on cookie.

edburns: Thank you for the great news. Brendan: you can start making the cookie manager (sigh) do its image-filtering through the nsIContentPolicy at your leisure; the hooks needed for that have been in the tree for a while, awaiting your tender loving care. Bryner/Pavlov: I need your security stuff in the tree to make sure that this is all kosher. Can I get an ETA?

My first patch above (for netwerk) is in the tree. More patches coming ASAP.

The parade continues. mailnews is more of the same as netwerk, really. xpfe is removal of some wallet/cookie stuff, though the Big Appcore Mess lives. netwerk is an updated interface, to pull in the prompting stuff. alecf, jud, ben: I await your r. brendan, I'd like some more a. (The wallet and cookie functionality will be somewhat-to-completely broken until this surgery -- and the work to make the wallet code use new interfaces -- is complete. Stay tuned!)

pnunn, alecf, tor: welcome to the party! BYOr=.

The plan as far as nsIBrowserInstance sounds fine to me. Several of us have been wanting to kill that for some time now and the wallet methods are among some of those in the way of that effort. Ditto the overlaying mechanism. If possible, the methods should be shifted into some wallet interface, and they can be called from overlayed UI.

Patch coming up to stub out those methods, for starters. I think those methods should probably migrate over to the ever-shrinking nsIWalletService, but I'm not yet planning how to put the wallet pieces back together.

Hey brendan: can I get a #46423-style blanket approval for this work. (Not that I see any mention of approval at all in that bug, but I'll be charitable and presume it's been granted -- by Netscape marketing, natch! -- as per tinderbox.)

No blanket approvals yet, especially if there is a conflict between finishing this overdue cleanup, and a contributor making a branch for a commercial product that wants to include the "feature", implemented however. Either we restore the broken wallet functionality fast, or someone (shaver should help, at least) must back out the partial cleanup from the netscape.com branch. /be

What netscape.com branch? I'm up-to-date with my .seamonkey, and see no mention of any such thing, though I've heard bits of back-room plans about it. If netscape.com wants my help backing out these changes on their branch, I'll happily drive bonsai for them. I have to admit that I'm surprised to have heard only now, from brendan@mozilla.org, about netscape.com's wallet concerns, when there are a dozen netscape.com people on this bug, several of whom have expressed their support for this plan, and none of whom have objected. I know that the embedding crowd tried to nominate this for nsbeta3, and some of them have (privately) expressed their support for this, so I'm having a hard time getting an understanding of what netscape.com wants. Does someone @netscape.com want to comment here?

we need to permit building w/ out the extensions dir (as the bug summary indicates). This should be done w/ out breaking existing functionality IMO.

``Without breaking'' is a pretty tall order, but ``with repair afterwards'' isn't too bad at all. Are people willing to help with that? I know brendan has signed up to fix the cookie manager/image-blocking stuff. The only other big pieces of the repair are: - change wallet UI to call nsIWalletService directly, rather than using the (soon-to-be-elided) appcore hooks - teach wallet about the new interfaces (2 hours work, tops) - move wallet into overlays, like other optional components I think that's it. I'll even do it all, if people insist, as long as people can put up with my travel schedule's imposition of delays.

No, dammit, we're trying to ship something here. "With repair afterwards" would be perfectly fine months ago or in a week or two after Netscape branches. Why right now? This was not so broken that we needed this kind of destabalization this week. So now what--is it easier to go forward or go back? Your comments about the remaining tasks and your travel schedule were not reassuring compared to the ease of getting bonsai to spit out the cvs commands to back this stuff out.

If I could go back in time to several months ago, I would certainly do so, but I can't. Why not after you branch, you ask? Are you here to tell us when that will be, and under what terms? I know nothing concrete about netscape.com's branch plans -- only rumour and speculation -- but I _do_ know that many people within the Mozilla community, including some of your netscape.com coworkers, want this bug fixed to serve their needs. When will Netscape branch? For how long? I want to write something here about ``we're trying to ship'' vs. what I see every day being piled into the tree by netscape.com folk with weak review and weaker (IMO) approval, but I couldn't find anything that communicated how infuriated, frustrated and saddened I am, and yet didn't contain too much profanity. So you'll just have to use your imagination.

Mike: You're well aware of bug 46859 which removes dependencies on nsIWalletService, right? The diffs are in there -- just waiting for someone to verify it.

I'm reading the diffs for that now, Warren, thanks for the heads-up. I still see lots of PRUnichar * prompt_string = Wallet_Localize("PromptForPassword"); in the psm-glue code -- does that not have a wallet dependency? More comments in that bug, about that diff, coming.

Ok, I didn't remove all wallet dependencies, just the ones in necko (ForgetPassword). The idea here was to make it transparent whether wallet was in the chain or not between you and the dialog code.

shaver: we are completely with you on what the right fix is here. The issue was just about the timing that didn't turn out to be so good since we are really pressed for time (and stability) on the PR3 deadline. While we all from the bottom our heart appreciate the work you and several mozilla contributors are doing, we have to still "do the right thing" for weighing fixes against the risk. And as it turns out in this case, we clearly had regression. Based on that alone I have to request you to back this out. Thanks for understanding and for contributing a much needed cleanup of the codebase.

Trying to build moz, I keep on hitting this bug: ../../../../../mozilla/extensions/psm-glue/public/nsIPSMComponent.idl:27: can't open included file nsISecurityManagerComponent.idl for reading input callback returned failure make[3]: *** [_xpidlgen/nsIPSMComponent.h] Error 2 make[3]: Leaving directory `/mnt/ext2/1/STAGE/mozilla-cvs/BUILD/mozilla/extensions/psm-glue/public' make[2]: *** [export] Error 2 make[2]: Leaving directory `/mnt/ext2/1/STAGE/mozilla-cvs/BUILD/mozilla/extensions/psm-glue' make[1]: *** [export] Error 2 make[1]: Leaving directory `/mnt/ext2/1/STAGE/mozilla-cvs/BUILD/mozilla/extensions' make: *** [export] Error 2 I think the most recent update (http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/extensions/psm-glue/public&command=DIFF_FRAMESET&file=nsIPSMComponent.idl&rev1=1.6&rev2=1.7&root=/cvsroot) for nsIPSMComponent.idl is fishy. There is no nsISecurityManagerComponent.idl to be found anywhere in CVS... The bug occurs when building with default extensions as well as when including all extensions.

Are you using client.mk to check out?

No, I do a full SeaMonkeyAll checkout, and an update in the created mozilla tree. I build in an objdir (../BUILD/mozilla). This used to work flawlessly up until a couple of days ago...

If it did work, it was pure luck. See http://www.mozilla.org/build/unix.html for the proper check out rules that have been in effect for over a year(?) now.

Shaver: I think I picked a nit about copy/paste codebloat in the paragraph or so needed to get from nsIURIs to the strings needed for shouldLoad or shouldProcess already. Now over in bug 28327, mstoltz points out that he's adding yet more nsIScriptSecurityManager::CheckLoadURI calls, and it looks like nsIContentPolicy eats that interface method's dust. Can we not consolidate code now that we have a few concrete cases to abstract? Should we combine services, or make a super-service that handles security and privacy decisions? /be

I agree; it would make sense to combine the security and privacy aspects of content loading policy, so we aren't making two calls before every load. I'm working on a rewrite of CheckLoadURI right now, so it's a good time to do this. Can you suggest ways to improve the performance of this function? Something Vidur mentioned about these security checks: as it stands, we have to find everywhere in the code where a network load is initiated by web content (basically, everything but the initial page load). The safer way to do this would be to put the check as low as possible, say in the AsyncRead calls which always occur as part of a load, and require every call site to indicate what its security policy should be. This may be infeasible now (it'd be a huge change) but it would basically force code writers to consider this issue. We also talked about the very thing Brendan mentioned in 28327 as part of Shaver's content policy hooks: a parameter which gives the type of URI load. I'm probably going to have to add this to CheckLoadURI to do some of the things i want to do with it. It definitely makes sense to combine our security and privacy efforts, at least at the implementation level.

Milestone 0.8 has been released. We should either resolve this bug or update its milestone.

It breaks my heart (especially because we're doing work in other bugs that undoes some of the steps here), but...0.9.1.

(Someone less lame than me should feel free to take this bug from me.)

Asa just told me that we shipped 0.9.2! Who knew?

->0.9.5 since we have run out of time

I don't really know when I will get this done, so I'm removing the lying milestone marker.

Making this a meta bug for extensions/ build time dependencies.

Now that peterv has checked in the fix to bug 92377, this bug seems to be fixed. myotonic is green, and my own build with the following options: ac_add_options --disable-crypto ac_add_options --disable-mathml ac_add_options --disable-svg ac_add_options --disable-extensions ac_add_options --disable-jsd ac_add_options --disable-ldap ac_add_options --disable-mailnews ac_add_options --disable-xprint ac_add_options --disable-accessibility ac_add_options --disable-bidi works. These are the same options as myotonic uses. However, this should also be tested using --disable-extensions while enabling everything else (to ensure that mathml, crypto, mailnews, bidi, accessibility, etc., don't have incorrect dependencies on extensions).

Are we done here? 14889 seems like an independent perf bug. True? cls, can we escalate tinderbox no-extensions build failures (shrike and myotonic, IIRC) to sheriffs@mozilla.org now? /be

Yes, it looks like we're done here & 14889 is unrelated to the --disable-extensions issue. Shrike is building with all extensions (as are most ports) & myotonic is building without any extensions. Yes, sheriffs should know that bustage on myotonic is tree-stopping. Should we just move it to the main page?

We could move myotonic to the main page, but let's propose that to staff@mozilla.org. As dbaron pointed out, we might want some machine to reconfigure itself to test build-from-scratch cycling through all permutations of extensions -- that would be fun! This JS or equivalent gmake fu might help: function permuter(a) { if (!a || a.length == 0) throw "can't permute empty sequence"; if (a.length == 1) return a; var b = [] for (var i = 0; i < a.length; i++) { var elem = a.slice(i, i+1); var rest = permuter(a.slice(0, i).concat(a.slice(i+1))); for (var j = 0; j < rest.length; j++) b[b.length] = elem.concat(rest[j]); } return b; } print(uneval(permuter([1]))); print(uneval(permuter([1, 2]))); print(uneval(permuter([1, 2, 3]))); print(uneval(permuter([1, 2, 3, 4]))); Substitute extension names for the numbers 1-4. /be

that would be so cool, but should probably be a separate bug. cc'ing mcafee since he's been hacking the unix tinderbox scripts the most lately and might find this interesting.

there's now a bugzilla component for tinderbox changes, perhaps you should use it? personally i'd prefer to build all extensions and report all failures instead of permuting. I think mcafee or I could modify the build/reporting scripts to send back build failure notifications asap and then continue building and reporting them. assuming bustage breaks multiple extensions at once and that the build cycles aren't short or free, it's imo cheaper to report all the extension failures so that people can work on fixing all of them at once instead of waiting (consider the number of cycles it took to get the mac trees green [ignore the fact that the affected code wasn't extensions])

Reopening, we should not have this discussion in a fixed bug log. be: I don't like the permute thing, it's too confusing, people need something easy & brain-dead stupid. If there's a bunch of combinations we want to cover, then ok. I'd rather cover the cases that matter in a static way, this is much more maintainable for developers and sheriffs. If we want to keep the --disable-extensions option building, let's turn it on for one of the main tinderboxes that are already on the main page.

Open a separate bug for the tinderbox discussion or move it to the newsgroups. FWIW, I don't think the permutation based tinderbox would by us much. I tried something similar a couple of years ago for random build options combinations. I didn't seem to have much effect on tracking bustage & getting it fixed but that could have been because it was on the Ports page as well. I did test to make sure that we could still build on win32 & linux with everything enabled but extensions.

mcafee: yeah, the compleatist in me was out for a jog. timeless: we already test all extensions on one tinderbox, IIRC. We are now talking about (someone mailing staff@mozilla.org to propose that!) a main page tinderbox test the no-extensions case. I'm ok with catching bogo-dependencies among extensions by hand, or by luck, for now. /be