1170021 - QuotaManager: Merge QuotaManager with QuotaObject

Status: RESOLVED FIXED

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

Description

[Jan Varga [:janv]]

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

Comment 1

[Jan Varga [:janv]]

Attachment: 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*

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8615230 [details] [diff] [review]
patch

Review of attachment 8615230 [details] [diff] [review]:
-----------------------------------------------------------------

Yay!

Comment 3

[Jan Varga [:janv]]

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.

Comment 4

[Jan Varga [:janv]]

Ben, see the comment above.

Comment 5

[Jan Varga [:janv]]

Ben says there's no reason for quota clients to call those getters, so I'm going to try something else.

Comment 6

[Jan Varga [:janv]]

Attachment: patch

Rebased on top of DirectoryLock -> QuotaManager::DirectoryLock changes

Comment 7

[Jan Varga [:janv]]

Attachment: 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.

Comment 8

[Jan Varga [:janv]]

Attachment: Cleanup patch

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8626555 [details] [diff] [review]
Cleanup patch

Review of attachment 8626555 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1bc9e9c1c92
https://hg.mozilla.org/integration/mozilla-inbound/rev/2327a7105476

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c1bc9e9c1c92
https://hg.mozilla.org/mozilla-central/rev/2327a7105476