Closed Bug 917432 Opened 6 years ago Closed 6 years ago

HTTP cache v2: hook to "webapps-clear-data" notification

Categories

(Core :: Networking: Cache, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

(Whiteboard: [cache2][qa-])

Attachments

(1 file, 4 obsolete files)

See nsHttpHandler::Observe()
Attached patch WIP1 (obsolete) — Splinter Review
This is a WIP to check we can do it with the new API.  The patch might be added to bug 917275, as I'm thinking of it.

However, I think this doesn't belong to the nsHttpHandler any more.  This needs to move to CacheObserver.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Blocks: 917423
Attached patch v1 (obsolete) — Splinter Review
- correct hook to web-apps-clear-data inside the new cache code
- however, still doesn't delete data on disk:

  EvictionRunnable(nsCSubstring const & aContextKey, TCacheEntryTable* aEntries,
  ...
  NS_IMETHOD Run()
  {
    LOG(("EvictionRunnable::Run [this=%p, disk=%d]", this, mUsingDisk));
    if (CacheStorageService::IsOnManagementThread()) {
      if (mUsingDisk) {
        // TODO for non private entries:
        // - rename/move all files to TRASH, block shutdown
        // - start the TRASH removal process
        // - may also be invoked from the main thread...
      }

We need a plan also on deleting individual files by certain criteria.
Attachment #806216 - Attachment is obsolete: true
Blocks: 922741
Comment on attachment 810021 [details] [diff] [review]
v1

This shows up being a regular fix.  Deleting individual files should be handled in a different bug (the eviction or index bug).

Needs a more careful review this time, some of the default running code is modified.
Attachment #810021 - Attachment description: WIP2 → v1
Attachment #810021 - Flags: review?(michal.novotny)
I have an updated patch already.  Will submit tomorrow.
Attachment #810021 - Flags: review?(michal.novotny)
Attached patch v2 (obsolete) — Splinter Review
Changes from v1:
- if a URL scheme is not one of http,ftp,wiciwyg, then the URL scheme is used as session name ; v1 was assert(false)'ing
Attachment #810021 - Attachment is obsolete: true
Attachment #813756 - Flags: review?(michal.novotny)
Comment on attachment 813756 [details] [diff] [review]
v2

Here is a try run of stack of patches for:
bug 922659
bug 922671
bug 922741
bug 917432 (this one)

https://tbpl.mozilla.org/?tree=Try&rev=b830b18e9c1c

and is green for cache2 disabled :)
Attached patch v2 [merged] (obsolete) — Splinter Review
- same as previous v2, just merged
Attachment #813756 - Attachment is obsolete: true
Attachment #813756 - Flags: review?(michal.novotny)
Attachment #827976 - Flags: review?(michal.novotny)
Attached patch v3Splinter Review
- cleanup of GetCacheSessionNameForStoragePolicy ; using sessionName.Assign(NS_LITERAL_CSTRING()) which is the fastest way of assigning a literal
- schemes other then http, wyciwyg or ftp are using session name 'other' or 'other-private' + appid/in-browser jar
- this allows the new API to consistently delete any user data inside a defined load context (correct cache partitioning according the new api) even with cache2 off

cache2 on:  https://tbpl.mozilla.org/?tree=Gum&rev=ae6d25ad7724
cache2 off: https://tbpl.mozilla.org/?tree=Try&rev=7b66d8eaea88
Attachment #827976 - Attachment is obsolete: true
Attachment #827976 - Flags: review?(michal.novotny)
Attachment #831057 - Flags: review?(michal.novotny)
Attachment #831057 - Flags: review?(michal.novotny) → review+
https://hg.mozilla.org/mozilla-central/rev/66a063b14ddb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [cache2] → [cache2][qa-]
You need to log in before you can comment on or make changes to this bug.