Closed
Bug 372970
Opened 17 years ago
Closed 17 years ago
implement navigator.offlineResources
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha5
People
(Reporter: dcamp, Assigned: dcamp)
References
()
Details
Attachments
(1 file, 7 obsolete files)
43.76 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
Checkpointing my implementation of the proposed navigator.offlineResources API described at http://www.campd.org/stuff/Offline%20Cache.html
Attachment #257599 -
Attachment is patch: true
Attachment #257599 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 1•17 years ago
|
||
Attachment #257599 -
Attachment is obsolete: true
Attachment #264327 -
Flags: review?(jst)
Assignee | ||
Comment 2•17 years ago
|
||
This version uses the innermost uri when deciding the owner domain.
Attachment #264327 -
Attachment is obsolete: true
Attachment #264567 -
Flags: review?(jst)
Attachment #264327 -
Flags: review?(jst)
Comment 3•17 years ago
|
||
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.
Attachment #264567 -
Flags: review?(jst) → review+
Assignee | ||
Comment 4•17 years ago
|
||
Attachment #264567 -
Attachment is obsolete: true
Assignee | ||
Comment 5•17 years ago
|
||
> - 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.
Assignee | ||
Comment 6•17 years ago
|
||
updates the prefetch service check on Refresh() too.
Attachment #264910 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Attachment #264921 -
Flags: superreview?(jst)
Assignee | ||
Comment 7•17 years ago
|
||
oops, return rv if mOfflineResources->Init() fails
Attachment #264921 -
Attachment is obsolete: true
Attachment #264923 -
Flags: superreview?(jst)
Attachment #264921 -
Flags: superreview?(jst)
Assignee | ||
Comment 8•17 years ago
|
||
attached the wrong patch, sorry :/
Attachment #264923 -
Attachment is obsolete: true
Attachment #264924 -
Flags: superreview?(jst)
Attachment #264923 -
Flags: superreview?(jst)
Assignee | ||
Comment 9•17 years ago
|
||
updates the uuid, deals with a null docshell
Attachment #264924 -
Attachment is obsolete: true
Attachment #264935 -
Flags: superreview?(jst)
Attachment #264924 -
Flags: superreview?(jst)
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.9?
Target Milestone: --- → mozilla1.9alpha5
Comment 10•17 years ago
|
||
Comment on attachment 264935 [details] [diff] [review] v6 r+sr=jst
Attachment #264935 -
Flags: superreview?(jst)
Attachment #264935 -
Flags: superreview+
Attachment #264935 -
Flags: review+
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Checked in. Dave, you're ready to get CVS access :-)
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 12•17 years ago
|
||
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/dom/tests/mochitest/ajax/offline/test_offlineResources.html&root=/cvsroot
Flags: in-testsuite+
This caused bug 382482, which breaks zimbra (and anything else that tries to access navigator.offlineResources. Makes sad dogfood!
Updated•11 years ago
|
Component: DOM: Mozilla Extensions → DOM
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•