QuotaManager: Merge QuotaManager with QuotaObject

RESOLVED FIXED in Firefox 42

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: janv, Assigned: janv)

Tracking

Trunk
mozilla42
Points:
---

Firefox Tracking Flags

(firefox41 affected, firefox42 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

3 years ago
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
(Assignee)

Comment 1

3 years ago
Created attachment 8615230 [details] [diff] [review]
patch

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+
(Assignee)

Comment 3

3 years ago
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.
(Assignee)

Comment 4

3 years ago
Ben, see the comment above.
Flags: needinfo?(bent.mozilla)
(Assignee)

Comment 5

3 years ago
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)
(Assignee)

Comment 6

3 years ago
Created attachment 8626551 [details] [diff] [review]
patch

Rebased on top of DirectoryLock -> QuotaManager::DirectoryLock changes
Attachment #8615230 - Attachment is obsolete: true
Attachment #8626551 - Flags: review+
(Assignee)

Comment 7

3 years ago
Created attachment 8626553 [details] [diff] [review]
Cleanup patch

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)
(Assignee)

Comment 8

3 years ago
Created attachment 8626555 [details] [diff] [review]
Cleanup patch
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+
https://hg.mozilla.org/mozilla-central/rev/c1bc9e9c1c92
https://hg.mozilla.org/mozilla-central/rev/2327a7105476
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.