Bug 31451 – LocalStorage quota should include key sizes in its count

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jeremy Orlow]

LocalStorage quota should include key sizes in its count.

[Jeremy Orlow]

Created attachment 43126 [details]
Patch

[Dmitry Titov]

Comment on attachment 43126 [details]
Patch

> +    bool overflow = (newLength > m_currentLength) != (value.length() + keyLength > oldValue.length());
> +    ASSERT(!overflow);  // Make a fuss if we're debugging.  But even if we aren't, don't allow overflow.

If value.length() + keyLength overflows by itself, then this check might not detect the overflow.
It seems the two-step method of detection as in StorageMap::importItem would work better.

[Jeremy Orlow]

Created attachment 43129 [details]
Patch

[Jeremy Orlow]

If you have any ideas on how to simplify the logic, please let me know.  It seems like it should be possible, but I didn't see any good ways.  (Overflow sucks....)

I could just make everything long longs and assume it's not possible.  :-)

[Dmitry Titov]

Comment on attachment 43129 [details]
Patch

> +    unsigned adjustedKeyLength = oldValue.isNull() ? key.length() : 0;
> +    newLength += adjustedKeyLength;
> +    overflow |= (newLength > m_currentLength) != (value.length() + adjustedKeyLength > oldValue.length());

I think the last line could be simply:
overflow |= newLength < m_currentLength;

r=me

[Jeremy Orlow]

Created attachment 43173 [details]
Patch

[Jeremy Orlow]

Dmitry: What you posted won't quite work.  overflow |= newLength + adjustedKeyLength < newLength  would work.  That gave me an idea on how to simplify the whole thing, though.  So I re-did it with only making one adjustment at a time which greatly simplified the logic.  Take another look?

[Dmitry Titov]

Comment on attachment 43173 [details]
Patch

Awesome!
r=me

[Jeremy Orlow]

Committed r50979: <http://trac.webkit.org/changeset/50979>