Open Bug 942542 Opened 11 years ago Updated 2 years ago

Easier implementation of new QuotaManager clients

Categories

(Core :: Storage: Quota Manager, defect, P5)

defect

Tracking

()

People

(Reporter: janv, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 11 obsolete files)

- remove IsFileServiceActivated, IsTransactionServiceActivated, AbortTransactionsForStorage, HasTransactionsForStorage from the Client interfave - replace Client::WaitForStoragesToComplete with Client::WaitForOriginsToComplete or something similar which doesn't need to pass storages to clients - add Client::WillShutdownIOThread(), IDB client needs to call FileService::WaitForStoragesToComplete there - rename Client::ShutdownTransactionService to Client::ShutdownBackgroundThreads or something - remove IDBDatabase::FromStorage - deCOM nsIFileStorage and nsIOfflineStorage - having IsShuttingDown() in nsIFileStorage is also not very clean
Blocks: 742822
Blocks: AsyncIDB
Depends on: 963064
Blocks: 847913
Depends on: 975696
Blocks: 961049
No longer blocks: AsyncIDB, 742822, 847913
Depends on: 980275, 860731
Depends on: 984789
Depends on: 992888
No longer depends on: 992888
Depends on: 1006485
Depends on: 1011510
Depends on: 1029209
Attached patch Patch 1: Cleanup (obsolete) (deleted) — Splinter Review
Assignee: nobody → Jan.Varga
Status: NEW → ASSIGNED
Attached patch Patch 2: Sync ops fixes (obsolete) (deleted) — Splinter Review
Attachment #8466187 - Attachment is patch: true
Attached patch Patch 3: Remove storage id from sync ops (obsolete) (deleted) — Splinter Review
Depends on: 771288
Attached file QuotaManager.h (obsolete) (deleted) —
This is how the new internal quota manager API looks like. There are some small differences from what we originally discussed. Instead of plain ref counting of origins, I decided to use directory locks and there's no Client::FinishUp() method, because we can just wait for directory locks to be released before clearing origins. Existing directory locks for an origin are invalidated by quota manager when the origin is to be cleared. When that happens an invalidate callback is called, so a client implementation can abort running operations (for example abort IndexedDB transactions). When all transactions are finished, the client releases the lock. This new architecture allows to abort even the process of opening/deleting of IndexedDB databases or asm.js caching. There's one drawback though, some methods are now virtual, but only one is called in a loop, MaybeAllocateMoreSpae(). Anyway disk I/O makes everything much slower, so I don't think those extra virtual methods would slow down things much.
Attached file QuotaManager.h (obsolete) (deleted) —
Attachment #8478015 - Attachment is obsolete: true
Attachment #8478535 - Flags: feedback?(bent.mozilla)
Attachment #8464030 - Attachment is obsolete: true
Attached patch initial patch (obsolete) (deleted) — Splinter Review
Attachment #8466187 - Attachment is obsolete: true
Attachment #8466199 - Attachment is obsolete: true
Attachment #8467743 - Attachment is obsolete: true
Attachment #8467745 - Attachment is obsolete: true
Attachment #8478535 - Attachment is obsolete: true
Attachment #8478535 - Flags: feedback?(bent.mozilla)
Attached patch patch v1 (obsolete) (deleted) — Splinter Review
Bye Bye nsIOfflineStorage and WaitForStoragesToComplete() :) https://tbpl.mozilla.org/?tree=Try&rev=9f800e0a8593
Attachment #8489721 - Attachment is obsolete: true
Attachment #8490278 - Flags: review?(bent.mozilla)
Attached patch patch v1 (obsolete) (deleted) — Splinter Review
Attachment #8490278 - Attachment is obsolete: true
Attachment #8490278 - Flags: review?(bent.mozilla)
Attachment #8497085 - Flags: review?(bent.mozilla)
Attached patch patch v1 (obsolete) (deleted) — Splinter Review
Attachment #8497085 - Attachment is obsolete: true
Attachment #8497085 - Flags: review?(bent.mozilla)
Attachment #8504251 - Flags: review?(bent.mozilla)
Attached patch patch v1 (deleted) — Splinter Review
Attachment #8504251 - Attachment is obsolete: true
Attachment #8504251 - Flags: review?(bent.mozilla)
Attachment #8506301 - Flags: review?(bent.mozilla)
Comment on attachment 8506301 [details] [diff] [review] patch v1 Review of attachment 8506301 [details] [diff] [review]: ----------------------------------------------------------------- Before reviewing the actual code here I'd prefer the API stick a little closer to our original design. Maybe I'm just missing the rationale for this, but: 1. I like the DirectoryLock idea, but I don't think it should have its own invalidate callback mechanism. Let's leave that on the Client API (that was FinishUp on our sketch). You can still do the thing where you wait for all DirectoryLocks to finish. 2. Why does DirectoryLock have an Unlock method? I thought the whole point was to use reference counting rather than explicit calls? 3. Why do we still have QuotaManager::EnsureOriginIsInitialized? Can't that be rolled into Open? 4. Why is the IO thread still exposed via QuotaManager public methods? 5. Why does Client need OriginClearCompleted, ReleaseIOThreadObjects, or ShutdownWorkThreads? 6. Why does Client::CalculateUsage have a UsageInfo param? Why isn't it just setting a uint64_t like we planned? 7. Why do we still have QuotaObject rather than the simpler QuotaManager::PrepareFileSizeChange?
Attachment #8506301 - Flags: review?(bent.mozilla)
Depends on: 1125102
No longer depends on: 771288
Depends on: 1130775
Is this bug still valid?
Flags: needinfo?(jvarga)
Priority: -- → P3
(In reply to Florian Bender from comment #14) > Is this bug still valid? Yes, we can still do more simplification as described in comment 13. However, it's not a priority right now.
Flags: needinfo?(jvarga)
Component: DOM → DOM: Quota Manager
I still want to this eventually, but there are more important things to do at the moment. Giving this a lower priority.
Priority: P3 → P5
Assignee: jvarga → nobody
Status: ASSIGNED → NEW
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: