Changeset 30154 in webkit

Timestamp:

Feb 11, 2008 2:54:53 PM (16 years ago)

Author:

timothy@apple.com

Message:

Reviewed by Brady Eidson.

<rdar://problem/5733069> Many m_quotaMap uses do not hold the m_quotaMapGuard

• storage/DatabaseTracker.cpp: (WebCore::DatabaseTracker::hasEntryForOrigin): Hold m_quotaMapGuard when using m_quotaMap. (WebCore::DatabaseTracker::origins): Ditto. (WebCore::DatabaseTracker::setQuota): Ditto. (WebCore::DatabaseTracker::deleteAllDatabases): Call origins() and itterate over the origins to call deleteOrigin(). (WebCore::DatabaseTracker::deleteOrigin): Hold m_quotaMapGuard when using m_quotaMap.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• storage/DatabaseTracker.cpp (modified) (5 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-02-11  Timothy Hatcher  <timothy@apple.com>
+
+        Reviewed by Brady Eidson.
+
+        <rdar://problem/5733069> Many m_quotaMap uses do not hold the m_quotaMapGuard
+
+        * storage/DatabaseTracker.cpp:
+        (WebCore::DatabaseTracker::hasEntryForOrigin): Hold m_quotaMapGuard when using m_quotaMap.
+        (WebCore::DatabaseTracker::origins): Ditto.
+        (WebCore::DatabaseTracker::setQuota): Ditto.
+        (WebCore::DatabaseTracker::deleteAllDatabases): Call origins() and itterate over the
+        origins to call deleteOrigin().
+        (WebCore::DatabaseTracker::deleteOrigin): Hold m_quotaMapGuard when using m_quotaMap.
+
 2008-02-11  David Hyatt  <hyatt@apple.com>
 

trunk/WebCore/storage/DatabaseTracker.cpp

@@ -163 +163 @@
     ASSERT(currentThread() == m_thread);
     populateOrigins();
+    MutexLocker lockQuotaMap(m_quotaMapGuard);
     return m_quotaMap->contains(origin);
 }
@@ -299 +300 @@
     ASSERT(currentThread() == m_thread);
     populateOrigins();
+    MutexLocker lockQuotaMap(m_quotaMapGuard);
     copyKeysToVector(*m_quotaMap, result);
 }
@@ -536 +538 @@
     if (!m_database.isOpen())
         return;
-    if (!m_quotaMap->contains(origin)) {
-        SQLiteStatement statement(m_database, "INSERT INTO Origins VALUES (?, ?)");
-        if (statement.prepare() != SQLResultOk) {
-            LOG_ERROR("Unable to establish origin %s in the tracker", origin->stringIdentifier().ascii().data());
-        } else {
-            statement.bindText(1, origin->stringIdentifier());
-            statement.bindInt64(2, quota);
-
-            if (statement.step() != SQLResultDone)
-                LOG_ERROR("Unable to establish origin %s in the tracker", origin->stringIdentifier().ascii().data());
-        }
-    } else {
-        SQLiteStatement statement(m_database, "UPDATE Origins SET quota=? WHERE origin=?");        
-        bool error = statement.prepare() != SQLResultOk;
-        if (!error) {
-            statement.bindInt64(1, quota);
-            statement.bindText(2, origin->stringIdentifier());
-
-            error = !statement.executeCommand();
-        }
-        if (error)
-            LOG_ERROR("Failed to set quota %llu in tracker database for origin %s", quota, origin->stringIdentifier().ascii().data());
-    }
-    // FIXME: Is it really OK to update the quota in memory if we failed to update it on disk?
 
     {
         MutexLocker lockQuotaMap(m_quotaMapGuard);
+
+        if (!m_quotaMap->contains(origin)) {
+            SQLiteStatement statement(m_database, "INSERT INTO Origins VALUES (?, ?)");
+            if (statement.prepare() != SQLResultOk) {
+                LOG_ERROR("Unable to establish origin %s in the tracker", origin->stringIdentifier().ascii().data());
+            } else {
+                statement.bindText(1, origin->stringIdentifier());
+                statement.bindInt64(2, quota);
+
+                if (statement.step() != SQLResultDone)
+                    LOG_ERROR("Unable to establish origin %s in the tracker", origin->stringIdentifier().ascii().data());
+            }
+        } else {
+            SQLiteStatement statement(m_database, "UPDATE Origins SET quota=? WHERE origin=?");        
+            bool error = statement.prepare() != SQLResultOk;
+            if (!error) {
+                statement.bindInt64(1, quota);
+                statement.bindText(2, origin->stringIdentifier());
+
+                error = !statement.executeCommand();
+            }
+
+            if (error)
+                LOG_ERROR("Failed to set quota %llu in tracker database for origin %s", quota, origin->stringIdentifier().ascii().data());
+        }
+
+        // FIXME: Is it really OK to update the quota in memory if we failed to update it on disk?
         m_quotaMap->set(origin, quota);
     }
-    
+
     if (m_client)
         m_client->dispatchDidModifyOrigin(origin);
 }
-    
+
 bool DatabaseTracker::addDatabase(SecurityOrigin* origin, const String& name, const String& path)
 {
@@ -603 +608 @@
 {
     ASSERT(currentThread() == m_thread);
-    populateOrigins();
-    
-    QuotaMap quotaMapCopy = *m_quotaMap;
-    QuotaMap::const_iterator end = quotaMapCopy.end();
-    for (QuotaMap::const_iterator iter = quotaMapCopy.begin(); iter != end; ++iter)
-        deleteOrigin(iter->first.get());
+
+    Vector<RefPtr<SecurityOrigin> > originsCopy;
+    origins(originsCopy);
+
+    for (unsigned i = 0; i < originsCopy.size(); ++i)
+        deleteOrigin(originsCopy[i].get());
 }
 
@@ -663 +668 @@
         MutexLocker lockQuotaMap(m_quotaMapGuard);
         m_quotaMap->remove(origin);
+
         Locker<OriginQuotaManager> quotaManagerLocker(originQuotaManager());
         originQuotaManager().removeOrigin(origin);
-    }
-
-    // If we removed the last origin, do some additional deletion.
-    if (m_quotaMap->isEmpty()) {
-        if (m_database.isOpen())
-            m_database.close();
-        deleteFile(trackerDatabasePath());
-        deleteEmptyDirectory(m_databaseDirectoryPath);
+
+        // If we removed the last origin, do some additional deletion.
+        if (m_quotaMap->isEmpty()) {
+            if (m_database.isOpen())
+                m_database.close();
+            deleteFile(trackerDatabasePath());
+            deleteEmptyDirectory(m_databaseDirectoryPath);
+        }
     }