Changeset 50979 in webkit

Timestamp:

Nov 13, 2009 4:27:25 PM (15 years ago)

Author:

jorlow@chromium.org

Message:

2009-11-12 Jeremy Orlow <​jorlow@chromium.org>

Reviewed by Dmitry Titov.

LocalStorage quota should include key sizes in its count
​https://bugs.webkit.org/show_bug.cgi?id=31451

• storage/StorageMap.cpp: (WebCore::StorageMap::setItem):

Count keys in the quota when adding a new item.

(WebCore::StorageMap::removeItem):

Remove the key's length from the quota if we're removing the item.

(WebCore::StorageMap::importItem):

Assume that we're adding things for the first time.
Count keys in the quota.

2009-11-12 Jeremy Orlow <​jorlow@chromium.org>

Reviewed by Dmitry Titov.

Now that we're tracking key size in the quota, we can't fit as much in.
​https://bugs.webkit.org/show_bug.cgi?id=31451

• storage/domstorage/quota-expected.txt:
• storage/domstorage/script-tests/quota.js: (testQuota):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/storage/domstorage/quota-expected.txt (modified) (1 diff)
• LayoutTests/storage/domstorage/script-tests/quota.js (modified) (2 diffs)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/storage/StorageMap.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2009-11-12  Jeremy Orlow  <jorlow@chromium.org>
+
+        Reviewed by Dmitry Titov.
+
+        Now that we're tracking key size in the quota, we can't fit as much in.
+        https://bugs.webkit.org/show_bug.cgi?id=31451
+
+        * storage/domstorage/quota-expected.txt:
+        * storage/domstorage/script-tests/quota.js:
+        (testQuota):
+
 2009-11-13  Shinichiro Hamaji  <hamaji@chromium.org>
 

trunk/LayoutTests/storage/domstorage/quota-expected.txt

@@ -19 +19 @@
 Creating 'data' which contains 64K of data
 PASS data.length is 65536
-Putting 'data' into 40 localStorage buckets.
+Putting 'data' into 39 localStorage buckets.
 Putting 'data' into another bucket.h
 PASS Hit exception as expected
 Verify that data was never inserted.
-PASS storage.getItem(40) is null
-Removing bucket 39.
+PASS storage.getItem(39) is null
+Removing bucket 38.
 Adding 'Hello!' into a new bucket.
 PASS Insertion worked.

trunk/LayoutTests/storage/domstorage/script-tests/quota.js

@@ -20 +20 @@
     shouldBe("data.length", "65536");
 
-    debug("Putting 'data' into 40 " + storageString + " buckets.");
-    for (var i=0; i<40; i++)
+    debug("Putting 'data' into 39 " + storageString + " buckets.");
+    for (var i=0; i<39; i++)
         storage[i] = data;
 
     debug("Putting 'data' into another bucket.h");
     try {
-        storage[40] = data;
+        storage[39] = data;
         testFailed("Did not hit quota error.");
     } catch (e) {
@@ -33 +33 @@
 
     debug("Verify that data was never inserted.");
-    shouldBeNull("storage.getItem(40)");
+    shouldBeNull("storage.getItem(39)");
 
-    debug("Removing bucket 39.");
-    storage.removeItem('39');
+    debug("Removing bucket 38.");
+    storage.removeItem('38');
 
     debug("Adding 'Hello!' into a new bucket.");

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-11-12  Jeremy Orlow  <jorlow@chromium.org>
+
+        Reviewed by Dmitry Titov.
+
+        LocalStorage quota should include key sizes in its count
+        https://bugs.webkit.org/show_bug.cgi?id=31451
+
+        * storage/StorageMap.cpp:
+        (WebCore::StorageMap::setItem):
+            Count keys in the quota when adding a new item.
+        (WebCore::StorageMap::removeItem):
+            Remove the key's length from the quota if we're removing the item.
+        (WebCore::StorageMap::importItem):
+            Assume that we're adding things for the first time.
+            Count keys in the quota.
+
 2009-11-13  Dominik Röttsches  <dominik.roettsches@access-company.com>
 

trunk/WebCore/storage/StorageMap.cpp

@@ -112 +112 @@
     }
 
-    // Quota tracking.  If the quota is enabled and this would go over it, bail.
+    // Quota tracking.  This is done in a couple of steps to keep the overflow tracking simple.
+    unsigned newLength = m_currentLength;
+    bool overflow = newLength + value.length() < newLength;
+    newLength += value.length();
+
     oldValue = m_map.get(key);
-    unsigned newLength = m_currentLength + value.length() - oldValue.length();
+    overflow |= newLength - oldValue.length() > newLength;
+    newLength -= oldValue.length();
+
+    unsigned adjustedKeyLength = oldValue.isNull() ? key.length() : 0;
+    overflow |= newLength + adjustedKeyLength < newLength;
+    newLength += adjustedKeyLength;
+
+    ASSERT(!overflow);  // Overflow is bad...even if quotas are off.
     bool overQuota = newLength > m_quotaSize / sizeof(UChar);
-    bool overflow = (newLength > m_currentLength) != (value.length() > oldValue.length());
-    ASSERT(!overflow);  // If we're debugging, make a fuss.  But it's still worth checking this in the following if statement.
     if (m_quotaSize != noQuota && (overflow || overQuota)) {
         quotaException = true;
@@ -144 +153 @@
 
     oldValue = m_map.take(key);
-    if (!oldValue.isNull())
+    if (!oldValue.isNull()) {
         invalidateIterator();
-
-    // Update quota.
+        ASSERT(m_currentLength - key.length() <= m_currentLength);
+        m_currentLength -= key.length();
+    }
     ASSERT(m_currentLength - oldValue.length() <= m_currentLength);
     m_currentLength -= oldValue.length();
@@ -163 +173 @@
     // Be sure to copy the keys/values as items imported on a background thread are destined
     // to cross a thread boundary
-    pair<HashMap<String, String>::iterator, bool> result = m_map.add(key.threadsafeCopy(), String());
+    pair<HashMap<String, String>::iterator, bool> result = m_map.add(key.threadsafeCopy(), value.threadsafeCopy());
+    ASSERT(result.second);  // True if the key didn't exist previously.
 
-    if (result.second)
-        result.first->second = value.threadsafeCopy();
-
-    // Update quota.
+    ASSERT(m_currentLength + key.length() >= m_currentLength);
+    m_currentLength += key.length();
     ASSERT(m_currentLength + value.length() >= m_currentLength);
     m_currentLength += value.length();