Open
Bug 942542
Opened 11 years ago
Updated 2 years ago
Easier implementation of new QuotaManager clients
Categories
(Core :: Storage: Quota Manager, defect, P5)
Core
Storage: Quota Manager
Tracking
()
NEW
People
(Reporter: janv, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 11 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
- 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
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Comment 1•10 years ago
|
||
Assignee: nobody → Jan.Varga
Status: NEW → ASSIGNED
Reporter | ||
Comment 2•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Attachment #8466187 -
Attachment is patch: true
Reporter | ||
Comment 3•10 years ago
|
||
Reporter | ||
Comment 4•10 years ago
|
||
Reporter | ||
Comment 5•10 years ago
|
||
Reporter | ||
Comment 6•10 years ago
|
||
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.
Reporter | ||
Comment 7•10 years ago
|
||
Attachment #8478015 -
Attachment is obsolete: true
Attachment #8478535 -
Flags: feedback?(bent.mozilla)
Reporter | ||
Updated•10 years ago
|
Attachment #8464030 -
Attachment is obsolete: true
Reporter | ||
Comment 8•10 years ago
|
||
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)
Reporter | ||
Comment 9•10 years ago
|
||
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)
Reporter | ||
Comment 10•10 years ago
|
||
Attachment #8490278 -
Attachment is obsolete: true
Attachment #8490278 -
Flags: review?(bent.mozilla)
Attachment #8497085 -
Flags: review?(bent.mozilla)
Reporter | ||
Comment 11•10 years ago
|
||
Attachment #8497085 -
Attachment is obsolete: true
Attachment #8497085 -
Flags: review?(bent.mozilla)
Attachment #8504251 -
Flags: review?(bent.mozilla)
Reporter | ||
Comment 12•10 years ago
|
||
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)
Reporter | ||
Updated•10 years ago
|
Blocks: webgl-shader-cache
Updated•6 years ago
|
Priority: -- → P3
Reporter | ||
Comment 15•6 years ago
|
||
(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)
Reporter | ||
Updated•6 years ago
|
Component: DOM → DOM: Quota Manager
Reporter | ||
Comment 16•6 years ago
|
||
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
Reporter | ||
Updated•6 years ago
|
Assignee: jvarga → nobody
Status: ASSIGNED → NEW
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•