Checkpointing my implementation of the proposed navigator.offlineResources API described at http://www.campd.org/stuff/Offline%20Cache.html

This version uses the innermost uri when deciding the owner domain.

Comment on attachment 264567 [details] [diff] [review] v3 - In dom/public/idl/offline/nsIDOMOfflineResourceList.idl: + void refresh(); +}; \ No newline at end of file Add a newline. - In nsNavigator::GetOfflineResources(): + nsresult rv = nsContentUtils::GetSecurityManager()-> + GetSubjectPrincipal(getter_AddRefs(principal)); + + if (NS_FAILED(rv) || !principal) return NS_ERROR_FAILURE; + + nsCOMPtr<nsIURI> uri; + rv = principal->GetURI(getter_AddRefs(uri)); + if (NS_FAILED(rv) || !uri) return NS_ERROR_FAILURE; + + nsDOMOfflineResourceList *offlineResources; + offlineResources = new nsDOMOfflineResourceList(); + if (!offlineResources) return NS_ERROR_OUT_OF_MEMORY; + + rv = offlineResources->Init(uri); ... This'll use the caller's URI and not that of where the navigator object came from, and I think we want the latter. The cases where it makes a difference is if chrome or other elevated privilege script reaches out to another windows navigator.offlineResources. nsNavigator has a GetDocShell() method that you can use for this (though it may return null if someone uses a navigator object from an old window). + if (NS_FAILED(rv)) { + delete offlineResources; + return rv; + } You could use nsRefPtr for offlineResource to avoid having to do this manually. - In dom/src/offline/Makefile.in: +REQUIRES = xpcom \ + string \ + content \ + caps \ + gfx \ + js \ + layout \ + locale \ + necko \ + nkcache \ + pref \ + prefetch \ + unicharutil \ + widget \ + xpconnect \ + $(NULL) That seems like a lot of dependencies for this code. Do we really need all those? Is there a #include that pulls in tons of stuff that requires all those, or can we trim down that list? - In dom/src/offline/nsDOMOfflineResourceList.cpp: +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* ***** BEGIN LICENSE BLOCK ***** +* Version: MPL 1.1/GPL 2.0/LGPL 2.1 +* This doesn't look like a direct copy of the boilerplate licenses (leading spaces missing). See http://www.mozilla.org/MPL/boilerplate-1.1/mpl-tri-license-c. Formatting matters here since these things are sometimes tweaked in the thousands by scripts etc, so keep it as close as possible. Same thing in nsDOMOfflineResourceList.h. - In nsDOMOfflineResourceList::Item(): + aURI = NS_ConvertUTF8toUTF16(gCachedKeys[aIndex]); Using CopyUTF8toUTF16() would save you a string copy here. - In nsDOMOfflineResourceList::Add(): + // try to start fetching it now, but it's not fatal if it fails + nsCOMPtr<nsIPrefetchService> prefetchService = + do_GetService(NS_PREFETCHSERVICE_CONTRACTID); + if (!prefetchService) return NS_OK; Wouldn't you still want to let the caller know if we can't find a prefetch service? That pretty much means that no prefetching will happen, ever. r=jst with that.

> - In dom/src/offline/Makefile.in: > > +REQUIRES = xpcom \ > + string \ ... > + widget \ > + xpconnect \ > + $(NULL) > > That seems like a lot of dependencies for this code. Do we really need all > those? Is there a #include that pulls in tons of stuff that requires all those, > or can we trim down that list? nsDOMClassInfo.h ends up pulling a lot of that in; I was able to get rid of gfx, locale, and unicharutil. Fixed the other stuff.

updates the prefetch service check on Refresh() too.

oops, return rv if mOfflineResources->Init() fails

attached the wrong patch, sorry :/

updates the uuid, deals with a null docshell

Comment on attachment 264935 [details] [diff] [review] v6 r+sr=jst

Checked in. Dave, you're ready to get CVS access :-)

This caused bug 382482, which breaks zimbra (and anything else that tries to access navigator.offlineResources. Makes sad dogfood!