It's suppose, that you set "offline" if you are not connected to Internet. But still, as a developer, you could have local WebServer - and you still can't access it!!! It's a bug IMHO. More for this - PSM, Help also acts as a local web server jut to display it's help pages. So you firbids to see help in "offline" mode for _every_ user. See also bug about cache/offline.

I can think of arguments for this both ways. Can someone post to the newsgroup and moderate this?

127.0.0.1 shouldn't work in offline mode, but localhost might.

Don't you thinks this is same?! :) Anyway - PSM Help uses "127.0.0.1" You could see it by clicking on HELP in PSM and then close the window appears before it loads HTML page inside. Then new window opens - and it is normal Mozilla window so you could see URL yourself.

If they are the same, then this bug is invalid. 127.0.0.1 is used for loopback

Sorry, could you explain? 127.0.0.1 - is used for loopback. To connect to [current] network device. localhost is just a name. through "/etc/hosts" file it converts to 127.0.0.1 (no IP packets use names , only IPs ;) And the main reason - "HELP of PSM is not working in 'offline' mode" :)

I'd also like to see http://localhost/ accessible in Offline mode but just making an exception for "localhost" and 127.0.0.1 is the wrong way to go IMO: I'm running an offline Apache webserver and installed further virtual hosts (in my case simple subdomains) such as "info.localhost", but these could be completely different names. The best way would be to allow making a list of URLs that even run in Offline mode, maybe in prefs.js? So if you just want to make Help available in Offline mode, please don't use that bug but file another one.

basic: I think that it would make more sense to argue the opposite. 127.0.0.1 is the "loopback" address, and it basically should always be on if TCP/IP is on. "localhost" is a convential name for 127.0.0.1. It would not make a lot of sense to let localhost work but not 127.0.0.1. malvin: If Mozilla is dependent on seeing PSM on the loopback address, then this might be one of those "lets argue about it because there might be something I haven't thought of" points. Jens: Selectable off-line "zones"/"realms" (list of domains or URLs) is probably a good RFE. It would solve this problem, but it should probably be a new bug. Create a new bug if this doesn't exist already. You feature request might go well with what I've proposed in bug 88218. For your specific "I want to run virtual domains over 127.0.0.1", I'm not sure what hacked hosts file can't do that offline does.

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 (you can query for this string to delete spam or retrieve the list of bugs I've moved)

neeti: What is the expected behavior here? I think this should be RESOLVED/WFM, because offline takes the application offline. If we want to make the Offline mode more network-level disconnect friendly, we basically need to implement bug 88218.

moving neeti's futured bugs for triaging.

Maybe this is tied into bug 164802 [MozillaFirebird] v 0.6.1 http://localhost:8080 goes to http://www.localhost.net.au/ http://127.0.0.0:8080 can not be found. Under Mozilla 1.5a http://localhost:8080 and http://127.0.0.0:8080 both bring up tomcats front page When I set my firwall to disallow internet connections it says it cannot reach www.google.com. It should never do this search because I am running tomcat on that port ! I brought my browser settings over form Mozilla 1.4 final, and I --hate-- keyword/search completion, so this was turned off. Now I can find no way to turn this feature off.

-> defaults: This scenario affects alot of our higher-end users who are offline. Robert, see the bug you referenced for more info. That has nothing to do w/ going offline.

fixed dependency.

Hello, Im using Firefox 3.0.1 and Windows XP and I use a lot local websites with no internet connection. I have the Offline problem all the time and I have to uncheck the work offline option. But everytime I restart Firefox I have to do the same again. I tried with localhost and with 127.0.0.1 and everytime happens the same. Why this option is not saved and how I can solve this problem? Is that a bug?

New behaviour of the 'work offline' feature changes the degree of annoyance of this bug. Firefox now tries to detect when you are offline and starts in offline mode. Now, users who need to access local web servers cannot do so without unchecking "work offline", because the auto-detection may have disabled their access to "localhost".

Another use case: the R online help (see http://www.r-project.org/ about R) does open help pages in browser, and functions as a http server to provide these pages, probably from some compressed representation. The opened URL refers to 127.0.0.1:<some high port number>. Due to this bug, users will only get an error message when they try to open the help, which can be confusing indeed. The correct solution of unckecking the offline mode menu item isn't obvious enough.

This doesn't give network zones, but it does rework offline mode to let loopback stay live. It's not completely network-silent, as DNS requests are still attempted (and succeed if the network is available). Suggestions welcome. The changes: + nsIOService no longer shuts down DNS and the socket service when going offline, it only does that on shutdown. Needs a few more changes, about what observers to notify and when. + nsHttpChannel no longer allows only cached pages to load. Non-local pages in cache should still load from cache. + nsSocketTransport2 will now balk (with NS_ERROR_OFFLINE) if offline and the address isn't loopback. + nsHttpHandler now observes the NS_IOSERVICE_GOING_OFFLINE_TOPIC and clears existing connections (otherwise, after going offline you could load pages only from domains with reusable connections). This was not needed for FTP, as it already did that. Not sure, but probably will be needed for websockets? + NSPRPUB's PR_IsNetAddrType() needed to modify its checks for IPv4 loopback prefix rather than for just the typical address of 127.0.0.1

I split this out into a separate patch, as NSPR is a separate product.

Adam: You have to request reviews from someone !

Comment on attachment 575552 [details] [diff] [review] netwerk: single check for observer service before notifying Jason chosen by random selection.

Comment on attachment 575552 [details] [diff] [review] netwerk: single check for observer service before notifying Review of attachment 575552 [details] [diff] [review]: ----------------------------------------------------------------- I'll steal the review, because there is at least one significant problem.. but, first, thanks Adam for the patch! As you note in comment 19 this no longer shuts down the socket service when going offline. Shutting that down actually results in detaching all the active sockets which is something some subsystems are going to depend on. You do add code to close idle connections in http connection manager, but that's just a subset of things. For instance active sockets might just hang - this includes spdy sockets which are always 'active' and as you note websockets too. Also anything using the socket api - such as maybe IMAP in Thunderbird probably has this implicit assumption. So I would say that minimally a] all the sockets should be detached b] dns should have an offline mode where it only resolves literals and (bonus points) cache hits.

When the STS is offline, the socket thread detects that and calls |Reset()|, which detaches the sockets. If not shutting down, it only detaches non-local sockets. When the DNS service is offline, it adds a new RESOLVE_OFFLINE flag to calls to |nsHostResolver::ResolveHost()|. This flag restricts resolution to literals and cached entries. Should |nsHostResolver::ResolveHost()| still try to resolve when RESOLVE_OFFLINE, but only return loopback records (to allow local hostnames like "localhost")?

Comment on attachment 619220 [details] [diff] [review] netwerk: Add offline mode to DNS service and STS. Add RESOLVE_OFFLINE flag for nsHostResolver. Review of attachment 619220 [details] [diff] [review]: ----------------------------------------------------------------- r=mcmanus with the processpendingevnets() issue addressed. Thanks! ::: netwerk/base/src/nsSocketTransport2.cpp @@ +1082,5 @@ > > nsresult rv; > > + if (gIOService->IsOffline() && > + PR_IsNetAddrType(&mNetAddr, PR_IpAddrLoopback) == PR_FALSE) nit - style is to check !foo instead of foo == false ::: netwerk/base/src/nsSocketTransportService2.cpp @@ +726,5 @@ > + > + // Final pass over the event queue. This makes sure that events posted by > + // socket detach handlers get processed. > + nsIThread *thread = NS_GetCurrentThread(); > + NS_ProcessPendingEvents(thread); 2 problems here - I think you only want this on the real shutdown, not on the offline case.. and more importantly you now spin the event loop while holding the lock (back down 1 step on the stack) - I was able to deadlock this patch pretty easily because of that. but to fix it I think you just need to only spin it manually on shutdown after the lock is given up, like before.

there's an idl change here, so don't forget the sr.

This only makes the final pass through the event loop when shutting down. Calling |Reset| in that locked block has a potential deadlock while running the tests: > ###!!! ASSERTION: Potential deadlock detected: > Cyclical dependency starts at > Mutex : nsAStreamCopier.mLock > Next dependency: > Mutex : nsSocketTransportService::mLock (currently acquired) > Cycle completed at > Mutex : nsAStreamCopier.mLock > Deadlock may happen for some other execution To avoid that possibility, this pulls |Reset| out of the locked block.

Comment on attachment 625811 [details] [diff] [review] Avoid deadlocks from events. sorry for the delay :( +++ b/netwerk/base/public/nsASocketHandler.h + // for server socket, whether only accepts local connections. whether -> which, I think +++ b/netwerk/base/src/nsIOService.cpp + nsIIOService *topic = static_cast<nsIIOService *>(this); topic -> subject +++ b/netwerk/base/src/nsSocketTransportService2.cpp I don't really understand why you need goingOffline here, can you explain? +++ b/netwerk/protocol/http/nsHttpChannel.cpp - if (offline) - mLoadFlags |= LOAD_ONLY_FROM_CACHE; hrm, so I see why you're doing this, but a better solution would be highly desirable so that you can at least get at the cached page when you have no internet connection, instead of just an error page...

(In reply to Christian :Biesinger (don't email me, ping me on IRC) from comment #30) > +++ b/netwerk/protocol/http/nsHttpChannel.cpp > - if (offline) > - mLoadFlags |= LOAD_ONLY_FROM_CACHE; > > hrm, so I see why you're doing this, but a better solution would be highly > desirable so that you can at least get at the cached page when you have no > internet connection, instead of just an error page... it's not my patch - so maybe I'm missing something - but I think this change just allows you to go to the network (localhost) if you are offline - but you'll still use the cache for everything. (or does load_only_from_cache give you back expired entries or something you wouldn't normally get?)

Thanks, Christian. > > +++ b/netwerk/base/public/nsASocketHandler.h > > + // for server socket, whether only accepts local connections. > > whether -> which, I think It was supposed to be whether, but I cleaned up the comment a bit to be clearer. For server sockets, if they are bound to loopback, then they only accept local connections. > > +++ b/netwerk/base/src/nsSocketTransportService2.cpp > > I don't really understand why you need goingOffline here, can you explain? This was what I referred to in comment 29. An assertion had indicated that deadlock was possible when |Reset| was called in the locked block. While no deadlockable path jumped out at me when I looked, I wanted to play it safe. I tried to recreate it, but the warning no longer comes up. This patch puts |Reset| back in the locked block and removes |goingOffline|. > > +++ b/netwerk/protocol/http/nsHttpChannel.cpp > > - if (offline) > > - mLoadFlags |= LOAD_ONLY_FROM_CACHE; > > hrm, so I see why you're doing this, but a better solution would be highly > desirable so that you can at least get at the cached page when you have no > internet connection, instead of just an error page... As Patrick mentioned in comment 31, it still tries to load from cache if there's an entry for it. For example, if I start the browser and load the Google Search page in a tab, close it, go offline, and try to load Google Search in a new tab, it loads from the cache. Same thing if I start the browser, immediately go offline, and then try to load a URL from the about:cache list of cache entries.

(In reply to Adam [:hobophobe] from comment #32) > As Patrick mentioned in comment 31, it still tries to load from cache if > there's an entry for it. > > For example, if I start the browser and load the Google Search page in a > tab, close it, go offline, and try to load Google Search in a new tab, it > loads from the cache. Same thing if I start the browser, immediately go > offline, and then try to load a URL from the about:cache list of cache > entries. Even if the page is expired?

(In reply to Patrick McManus [:mcmanus] from comment #31) > (or does load_only_from_cache give you back expired entries or something you > wouldn't normally get?) Correct, it does. Maybe that change is ok...

FWIW I tried the 'expires' test on http://www.procata.com/cachetest/ using the patch. STR: 1. Go to http://www.procata.com/cachetest/ 2. Click on the Expires "Perform Test" link 3. Click on the "Click here to go to step 2" link 4. Go Offline 5. Click on the "Click here to go to step 3" link (loads the same page as [2], but to pass it must not load the cached version). 6. Hover the "Click here to complete the test" and note it is "fail.html" In [6], if we weren't loading an expired entry, we would get the Offline error page. If we were online, it would replace the page with one where the link is to "pass.html" Unless I'm misunderstanding the test, we're still getting expired entries (either without |LOAD_ONLY_FROM_CACHE|, or it's set later). I'm also seeing warnings about potential deadlock again. Is this just a conservative warning, or something to be addressed? Output: > ###!!! ASSERTION: Potential deadlock detected: > Cyclical dependency starts at > ReentrantMonitor : nsHttpConnectionMgr.mReentrantMonitor > Next dependency: > Mutex : nsSocketTransportService::mLock (currently acquired) > Cycle completed at > ReentrantMonitor : nsHttpConnectionMgr.mReentrantMonitor > Deadlock may happen for some other execution Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=7d24d71d64b2

The |test_error_codes.js| test failure was getting |NS_ERROR_NOT_INITIALIZED| from |nsHTTPConnectionMgr::PostEvent|; |EnsureSocketThreadTargetIfOnline| becomes |EnsureSocketThreadTarget| and the test gets updated. The other tests broke because localhost is now reachable in offline mode. The fix for all three is disabling the proxy ("network.proxy.type" to |0|) when those tests are offline. They would pass when run in isolation, either because the proxy itself hadn't been resolved before or maybe there's some browser-side caching of proxy endpoints? The potential deadlock situation appears to be the cause for at least one crash [1]. That crash is related to the test for bug 336682, which is offline-mode related, though the test itself passed. This patch will call |Reset| outside of the locked block. 1. https://tbpl.mozilla.org/php/getParsedLog.php?id=14768084&tree=Try Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=a1a4ebe12100

Not sure why this didn't hit before; maybe try didn't have -Werror or had an exception for it before? Try: https://tbpl.mozilla.org/?tree=Try&rev=2766d56f0ff9

Doing a full try push this time, to try to sniff out any remaining problems before I ask for review. https://tbpl.mozilla.org/?tree=Try&rev=40afcaef2976

The last patch required slight fuzzing to apply (from the |nsAutoCString| change).

> > I'm also seeing warnings about potential deadlock again. Is this just a > conservative warning, or something to be addressed? > That's quite real.. I think the reset() logic should take care of it - are you confident this is fixed? > Output: > > > ###!!! ASSERTION: Potential deadlock detected: > > Cyclical dependency starts at > > ReentrantMonitor : nsHttpConnectionMgr.mReentrantMonitor > > Next dependency: > > Mutex : nsSocketTransportService::mLock (currently acquired) > > Cycle completed at > > ReentrantMonitor : nsHttpConnectionMgr.mReentrantMonitor > > Deadlock may happen for some other execution > > (In reply to Adam [:hobophobe] from comment #35) > > 6. Hover the "Click here to complete the test" and note it is "fail.html" > > In [6], if we weren't loading an expired entry, we would get the Offline > error page. If we were online, it would replace the page with one where the > link is to "pass.html" > > Unless I'm misunderstanding the test, we're still getting expired entries > (either without |LOAD_ONLY_FROM_CACHE|, or it's set later). > good test. did you get it resolved so it passes?

Comment on attachment 657902 [details] [diff] [review] Unbitrot Review of attachment 657902 [details] [diff] [review]: ----------------------------------------------------------------- The net effect of this is going to mean the socket thread stays constant even when offline/online events occur. I think that's a really good thing and it will eliminate a few classes of bugs. r+ contingent on these small items and answers to comment 40. ::: browser/base/content/test/browser_bug435325.js @@ +40,5 @@ > finish(); > } > > registerCleanupFunction(function() { > + Services.prefs.setIntPref("network.proxy.type", 2); you need at least push/pop. Please comment that this is necessary because the mochitest proxy runs n localhost and localhost is now reachable even when offline. ::: netwerk/base/src/nsSocketTransportService2.cpp @@ +684,5 @@ > +void > +nsSocketTransportService::Reset(bool aGuardLocals) > +{ > + // detach any sockets > + PRInt32 i; you've reintroduced PRint types.. need to scrub for those. @@ +686,5 @@ > +{ > + // detach any sockets > + PRInt32 i; > + bool isGuarded; > + for (i=mActiveCount-1; i>=0; --i) { I know its copy and paste, but fix it up to for (i = mActiveCouint -1; i >= 0; --i) @@ +693,5 @@ > + mActiveList[i].mHandler->IsLocal(&isGuarded); > + if (!isGuarded) > + DetachSocket(mActiveList, &mActiveList[i]); > + } > + for (i=mIdleCount-1; i>=0; --i) { again, fix the spacing ::: toolkit/components/places/tests/browser/browser_bug680727.js @@ +17,5 @@ > gBrowser.selectedTab = gBrowser.addTab(); > > + // Bypass proxy temporarily, (local is available in offline, see bug 87717) > + Services.prefs.setIntPref("network.proxy.type", 0); > + same comments from bug435325 ::: toolkit/mozapps/extensions/test/xpinstall/browser_offline.js @@ +16,5 @@ > } > > function download_progress(addon, value, maxValue) { > try { > + Services.prefs.setIntPref("network.proxy.type", 0); same comments from bug 435325

(In reply to Patrick McManus [:mcmanus] from comment #40) > > > > I'm also seeing warnings about potential deadlock again. Is this just a > > conservative warning, or something to be addressed? > > > > That's quite real.. I think the reset() logic should take care of it - are > you confident this is fixed? Yes. The fact that |Reset| always happens outside of that lock means it can't cause deadlock. > (In reply to Adam [:hobophobe] from comment #35) > > > > 6. Hover the "Click here to complete the test" and note it is "fail.html" > > > > In [6], if we weren't loading an expired entry, we would get the Offline > > error page. If we were online, it would replace the page with one where the > > link is to "pass.html" > > > > Unless I'm misunderstanding the test, we're still getting expired entries > > (either without |LOAD_ONLY_FROM_CACHE|, or it's set later). > > > > good test. did you get it resolved so it passes? That test was to check on Christian's concern in comment 33 and comment 34, that before, when offline, we would still get expired pages, and that it might not be happening with this change. The test gives us a page that should immediately expire. Failure means we did not grab a fresh copy, passing means we did. My impression is that we're accepting that offline mode may violate cache expiration, because it can't fetch a fresh copy; that is, that the failure is the Right Thing. When online, the test passes. (In reply to Patrick McManus [:mcmanus] from comment #41) > ::: browser/base/content/test/browser_bug435325.js > @@ +40,5 @@ > > finish(); > > } > > > > registerCleanupFunction(function() { > > + Services.prefs.setIntPref("network.proxy.type", 2); > > you need at least push/pop. Please comment that this is necessary because > the mochitest proxy runs n localhost and localhost is now reachable even > when offline. Added the comments, but wasn't clear if the "push/pop" referred to something else?

> > (In reply to Patrick McManus [:mcmanus] from comment #41) > > ::: browser/base/content/test/browser_bug435325.js > > @@ +40,5 @@ > > > finish(); > > > } > > > > > > registerCleanupFunction(function() { > > > + Services.prefs.setIntPref("network.proxy.type", 2); > > > > you need at least push/pop. Please comment that this is necessary because > > the mochitest proxy runs n localhost and localhost is now reachable even > > when offline. > > Added the comments, but wasn't clear if the "push/pop" referred to something > else? by push/pop I mean you need to save the original state of the pref and restore that rather than setting it explicitly to "2" when we're done, in case that's not what is used in the future.

> My impression is that we're accepting that offline mode may violate cache expiration, because > it can't fetch a fresh copy; that is, that the failure is the Right Thing. When online, the > test passes From the user's point of view, I wouldn't call it "the right thing". We're not showing her a page even though we have the data.

Comment on attachment 658634 [details] [diff] [review] Save and restore the network proxy preference value in tests for robustness Review of attachment 658634 [details] [diff] [review]: ----------------------------------------------------------------- This patch will still not allow you to connect to "localhost" when offline, right? Just to ::1/127.0.0.1 ::: netwerk/protocol/http/nsHttpChannel.cpp @@ -409,5 @@ > > - // are we offline? > - bool offline = gIOService->IsOffline(); > - if (offline) > - mLoadFlags |= LOAD_ONLY_FROM_CACHE; anyway, I'm ok with this removal. just saying it's not clear that it's the right thing.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=2b84bacf7276 > From the user's point of view, I wouldn't call it "the right > thing". We're not showing her a page even though we have the > data. and > anyway, I'm ok with this removal. just saying it's not clear that > it's the right thing. We _are_ showing her the page. That's why the test points to "fail.html", because we loaded/displayed an expired, cached page. The removal does not block that behavior, only allows local connections prior to that. > This patch will still not allow you to connect to "localhost" > when offline, right? Just to ::1/127.0.0.1 It could connect to "localhost" through a local proxy (why the tests were failing before), but otherwise it won't. If you connected to "localhost" recently before going offline, it will load a cached copy.

Sorry for the review churn here. Previous patch wasn't passing try for Android, which I had thought was due to an intermittent problem but proved to be patch's fault. Change here is in STS::Init, allowing initialization when offline. Greenish try is: https://tbpl.mozilla.org/?tree=Try&rev=86c7cc62bde9

i would think you can carry the sr forward

Thanks, Patrick!

Green try at: https://tbpl.mozilla.org/?tree=Try&rev=9147e7e13b02

If I switch Firefox to offline mode in current nightly and type in "http://localhost/", it tells me Firefox is offline and can't browse the web. Surely I should expect it to work? Gerv

(In reply to Gervase Markham [:gerv] from comment #54) > If I switch Firefox to offline mode in current nightly and type in > "http://localhost/", it tells me Firefox is offline and can't browse the > web. Surely I should expect it to work? The localhost name is just a name. It follows the normal process to resolve to an address. So it only works if localhost is in the DNS cache (ie, you recently connected to it while online), because name resolution of non-cached names doesn't occur in offline mode. The patch itself only enables connection to loopback interfaces. It doesn't modify the DNS resolver to try to resolve unknown names in offline mode. The browser doesn't have any say over the resolution process once it asks for a record. That is, we can't restrict a lookup to only the hosts file (where localhost and other locally defined names are configured). An attempted lookup that doesn't resolve in the hosts file would try to hit the network. If that's deemed acceptable, then a subsequent patch could simply filter resolution requests in offline mode. That is, only pass them on if they are loopback. But it would mean that we would cause network traffic in "offline" mode.

For me, all this patching was just a bad idea from modem era to save couple of bytes. When I want to disable internet activity, I click on the network connection icon in my OS GUI (any Linux or Windows) and use disconnect option. And I use browser offline mode to avoid ANY browser network activity with web sites, local proxy, DNS, external/local mail server (in SeaMonkey and Thunderbird) etc. I just need to view a cached web page or to stop looped JS refreshes of pages opened. It was browser offline mode, not system one. And now we have "localhost" requests depending on browser DNS cache. Is there any workaround to close all sockets or just to prevent browser from using them?

Not sure how strict your use case is. A local proxy might fit, but not if you're trying to prevent even socket creation; for the latter something like SELinux or systrace might work, but harder to set up and manage.

Well... People who wanted to control the OS NETWORK ACCESS per program got this firewall feature in the browser core. People who wanted to control the BROWSER to use or not to use networking got regression with the broken behavior.

It looks like RESOLVE_OFFLINE is unused now. In fact it looks like it was never used in the patch landed in this bug. How does this work, or is that a mistake?

Not sure if hobophobe is active any more, so also tagging mcmanus with needinfo as reviewer.

Unless I'm missing something, I guess this is a different-names-broke-search problem? It's called |RES_OFFLINE| when it's actually checked/used: http://hg.mozilla.org/mozilla-central/file/9605da94e75d/netwerk/dns/nsHostResolver.cpp#l826 The motivation for the different namings was due to the fact that flags in the IDL (|nsIDNSService.idl|) get named as |RESOLVE_FOO| while flags in the |nsHostResolver.h| name as |RES_FOO|.

Oh man, that's awful. Thanks for the prompt response.