1110585 - Inconsistent timestamp in storage metadata file

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Hector Zhao [:hectorz]]

Timestamp in storage metadata comes from two sources: PR_Now for new directory, GetLastModifiedTime for existing directory. But value from PR_Now is in microsecond while that from GetLastModifiedTime is in millisecond.

Comment 1

[Gregor Wagner [:gwagner]]

During B2G startup:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1781.1857]
0xb4f09a32 in mozilla::dom::quota::QuotaManager::EnsureOriginIsInitialized (this=this@entry=0xb6a98980, aPersistenceType=aPersistenceType@entry=mozilla::dom::quota::PERSISTENCE_TYPE_PERSISTENT, aGroup=..., aOrigin=..., 
    aIsApp=aIsApp@entry=false, aHasUnlimStoragePerm=aHasUnlimStoragePerm@entry=false, aDirectory=0xa9ec1990) at ../../../dom/quota/QuotaManager.cpp:2379
2379	      MOZ_ASSERT(timestamp <= PR_Now());

Comment 2

[Jan Varga [:janv]]

Attachment: patch

Comment 3

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

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

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

Can we do this the other way and convert PR_Now to milliseconds instead? I don't think we need microsecond resolution in quota stuff...

Comment 4

[Jan Varga [:janv]]

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #3)
> Comment on attachment 8545185 [details] [diff] [review]
> patch
> 
> Review of attachment 8545185 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can we do this the other way and convert PR_Now to milliseconds instead? I
> don't think we need microsecond resolution in quota stuff...

Yeah, but we already have metadata files on the disk with origin access time stored in it and the value is in milliseconds.

Comment 5

[Jan Varga [:janv]]

(In reply to Jan Varga [:janv] from comment #4)
> (In reply to ben turner [:bent] (use the needinfo? flag!) from comment #3)
> > Comment on attachment 8545185 [details] [diff] [review]
> > patch
> > 
> > Review of attachment 8545185 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Can we do this the other way and convert PR_Now to milliseconds instead? I
> > don't think we need microsecond resolution in quota stuff...
> 
> Yeah, but we already have metadata files on the disk with origin access time
> stored in it and the value is in milliseconds.

sorry, I meant *microseconds*

Comment 6

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

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

I'm ok with this but please assert that we don't overflow (e.g. (INT64_MAX / PR_USEC_PER_MSEC) > timestamp))

Comment 7

[Jan Varga [:janv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/20a7f674d668

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/20a7f674d668

Comment 9

[Jan Varga [:janv]]

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

Approval Request Comment

[Feature/regressing bug #]: This is a followup fix for bug 1083927.

[User impact if declined]: I'm not aware of any big problems, but this fix is definitely needed for correctness

[Describe test coverage new/current, TBPL]: We have an assertion for this, bug 1110010 proves it.

[Risks and why]: The patch is very simple.

[String/UUID change made/needed]: No string/UUID changes.

Comment 10

[Jan Varga [:janv]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7679bb29a7c2