[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
jorlow at chromium.org
jorlow at chromium.org
Wed Apr 7 23:23:36 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 969574dbb34d22f7bdcd6eb56e4754e6a930062f
Author: jorlow at chromium.org <jorlow at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Nov 5 08:36:22 2009 +0000
2009-11-03 Jeremy Orlow <jorlow at chromium.org>
Reviewed by Darin Fisher.
Clean up StorageAreaSync
https://bugs.webkit.org/show_bug.cgi?id=31100
Major fixes: Break the ref count cycle for StorageArea on the main
thread, not the background thread since the latter is not safe.
Length() needs to block on the import completing.
Small fixes: setItem needs to handle the copy on write case even if it
has an exception. setItem and removeItem should just bail from the
the function if the value hasn't changed rather than wrapping the end
in an if block. Clear should only send an event if it wasn't already
cleared. StorageAreaSync should assert that the final sync was
scheduled.
* storage/StorageAreaImpl.cpp:
(WebCore::StorageAreaImpl::length):
Forgot to block on the import.
(WebCore::StorageAreaImpl::key):
(WebCore::StorageAreaImpl::setItem):
Handle the copy on write case even when there's an exception.
(WebCore::StorageAreaImpl::removeItem):
(WebCore::StorageAreaImpl::clear):
* storage/StorageAreaSync.cpp:
(WebCore::StorageAreaSync::~StorageAreaSync):
(WebCore::StorageAreaSync::scheduleFinalSync):
(WebCore::StorageAreaSync::performImport):
(WebCore::StorageAreaSync::markImported):
(WebCore::StorageAreaSync::blockUntilImportComplete):
* storage/StorageAreaSync.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@50555 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index cb0bd08..26b3e19 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,37 @@
+2009-11-03 Jeremy Orlow <jorlow at chromium.org>
+
+ Reviewed by Darin Fisher.
+
+ Clean up StorageAreaSync
+ https://bugs.webkit.org/show_bug.cgi?id=31100
+
+ Major fixes: Break the ref count cycle for StorageArea on the main
+ thread, not the background thread since the latter is not safe.
+ Length() needs to block on the import completing.
+
+ Small fixes: setItem needs to handle the copy on write case even if it
+ has an exception. setItem and removeItem should just bail from the
+ the function if the value hasn't changed rather than wrapping the end
+ in an if block. Clear should only send an event if it wasn't already
+ cleared. StorageAreaSync should assert that the final sync was
+ scheduled.
+
+ * storage/StorageAreaImpl.cpp:
+ (WebCore::StorageAreaImpl::length):
+ Forgot to block on the import.
+ (WebCore::StorageAreaImpl::key):
+ (WebCore::StorageAreaImpl::setItem):
+ Handle the copy on write case even when there's an exception.
+ (WebCore::StorageAreaImpl::removeItem):
+ (WebCore::StorageAreaImpl::clear):
+ * storage/StorageAreaSync.cpp:
+ (WebCore::StorageAreaSync::~StorageAreaSync):
+ (WebCore::StorageAreaSync::scheduleFinalSync):
+ (WebCore::StorageAreaSync::performImport):
+ (WebCore::StorageAreaSync::markImported):
+ (WebCore::StorageAreaSync::blockUntilImportComplete):
+ * storage/StorageAreaSync.h:
+
2009-11-05 Zoltan Horvath <zoltan at webkit.org>
Reviewed by Eric Seidel.
diff --git a/WebCore/storage/StorageAreaImpl.cpp b/WebCore/storage/StorageAreaImpl.cpp
index e5f792f..938bce9 100644
--- a/WebCore/storage/StorageAreaImpl.cpp
+++ b/WebCore/storage/StorageAreaImpl.cpp
@@ -107,6 +107,8 @@ static bool privateBrowsingEnabled(Frame* frame)
unsigned StorageAreaImpl::length() const
{
ASSERT(!m_isShutdown);
+ blockUntilImportComplete();
+
return m_storageMap->length();
}
@@ -114,6 +116,7 @@ String StorageAreaImpl::key(unsigned index) const
{
ASSERT(!m_isShutdown);
blockUntilImportComplete();
+
return m_storageMap->key(index);
}
@@ -139,21 +142,20 @@ void StorageAreaImpl::setItem(const String& key, const String& value, ExceptionC
String oldValue;
bool quotaException;
RefPtr<StorageMap> newMap = m_storageMap->setItem(key, value, oldValue, quotaException);
+ if (newMap)
+ m_storageMap = newMap.release();
if (quotaException) {
ec = QUOTA_EXCEEDED_ERR;
return;
}
- if (newMap)
- m_storageMap = newMap.release();
+ if (oldValue == value)
+ return;
- // Only notify the client if an item was actually changed
- if (oldValue != value) {
- if (m_storageAreaSync)
- m_storageAreaSync->scheduleItemForSync(key, value);
- StorageEventDispatcher::dispatch(key, oldValue, value, m_storageType, m_securityOrigin.get(), frame);
- }
+ if (m_storageAreaSync)
+ m_storageAreaSync->scheduleItemForSync(key, value);
+ StorageEventDispatcher::dispatch(key, oldValue, value, m_storageType, m_securityOrigin.get(), frame);
}
void StorageAreaImpl::removeItem(const String& key, Frame* frame)
@@ -169,12 +171,12 @@ void StorageAreaImpl::removeItem(const String& key, Frame* frame)
if (newMap)
m_storageMap = newMap.release();
- // Only notify the client if an item was actually removed
- if (!oldValue.isNull()) {
- if (m_storageAreaSync)
- m_storageAreaSync->scheduleItemForSync(key, String());
- StorageEventDispatcher::dispatch(key, oldValue, String(), m_storageType, m_securityOrigin.get(), frame);
- }
+ if (oldValue.isNull())
+ return;
+
+ if (m_storageAreaSync)
+ m_storageAreaSync->scheduleItemForSync(key, String());
+ StorageEventDispatcher::dispatch(key, oldValue, String(), m_storageType, m_securityOrigin.get(), frame);
}
void StorageAreaImpl::clear(Frame* frame)
@@ -185,6 +187,9 @@ void StorageAreaImpl::clear(Frame* frame)
if (privateBrowsingEnabled(frame))
return;
+ if (!m_storageMap->length())
+ return;
+
unsigned quota = m_storageMap->quota();
m_storageMap = StorageMap::create(quota);
diff --git a/WebCore/storage/StorageAreaSync.cpp b/WebCore/storage/StorageAreaSync.cpp
index ad41e28..89226e7 100644
--- a/WebCore/storage/StorageAreaSync.cpp
+++ b/WebCore/storage/StorageAreaSync.cpp
@@ -71,6 +71,7 @@ StorageAreaSync::~StorageAreaSync()
{
ASSERT(isMainThread());
ASSERT(!m_syncTimer.isActive());
+ ASSERT(m_finalSyncScheduled);
}
void StorageAreaSync::scheduleFinalSync()
@@ -78,6 +79,7 @@ void StorageAreaSync::scheduleFinalSync()
ASSERT(isMainThread());
// FIXME: We do this to avoid races, but it'd be better to make things safe without blocking.
blockUntilImportComplete();
+ m_storageArea = 0; // This is done in blockUntilImportComplete() but this is here as a form of documentation that we must be absolutely sure the ref count cycle is broken.
if (m_syncTimer.isActive())
m_syncTimer.stop();
@@ -206,27 +208,18 @@ void StorageAreaSync::performImport()
return;
}
- MutexLocker locker(m_importLock);
-
HashMap<String, String>::iterator it = itemMap.begin();
HashMap<String, String>::iterator end = itemMap.end();
for (; it != end; ++it)
m_storageArea->importItem(it->first, it->second);
- // Break the (ref count) cycle.
- m_storageArea = 0;
- m_importComplete = true;
- m_importCondition.signal();
+ markImported();
}
void StorageAreaSync::markImported()
{
- ASSERT(!isMainThread());
-
MutexLocker locker(m_importLock);
- // Break the (ref count) cycle.
- m_storageArea = 0;
m_importComplete = true;
m_importCondition.signal();
}
@@ -238,19 +231,18 @@ void StorageAreaSync::markImported()
// item currently in the map. Get/remove can work whether or not it's in the map, but we'll need a list
// of items the import should not overwrite. Clear can also work, but it'll need to kill the import
// job first.
-void StorageAreaSync::blockUntilImportComplete() const
+void StorageAreaSync::blockUntilImportComplete()
{
ASSERT(isMainThread());
- // Fast path to avoid locking.
- if (m_importComplete)
+ // Fast path. We set m_storageArea to 0 only after m_importComplete being true.
+ if (!m_storageArea)
return;
MutexLocker locker(m_importLock);
while (!m_importComplete)
m_importCondition.wait(m_importLock);
- ASSERT(m_importComplete);
- ASSERT(!m_storageArea);
+ m_storageArea = 0;
}
void StorageAreaSync::sync(bool clearItems, const HashMap<String, String>& items)
diff --git a/WebCore/storage/StorageAreaSync.h b/WebCore/storage/StorageAreaSync.h
index 9d6436f..3f54a2b 100644
--- a/WebCore/storage/StorageAreaSync.h
+++ b/WebCore/storage/StorageAreaSync.h
@@ -46,7 +46,7 @@ namespace WebCore {
~StorageAreaSync();
void scheduleFinalSync();
- void blockUntilImportComplete() const;
+ void blockUntilImportComplete();
void scheduleItemForSync(const String& key, const String& value);
void scheduleClear();
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list