Quota: Implement QuotaBackendImpl

Quota: Implement QuotaBackendImpl

This introduces QuotaBackendImpl working as bridge among FileSystemUsageCache,
QuotaManager and QuotaReservationManager. The backend will be created by
FileSystemContext and retained by QuotaReservationManager.


BUG=303443
TEST=content_unittests --gtest_filter=QuotaBackendImplTest.*
TEST=content_unittests --gtest_filter=QuotaReservationManagerTest.*
TBR=jochen@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233815

[nhiroki, 2013-11-07 06:51:38]

Hi, can you review this? Thanks!

[tzik, 2013-11-07 07:34:12]

Looks good.

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_backend_impl_unittest.cc (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl_unittest.cc:18: const char
kOrigin[] = "example.com";
this should be "http://example.com".

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl_unittest.cc:100: 
Maybe, we need TearDown to reset scoped_*ptr and flush message_loop.

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_reservation_manager.h (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_reservation_manager.h:74: explicit
QuotaReservationManager(QuotaBackend* backend);
Could you also replace this with scoped_ptr<QuotaBackend>?

[kinuko, 2013-11-07 10:10:08]

Thanks for working on this.  Haven't looked at the test code yet

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_backend_impl.cc (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl.cc:133:
callback.Run(base::PLATFORM_FILE_ERROR_NO_SPACE);
I don't think querying first and returning here really makes sense, since at
this point we've already written data into the disk?

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl.cc:166: :
base::PLATFORM_FILE_ERROR_FAILED;
The quota reservation code doesn't seem to handle failure case for
CommitQuotaUsage. I need to look into what other code is doing when
AtomicUpdateUsage fails (it shouldn't usually fail, I guess) but maybe we should
increment dirty count to force recalculation in the next time??  (tzik@, WDYT?)

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_backend_impl.h (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl.h:35: ReserveQuotaCallback;
nit: I think we can omit these typedefs in a subclass?

[tzik, 2013-11-07 11:45:14]

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_backend_impl.cc (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl.cc:166: :
base::PLATFORM_FILE_ERROR_FAILED;
On 2013/11/07 10:10:09, kinuko wrote:
> The quota reservation code doesn't seem to handle failure case for
> CommitQuotaUsage. I need to look into what other code is doing when
> AtomicUpdateUsage fails (it shouldn't usually fail, I guess) but maybe we
should
> increment dirty count to force recalculation in the next time??  (tzik@,
WDYT?)

Ah right, it ignores the result. CommitQuotaUsage shouldn't take a callback.

For errors on updating usage cache, I think it fails only on unrecoverable
disaster, so it's OK to ignore. :p

[nhiroki, 2013-11-08 00:12:05]

Updated, please take another look. Thanks!

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_backend_impl.cc (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl.cc:133:
callback.Run(base::PLATFORM_FILE_ERROR_NO_SPACE);
On 2013/11/07 10:10:09, kinuko wrote:
> I don't think querying first and returning here really makes sense, since at
> this point we've already written data into the disk?

Removed the querying part.

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl.cc:166: :
base::PLATFORM_FILE_ERROR_FAILED;
On 2013/11/07 11:45:15, tzik wrote:
> On 2013/11/07 10:10:09, kinuko wrote:
> > The quota reservation code doesn't seem to handle failure case for
> > CommitQuotaUsage. I need to look into what other code is doing when
> > AtomicUpdateUsage fails (it shouldn't usually fail, I guess) but maybe we
> should
> > increment dirty count to force recalculation in the next time??  (tzik@,
> WDYT?)
> 
> Ah right, it ignores the result. CommitQuotaUsage shouldn't take a callback.
> 
> For errors on updating usage cache, I think it fails only on unrecoverable
> disaster, so it's OK to ignore. :p

Okay, removed them. That looks simpler :)

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_backend_impl.h (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl.h:35: ReserveQuotaCallback;
On 2013/11/07 10:10:09, kinuko wrote:
> nit: I think we can omit these typedefs in a subclass?

QuotaReservationManager is not a superclass of QuotaBackend but an outerclass,
so we would need these typedefs.

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_backend_impl_unittest.cc (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl_unittest.cc:18: const char
kOrigin[] = "example.com";
On 2013/11/07 07:34:13, tzik wrote:
> this should be "http://example.com".

Oh... good catch! Done.

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_backend_impl_unittest.cc:100: 
On 2013/11/07 07:34:13, tzik wrote:
> Maybe, we need TearDown to reset scoped_*ptr and flush message_loop.

Done.

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
File webkit/browser/fileapi/quota/quota_reservation_manager.h (right):

https://codereview.chromium.org/61593002/diff/190001/webkit/browser/fileapi/q...
webkit/browser/fileapi/quota/quota_reservation_manager.h:74: explicit
QuotaReservationManager(QuotaBackend* backend);
On 2013/11/07 07:34:13, tzik wrote:
> Could you also replace this with scoped_ptr<QuotaBackend>?

Done.

[tzik, 2013-11-08 03:43:20]

lgtm

[kinuko, 2013-11-08 04:36:24]

lgtm

[commit-bot: I haz the power, 2013-11-08 04:39:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@chromium.org/61593002/520001

[commit-bot: I haz the power, 2013-11-08 05:04:35]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[nhiroki, 2013-11-08 05:08:37]

TBRing to jochen@ for "content_tests.gypi" since this adds just one test file.

[commit-bot: I haz the power, 2013-11-08 05:10:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@chromium.org/61593002/520001

[commit-bot: I haz the power, 2013-11-08 07:42:34]

Change committed as 233815