PAC should not load at startup, rather it should only load when it is needed. On a win32 dialup system, configured for auto dialing, it is rather annoying that starting mozilla causes a dialup just because it is configured with autoproxy.

*** This bug has been marked as a duplicate of 80885 ***

How is this bug a duplicate bug 80885? Autoproxy is turned on in this bug. Just that the browser doesn't need to use it when accessing files on the local hard drive. In bug 80885, autoproxy seems to be turned on even after it is turned off in preference.

Its the same thing. Whoever did this needs to check for network.proxy.type=2 and also needs to move the code so it fires at the right time.

This bug report was made a duplicate of bug 80885, which was made a duplicate of bug 85290, which is now fixed (as of build 2001061120 win32 talkback sea installer). I don't see the PAC:Loading PAC from <URL> but Mozilla still activates the autodialer on startup. Reopening

-->PAc

Serge, can you take a look at these PAC issues? Thanks - Jussi-Pekka

I don't see this when I set to direct connection. My ISP have stop using PAC, I am nolonger capable of testing this bug with a working PAC. This might only be reproducable the first time PAC is configured or when an invalid PAC is entered. Note: I load a blank page on my startup, not my homepage or previous page.

I was taking a look at what would be necessary to implement this last night, and the tricky bit seems to be that there's a race condition -- the PAC file is loaded asynchronously. So if PAC loading was triggered by a call to nsProtocolProxyService::ExamineForProxy or something, in all likelihood it would still be loading when the PAC code was actually executed. In this case, nsProxyAutoConfig.js will default to "direct". http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsProxyAutoConfig.js#54 The net effect is that the first url (or possibly more) loaded wouldn't be proxies under the current implementation, regardless of what the PAC says. There's also a rather ominous comment about potential deadlocks. Anyways, this warrants more study. On a related note, if this were implemented, should it be a pref? Some users may not want this.

PAC should only fire as an initialization event of the first network based event that is proxyable. I don't know why PAC would always load on startup, before any real network activity.

oen side effect of this: see 103979 which block 102958

adding back depend. everybody please be careful of midairs

cc darin; seeking for r/sr=

+ //continue from here means the current URL wouldn't be proxied: *FIX_ME* As I understand it, the first URL the user tries to load will instantiate the nsProtocolProxyService and kick off PAC load. But since that happens asynchronously, this first URL will probably not have the benefit of PAC/proxying. Is this assessment correct?

yes, this is correct.

In that case, I'm a little confused by the presence of |allowProxiedProtocols| and the flags. The same effect can be achieved by instantiating nsProtocolProxyService normally, but altering ConfigureFromPAC() to set a "loadOnNextRequest" flag, then kick off PAC load if the flag is set when a request comes in. This takes a good deal less, code, iirc. Six of one, half dozen of the other though, I guess. I had rejected this style of approach because I'm not sure how easy it is to fix the resulting "first request doesn't proxy" problem without making PAC loading synchronous -- it seemed like a fix for this bug would be immediately rewritten to fix the other. Have you got a good idea where this is going?

>I'm a little confused by the presence of |allowProxiedProtocols| and the flags. + if (mMaskOfProxiedProtocolFlags & PROXIED_PROTOCOL_FLAGS) will increase the performance, there is no needs to call mProxyService->ExamineForProxy() but we do, even there is no proxy specified in user prefs. >altering ConfigureFromPAC() to set a "loadOnNextRequest" flag, then kick off >PAC load if the flag is set when a request comes in. the thoughts like this have crossed my mind, but my approach easily allows to handle the case when LoadPACFromURL() failed, by setting mMaskOfProxiedProtocolFlags + ios.allowProxiedProtocols(true); //enable proxy in /mozilla/netwerk/base/src/nsProxyAutoConfig.js only after success on load and evaluation of *.pac file. >it seemed like a fix for this bug would be immediately rewritten to fix the >other. I did not address "first request doesn't proxy" problem here, which is a different bug, and it exists from beginning of nsProxyAutoConfig.js implementation, because of nsHttpChannel::AsyncOpen() call, What I'm trying to say is I do not introduce "first request doesn't proxy" problem in my patch, I just clarify that problem in my comments.

i don't know about this bug... it seems to me that you'd want to load the PAC as soon as possible, so as to help ensure that the user's homepage loads correctly. however, i understand that PAC isn't needed unless we are loading a website, so why bother delaying startup or why require the browser to be online just in order to startup? it's a difficult problem. what we probably really need is to create a PAC dummy channel that wraps the real channel. this would only be done, of course, if the corresponding protocol handler supported proxying. then AsyncOpen is called on the dummy channel, we could initiate the PAC download. when that is complete, we could then redirect the channel to the real channel. this would be pretty easy to implement i think. the solution of patch v2 worries me. it don't think it is the best solution to the problem, and it also seems to add more complexity to the IO service and protocol proxy service than that which is really necessary.

>so why bother delaying startup or why require the browser to be online just in >order to startup? that exactly why I moved in mozilla/netwerk/base/src/nsIOService.cpp (r1.115) 180 rv = nsServiceManager::GetService(kProtocolProxyServiceCID, 181 NS_GET_IID(nsIProtocolProxyService), 182 getter_AddRefs(mProxyService)); from 150 nsIOService::Init() into 717 nsIOService::NewChannelFromURI(nsIURI *aURI, nsIChannel **result) BTW: I think it should also fix bug103979 >then AsyncOpen is called on the dummy channel, we >could initiate the PAC download. when that is complete, we could then redirect >the channel to the real channel. I do not understand how it suppose to work, how we can block all other proxying protocols and wait until PACdownload is complete? What happen if *.pac file is located on slow servers?

Just from a quick look, I don't really like this. a) The mask flags need to be in nsIProtocolHandler, however, I don't think that it is needed: b) ExamineForProxy has all the proxy info. Thats intentional. Please do not move knowledge of proxies into the ioservice. (More than there is already, at least) c) Why did you & the flags from GetProtocolFlags with this mask? This function is, as its name suggests, meant to return all protocol flags. d) "prixytype"? e) The stuff at the end of createChannelFromURI is wrong - you now won't set a proxyinfo for ftp file, and so proxies will not work. Also, there is wierd tab/space interaction darin: When I fixed bug 89500, we agreed that a proxy channel wasn't really possible. Also, consider socks+pac+irc/mailnews, where the protocolhandlers do not implement nsIProxiedProtocolHandler (mailnews + my patch, that is). This was a design decision, because these protocols create socket transports differently. What about a new proxyType, of PAC_NEEDED, and then have the sockettransport thread avoid creating the socket until the results come back? We can toss the "java applet wants to know about this before pac is loaded" (java is the only other place needing to act of the results of ExamineForProxy) into the too-hard basket.

We need to block proxyable protocols until the pac file is loaded. Otherwise the results will not be correct. See my other comment.

bbaetz: you're right... a dummy channel wouldn't work in general :( the idea of creating using a proxyType would not work completely either i think. consider what would happen if the PAC says that FTP should be proxied via HTTP and we don't yet have the PAC. then we'd incorrectly create a FTP channel. perhaps we need both solutions... or perhaps all consumers should implement protocol handlers?!? i really think the PAC dummy channel is a clean solution for real protocol handlers.

darin: Bleh. How did ns4 handle this?

>b) ExamineForProxy has all the proxy info. Thats intentional. >knowledge of proxies into the ioservice. (More than there is already, at least) because of performance: 1) what is the reason to call ExamineForProxy() even when direct connection is set in user prefs? 2) what is the reason in ExamineForProxy() to call nsCOMPtr<nsIIOService> ios = do_GetService(kIOServiceCID, &rv); when we call it from nsIOService::NewChannelFromURI? maybe it's worth to add additional parameter nsIIOService *ios to ExamineForProxy() and do not call do_GetService when ios != 0 ? >c) Why did you & the flags from GetProtocolFlags with this mask? well, lets suppose PAC loading failed, in this case I do not want to ExamineForProxy() for any proxied protocols, the easy way to do so is unset ALLOW_PROXY flag for all those protocols. There are two trivial ways to do so: implementing SetProtocolFlags() method in nsIProtocolHandler, or mask nsIProtocolHandler::protocolFlags where it's needed, I choose the easiest one. I agree maybe it's not good idea to mask it in nsIOService::GetProtocolFlags(). >e) The stuff at the end of createChannelFromURI do you mean nsIOService::NewChannelFromURI() > is wrong - you now won't set a proxyinfo for ftp file, and so proxies will not >work. I do not see any diffs in code for http and ftp or any other protocols, could you clarify what is wrong, please? >What about a new proxyType, of PAC_NEEDED, and then have the sockettransport >thread avoid creating the socket until the results come back? I like this idea.

> 1) what is the reason to call ExamineForProxy() even when direct connection is > set in user prefs? proxy info belongs in the protocol proxy service. That said, passing in an optional protocolhandler argument, and moving the mUseProxy bailout checks to the top would be good. >>c) Why did you & the flags from GetProtocolFlags with this mask? >well, lets suppose PAC loading failed We should pop up a large error message in that case, like ns4 does. > in this case I do not want to >ExamineForProxy() for any proxied protocols, the easy way to >do so is unset ALLOW_PROXY flag for all those protocols. >There are two trivial ways to do so: >implementing SetProtocolFlags() method in nsIProtocolHandler, or mask >nsIProtocolHandler::protocolFlags where it's needed, I choose the easiest one. >I agree maybe it's not good idea to mask it in nsIOService::GetProtocolFlags(). No, you shouldn't mask it there. Only mask where needed >>e) The stuff at the end of createChannelFromURI >do you mean nsIOService::NewChannelFromURI() Yes, sorry > is wrong - you now won't set a proxyinfo for ftp file, and so proxies will not >work. > I do not see any diffs in code for http and ftp or any other protocols, could >you clarify what is wrong, please? Looking at the diff, you appear to only QI to nsIPRoxiedPRotocolHandler for the http case. That is wrong; socks proxies need to get the proxy info too. I can't see what that hunk of the patch is trying to do. I could just be misreading the patch, though. >>What about a new proxyType, of PAC_NEEDED, and then have the sockettransport >>thread avoid creating the socket until the results come back? >I like this idea. As darin mentioned, this won't work. Lots of places create a new channel, and expcet to be able to QI it to an nsIHttpChannel immediately. We can't go changing the type later.

Actually, what do people think of marking this bug WONTFIX? Provided that we don't try to fetch the url when pac is disabled, or we're offline, I think that it would be best to begin loading the url as soon as possible. This is important because of the lack of proxying while the pac file is being loaded, but also to improve perceived response time.

The original complaint -- that launching mozilla triggers a dialup connection, seems like a reasonable gripe to have. On the other hand, it seems like most solutions sucks, and delayed loading would introduce a noticeable latency in response time of the first page someone tries to load (probably the start page). There's a comment in bug 97605 that has always puzzled me -- Darin says: > a much simpler solution would be to push an event queue, which would still > allow UI events, but the PAC download would then become an app modal > operation. I don't entirye understand this proposal (I don't really understand the event architecture). It sort of sounds like it would make fixing this bug fairly easy (and fairly important, since you wouldn't want a modal load in the startup sequence). Could we mark a depend and call it a day?

tingly: right, but I'm proposing that we load this as soon as we go online. We had another discussion a few months ago about pac listening for these notifications, so that the myIPAdress stuff was correct. If you don't want dialup to happen, then start up offline. I assume that we allow this; 4.x did. Pushing an event queue won't really help. Thats to do with running the pac stuff; we just need to evaluate it at startup. Also, that bug is invalid - see the comments I'm about to make there.

Tying the load to going online makes a lot of sense. I still have code in my tree somewhere to make PAC a listener for this -- it should be easy to modify it to load the PAC as well.

how about if ExamineForProxy could return an error that said something like WOULD_BLOCK... then we could have an extra function call like AsyncExamineForProxy that callers could invoke to get the proxy info when it becomes available. this would allow the IO service to implement a dummy channel on behalf of proxied protocol handlers, and it would also allow mailnews a facility for asynchronously getting proxy info, without requiring any code changes to the socket transport service.

darin: No, that can't work. Consider someone typing ftp://ftp.mozilla.org/ into the urlbar. Someone calls NewChannelForURI, and we need to return a channel. If (and only if) we're using an http proxy, then that channel must be QIable to nsIHttpChannel immediately.

bbaetz: why must the dummy channel be QI'able to nsIHttpChannel?

Because you told me so, a few months ago? :) Here's one random example, from nsDocShell::DoURILoad (line 4506) : rv = NS_OpenURI(getter_AddRefs(channel), aURI, nsnull, loadGroup, NS_STATIC_CAST(nsIInterfaceRequestor *, this)); if (NS_FAILED(rv)) return rv; channel->SetOriginalURI(aURI); nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(channel)); // // If this is a HTTP channel, then set up the HTTP specific information // (ie. POST data, referer, ...) // if (httpChannel) { Now, we could argue that it doesn't matter for ftp-via-proxy, but you'd have to audit all the callers. Handling expiry times would matter, though. And you would need this for http/https over proxy. That could be done, possibly, I guess, but it would be really really ugly. Again, whats wrong with loading this at startup if we're unline? It is not unreasonable to assume that a person loading a web browser will want to access the web, in almost all cases.

did i say that? ;-) </sigh> your example mentions two things commonly added to a HTTP channel: POST data and referrer header, but neither of these would apply to the first HTTP page loaded... except maybe if editor starts supporting HTTP PUT for publishing documents... then, the dummy channel would need to support nsIUploadChannel, and then there'd probably be some other interface that would need to be supported. ok... perhaps this is the wrong way to go :(

And don't forget ssl via connect. I'm actually not sure how ftp upload via a proxy works - is it just a POST? This is what we thought of originally for the socks stuff, but it was just too complicated.

i don't recall thinking of it in exactly the same way, but yeah... makes sense.

to default owner

qa to me. Can anyone comment as to the validity of this bug in recent milestones (i.e., what is our situation going into mozilla 1.0)?

Temporarily "futuring" all PAC&SOCKS bugs to clear new-networking queue. I will review later. (I promise) If you object, and can make a case for a mozilla 1.0 fix, please reset milestone to "--" or email me.

removed jhooker from cc: list

+helpwanted, -futured. possible cause of the "I need to reload PAC" bugs, it loads while people are not on the network they expected to be on.

Bug #100022, bug #180811, and bug #188006 all appear to be dupes or related to bug #83984. Since #100022 has the most votes, and best matches what I'm seeing, I'll add my vote there in the hopes that it will do some good. This bug will force my corporation to go to IE if it's not fixed soon!

This is more visible if Quick Launch is enabled. The browser may try to load the PAC file before networking is available and then won't try again when the browser is first used. It’s been a minor support issue for our wireless users (who must authenticate before they can see the PAC server).

Related problem: bug 243277.

hum. it would be nice if we can force fx to load pac file. With fx 3.RC1 auto proxy do not work in my corporate network and fx forget anything about my proxy conf. So i have to set it manualy every time it start up my browser. Trust me, it's so boring i can stop using fx (even if i'm from firebird generation).

in the name of snappy, we really want to do things like this ahead of time if possible and the concern over dialup is significantly reduced these days. Most of the significant work of it happens off the main thread anyhow. So I'll wont fix this.