Closed Bug 1170021 Opened 9 years ago Closed 9 years ago

QuotaManager: Merge QuotaManager with QuotaObject

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox41 --- affected
firefox42 --- fixed

People

(Reporter: janv, Assigned: janv)

Details

Attachments

(2 files, 2 obsolete files)

patch
(deleted), patch
janv
: review+
Details | Diff | Splinter Review
Cleanup patch
(deleted), patch
bent.mozilla
: review+
Details | Diff | Splinter Review
The current split causes various implementation problems like: // XXX These four methods can go away once QuotaObject.cpp is merged with // QuotaManager.cpp which is part of the patch for bug 1130775
Attached patch patch (obsolete) (deleted) — — Splinter Review
No big changes in the code ... Just moved stuff from QuotaObject.h and QuotaObjects.cpp to QuotaManager.cpp, removed virtual methods from DirectoryLock and replaced some DirectoryLock* with DirectoryLocImpl*
Assignee: nobody → Jan.Varga
Status: NEW → ASSIGNED
Attachment #8615230 - Flags: review?(bent.mozilla)
Comment on attachment 8615230 [details] [diff] [review] patch Review of attachment 8615230 [details] [diff] [review]: ----------------------------------------------------------------- Yay!
Attachment #8615230 - Flags: review?(bent.mozilla) → review+
Ok, so the new architecture (public QuotaManager::DirectoryLock, private QuotaManager::DirectoryLockImpl) changes the situation here a bit. It seems I have to keep those four methods, because only QuotaManager class can use DirectoryLockImpl, helper classes in the anonymous namespace can't use it. It actually makes sense to have some getters on DirectoryLock and thanks to the new architecture these methods are now non-virtual, so there's no perf hit. Ben, I guess you are ok with that.
Ben, see the comment above.
Flags: needinfo?(bent.mozilla)
Ben says there's no reason for quota clients to call those getters, so I'm going to try something else.
Flags: needinfo?(bent.mozilla)
Attached patch patch (deleted) — — Splinter Review
Rebased on top of DirectoryLock -> QuotaManager::DirectoryLock changes
Attachment #8615230 - Attachment is obsolete: true
Attachment #8626551 - Flags: review+
Attached patch Cleanup patch (obsolete) (deleted) — — Splinter Review
I think having DirectoryLock as an inner class of QuotaManager complicates things. This patch keeps the private constructor/destructor DirectoryLockImpl friend class, but moves DirectoryLock and DirectoryLockImpl outside of QuotaManager. DirectoryLockImpl can't be moved to the anonymous namespace, but I can live with that.
Attachment #8626553 - Flags: review?(bent.mozilla)
Attached patch Cleanup patch (deleted) — — Splinter Review
Attachment #8626553 - Attachment is obsolete: true
Attachment #8626553 - Flags: review?(bent.mozilla)
Attachment #8626555 - Flags: review?(bent.mozilla)
Comment on attachment 8626555 [details] [diff] [review] Cleanup patch Review of attachment 8626555 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!
Attachment #8626555 - Flags: review?(bent.mozilla) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: