Diff
Modified: trunk/Source/WebKit2/ChangeLog (150029 => 150030)
--- trunk/Source/WebKit2/ChangeLog 2013-05-13 19:30:37 UTC (rev 150029)
+++ trunk/Source/WebKit2/ChangeLog 2013-05-13 19:31:04 UTC (rev 150030)
@@ -1,3 +1,42 @@
+2013-05-13 Anders Carlsson <[email protected]>
+
+ [WK2] Crash in WebKit::StorageAreaMap::didSetItem()
+ https://bugs.webkit.org/show_bug.cgi?id=116026
+
+ Reviewed by Andreas Kling.
+
+ Make sure that we ignore any leftover messages from the UI process after we've reset
+ the storage map. Achieve this by keeping a seed count in the StorageAreaMap object that's incremented
+ everytime the map is reset. Associate every storage area change with the seed and ignore any incoming
+ notification messages from the UI process if the seeds are different.
+
+ * Platform/CoreIPC/HandleMessage.h:
+ (CoreIPC):
+ (CoreIPC::callMemberFunction):
+ * UIProcess/Storage/StorageManager.cpp:
+ (WebKit::StorageManager::getValues):
+ (WebKit::StorageManager::setItem):
+ (WebKit::StorageManager::removeItem):
+ (WebKit::StorageManager::clear):
+ * UIProcess/Storage/StorageManager.h:
+ (StorageManager):
+ * UIProcess/Storage/StorageManager.messages.in:
+ * WebProcess/Storage/StorageAreaMap.cpp:
+ (WebKit::StorageAreaMap::StorageAreaMap):
+ (WebKit::StorageAreaMap::setItem):
+ (WebKit::StorageAreaMap::removeItem):
+ (WebKit::StorageAreaMap::clear):
+ (WebKit::StorageAreaMap::resetValues):
+ (WebKit::StorageAreaMap::loadValuesIfNeeded):
+ (WebKit::StorageAreaMap::didGetValues):
+ (WebKit::StorageAreaMap::didSetItem):
+ (WebKit::StorageAreaMap::didRemoveItem):
+ (WebKit::StorageAreaMap::didClear):
+ (WebKit::StorageAreaMap::applyChange):
+ * WebProcess/Storage/StorageAreaMap.h:
+ (StorageAreaMap):
+ * WebProcess/Storage/StorageAreaMap.messages.in:
+
2013-05-13 Jesus Sanchez-Palencia <[email protected]>
[WK2][CoordinatedGraphics] Avoid dispensable calls to WebView::updateViewportSize()
Modified: trunk/Source/WebKit2/Platform/CoreIPC/HandleMessage.h (150029 => 150030)
--- trunk/Source/WebKit2/Platform/CoreIPC/HandleMessage.h 2013-05-13 19:30:37 UTC (rev 150029)
+++ trunk/Source/WebKit2/Platform/CoreIPC/HandleMessage.h 2013-05-13 19:31:04 UTC (rev 150030)
@@ -259,6 +259,18 @@
(object->*function)(connection, args.argument1, args.argument2, args.argument3, args.argument4, args.argument5);
}
+template<typename C, typename MF, typename P1, typename P2, typename P3, typename P4, typename P5, typename P6>
+void callMemberFunction(Connection* connection, const Arguments6<P1, P2, P3, P4, P5, P6>& args, C* object, MF function)
+{
+ (object->*function)(connection, args.argument1, args.argument2, args.argument3, args.argument4, args.argument5, args.argument6);
+}
+
+template<typename C, typename MF, typename P1, typename P2, typename R1>
+void callMemberFunction(Connection* connection, const Arguments2<P1, P2>& args, Arguments1<R1>& replyArgs, C* object, MF function)
+{
+ (object->*function)(connection, args.argument1, args.argument2, replyArgs.argument1);
+}
+
template<typename C, typename MF, typename P1, typename R1>
void callMemberFunction(Connection* connection, const Arguments1<P1>& args, Arguments1<R1>& replyArgs, C* object, MF function)
{
Modified: trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp (150029 => 150030)
--- trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp 2013-05-13 19:30:37 UTC (rev 150029)
+++ trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp 2013-05-13 19:31:04 UTC (rev 150030)
@@ -443,7 +443,7 @@
m_storageAreasByConnection.remove(connectionAndStorageMapIDPair);
}
-void StorageManager::getValues(CoreIPC::Connection* connection, uint64_t storageMapID, HashMap<String, String>& values)
+void StorageManager::getValues(CoreIPC::Connection* connection, uint64_t storageMapID, uint64_t storageMapSeed, HashMap<String, String>& values)
{
StorageArea* storageArea = findStorageArea(connection, storageMapID);
@@ -452,10 +452,10 @@
values = storageArea->items();
- connection->send(Messages::StorageAreaMap::DidGetValues(), storageMapID);
+ connection->send(Messages::StorageAreaMap::DidGetValues(storageMapSeed), storageMapID);
}
-void StorageManager::setItem(CoreIPC::Connection* connection, uint64_t storageMapID, uint64_t sourceStorageAreaID, const String& key, const String& value, const String& urlString)
+void StorageManager::setItem(CoreIPC::Connection* connection, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& key, const String& value, const String& urlString)
{
StorageArea* storageArea = findStorageArea(connection, storageMapID);
@@ -464,10 +464,10 @@
bool quotaError;
storageArea->setItem(connection, sourceStorageAreaID, key, value, urlString, quotaError);
- connection->send(Messages::StorageAreaMap::DidSetItem(key, quotaError), storageMapID);
+ connection->send(Messages::StorageAreaMap::DidSetItem(storageMapSeed, key, quotaError), storageMapID);
}
-void StorageManager::removeItem(CoreIPC::Connection* connection, uint64_t storageMapID, uint64_t sourceStorageAreaID, const String& key, const String& urlString)
+void StorageManager::removeItem(CoreIPC::Connection* connection, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& key, const String& urlString)
{
StorageArea* storageArea = findStorageArea(connection, storageMapID);
@@ -475,10 +475,10 @@
ASSERT(storageArea);
storageArea->removeItem(connection, sourceStorageAreaID, key, urlString);
- connection->send(Messages::StorageAreaMap::DidRemoveItem(key), storageMapID);
+ connection->send(Messages::StorageAreaMap::DidRemoveItem(storageMapSeed, key), storageMapID);
}
-void StorageManager::clear(CoreIPC::Connection* connection, uint64_t storageMapID, uint64_t sourceStorageAreaID, const String& urlString)
+void StorageManager::clear(CoreIPC::Connection* connection, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& urlString)
{
StorageArea* storageArea = findStorageArea(connection, storageMapID);
@@ -486,7 +486,7 @@
ASSERT(storageArea);
storageArea->clear(connection, sourceStorageAreaID, urlString);
- connection->send(Messages::StorageAreaMap::DidClear(), storageMapID);
+ connection->send(Messages::StorageAreaMap::DidClear(storageMapSeed), storageMapID);
}
void StorageManager::createSessionStorageNamespaceInternal(uint64_t storageNamespaceID, CoreIPC::Connection* allowedConnection, unsigned quotaInBytes)
Modified: trunk/Source/WebKit2/UIProcess/Storage/StorageManager.h (150029 => 150030)
--- trunk/Source/WebKit2/UIProcess/Storage/StorageManager.h 2013-05-13 19:30:37 UTC (rev 150029)
+++ trunk/Source/WebKit2/UIProcess/Storage/StorageManager.h 2013-05-13 19:31:04 UTC (rev 150030)
@@ -65,11 +65,12 @@
void createLocalStorageMap(CoreIPC::Connection*, uint64_t storageMapID, uint64_t storageNamespaceID, const SecurityOriginData&);
void createSessionStorageMap(CoreIPC::Connection*, uint64_t storageMapID, uint64_t storageNamespaceID, const SecurityOriginData&);
void destroyStorageMap(CoreIPC::Connection*, uint64_t storageMapID);
- void getValues(CoreIPC::Connection*, uint64_t storageMapID, HashMap<String, String>& values);
- void setItem(CoreIPC::Connection*, uint64_t storageAreaID, uint64_t sourceStorageAreaID, const String& key, const String& value, const String& urlString);
- void removeItem(CoreIPC::Connection*, uint64_t storageMapID, uint64_t sourceStorageAreaID, const String& key, const String& urlString);
- void clear(CoreIPC::Connection*, uint64_t storageMapID, uint64_t sourceStorageAreaID, const String& urlString);
+ void getValues(CoreIPC::Connection*, uint64_t storageMapID, uint64_t storageMapSeed, HashMap<String, String>& values);
+ void setItem(CoreIPC::Connection*, uint64_t storageAreaID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& key, const String& value, const String& urlString);
+ void removeItem(CoreIPC::Connection*, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& key, const String& urlString);
+ void clear(CoreIPC::Connection*, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& urlString);
+
void createSessionStorageNamespaceInternal(uint64_t storageNamespaceID, CoreIPC::Connection* allowedConnection, unsigned quotaInBytes);
void destroySessionStorageNamespaceInternal(uint64_t storageNamespaceID);
void setAllowedSessionStorageNamespaceConnectionInternal(uint64_t storageNamespaceID, CoreIPC::Connection* allowedConnection);
Modified: trunk/Source/WebKit2/UIProcess/Storage/StorageManager.messages.in (150029 => 150030)
--- trunk/Source/WebKit2/UIProcess/Storage/StorageManager.messages.in 2013-05-13 19:30:37 UTC (rev 150029)
+++ trunk/Source/WebKit2/UIProcess/Storage/StorageManager.messages.in 2013-05-13 19:31:04 UTC (rev 150030)
@@ -25,9 +25,9 @@
CreateSessionStorageMap(uint64_t storageMapID, uint64_t storageNamespaceID, WebKit::SecurityOriginData securityOriginData) WantsConnection
DestroyStorageMap(uint64_t storageMapID) WantsConnection
- GetValues(uint64_t storageMapID) -> (WTF::HashMap<WTF::String, WTF::String> values) WantsConnection
+ GetValues(uint64_t storageMapID, uint64_t storageMapSeed) -> (WTF::HashMap<WTF::String, WTF::String> values) WantsConnection
- SetItem(uint64_t storageMapID, uint64_t sourceStorageAreaID, WTF::String key, WTF::String value, WTF::String urlString) WantsConnection
- RemoveItem(uint64_t storageMapID, uint64_t sourceStorageAreaID, WTF::String key, WTF::String urlString) WantsConnection
- Clear(uint64_t storageMapID, uint64_t sourceStorageAreaID, WTF::String urlString) WantsConnection
+ SetItem(uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, WTF::String key, WTF::String value, WTF::String urlString) WantsConnection
+ RemoveItem(uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, WTF::String key, WTF::String urlString) WantsConnection
+ Clear(uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, WTF::String urlString) WantsConnection
}
Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.cpp (150029 => 150030)
--- trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.cpp 2013-05-13 19:30:37 UTC (rev 150029)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.cpp 2013-05-13 19:31:04 UTC (rev 150030)
@@ -63,7 +63,9 @@
, m_storageNamespaceID(storageNamespace->storageNamespaceID())
, m_quotaInBytes(storageNamespace->quotaInBytes())
, m_securityOrigin(securityOrigin)
+ , m_currentSeed(0)
, m_hasPendingClear(false)
+ , m_hasPendingGetValues(false)
{
if (m_storageType == LocalStorage)
WebProcess::shared().connection()->send(Messages::StorageManager::CreateLocalStorageMap(m_storageMapID, storageNamespace->storageNamespaceID(), SecurityOriginData::fromSecurityOrigin(m_securityOrigin.get())), 0);
@@ -116,7 +118,7 @@
m_pendingValueChanges.add(key);
- WebProcess::shared().connection()->send(Messages::StorageManager::SetItem(m_storageMapID, sourceArea->storageAreaID(), key, value, sourceFrame->document()->url()), 0);
+ WebProcess::shared().connection()->send(Messages::StorageManager::SetItem(m_storageMapID, sourceArea->storageAreaID(), m_currentSeed, key, value, sourceFrame->document()->url()), 0);
}
void StorageAreaMap::removeItem(WebCore::Frame* sourceFrame, StorageAreaImpl* sourceArea, const String& key)
@@ -132,7 +134,7 @@
m_pendingValueChanges.add(key);
- WebProcess::shared().connection()->send(Messages::StorageManager::RemoveItem(m_storageMapID, sourceArea->storageAreaID(), key, sourceFrame->document()->url()), 0);
+ WebProcess::shared().connection()->send(Messages::StorageManager::RemoveItem(m_storageMapID, sourceArea->storageAreaID(), m_currentSeed, key, sourceFrame->document()->url()), 0);
}
void StorageAreaMap::clear(WebCore::Frame* sourceFrame, StorageAreaImpl* sourceArea)
@@ -141,7 +143,7 @@
m_hasPendingClear = true;
m_storageMap = StorageMap::create(m_quotaInBytes);
- WebProcess::shared().connection()->send(Messages::StorageManager::Clear(m_storageMapID, sourceArea->storageAreaID(), sourceFrame->document()->url()), 0);
+ WebProcess::shared().connection()->send(Messages::StorageManager::Clear(m_storageMapID, sourceArea->storageAreaID(), m_currentSeed, sourceFrame->document()->url()), 0);
}
bool StorageAreaMap::contains(const String& key)
@@ -154,8 +156,11 @@
void StorageAreaMap::resetValues()
{
m_storageMap = nullptr;
+
m_pendingValueChanges.clear();
m_hasPendingClear = false;
+ m_hasPendingGetValues = false;
+ m_currentSeed++;
}
void StorageAreaMap::loadValuesIfNeeded()
@@ -167,22 +172,29 @@
// FIXME: This should use a special sendSync flag to indicate that we don't want to process incoming messages while waiting for a reply.
// (This flag does not yet exist). Since loadValuesIfNeeded() ends up being called from within _javascript_ code, processing incoming synchronous messages
// could lead to weird reentrency bugs otherwise.
- WebProcess::shared().connection()->sendSync(Messages::StorageManager::GetValues(m_storageMapID), Messages::StorageManager::GetValues::Reply(values), 0);
+ WebProcess::shared().connection()->sendSync(Messages::StorageManager::GetValues(m_storageMapID, m_currentSeed), Messages::StorageManager::GetValues::Reply(values), 0);
m_storageMap = StorageMap::create(m_quotaInBytes);
m_storageMap->importItems(values);
- // We want to ignore all changes until we get the DidGetValues message, so treat this as a pending clear.
- m_hasPendingClear = true;
+ // We want to ignore all changes until we get the DidGetValues message.
+ m_hasPendingGetValues = true;
}
-void StorageAreaMap::didGetValues()
+void StorageAreaMap::didGetValues(uint64_t storageMapSeed)
{
- m_hasPendingClear = false;
+ if (m_currentSeed != storageMapSeed)
+ return;
+
+ ASSERT(m_hasPendingGetValues);
+ m_hasPendingGetValues = false;
}
-void StorageAreaMap::didSetItem(const String& key, bool quotaError)
+void StorageAreaMap::didSetItem(uint64_t storageMapSeed, const String& key, bool quotaError)
{
+ if (m_currentSeed != storageMapSeed)
+ return;
+
ASSERT(m_pendingValueChanges.contains(key));
if (quotaError) {
@@ -193,15 +205,21 @@
m_pendingValueChanges.remove(key);
}
-void StorageAreaMap::didRemoveItem(const String& key)
+void StorageAreaMap::didRemoveItem(uint64_t storageMapSeed, const String& key)
{
+ if (m_currentSeed != storageMapSeed)
+ return;
+
ASSERT(m_pendingValueChanges.contains(key));
-
m_pendingValueChanges.remove(key);
}
-void StorageAreaMap::didClear()
+void StorageAreaMap::didClear(uint64_t storageMapSeed)
{
+ if (m_currentSeed != storageMapSeed)
+ return;
+
+ ASSERT(m_hasPendingClear);
m_hasPendingClear = false;
}
@@ -224,8 +242,8 @@
{
ASSERT(m_storageMap->hasOneRef());
- // There's a clear pending, we don't want any changes until we've gotten the DidClear message.
- if (m_hasPendingClear)
+ // There's a clear pending or getValues pending we don't want to apply any changes until we get the corresponding DidClear/DidGetValues messages.
+ if (m_hasPendingClear || m_hasPendingGetValues)
return;
if (!key) {
Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.h (150029 => 150030)
--- trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.h 2013-05-13 19:30:37 UTC (rev 150029)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.h 2013-05-13 19:31:04 UTC (rev 150030)
@@ -65,10 +65,10 @@
// CoreIPC::MessageReceiver
virtual void didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&) OVERRIDE;
- void didGetValues();
- void didSetItem(const String& key, bool quotaError);
- void didRemoveItem(const String& key);
- void didClear();
+ void didGetValues(uint64_t storageMapSeed);
+ void didSetItem(uint64_t storageMapSeed, const String& key, bool quotaError);
+ void didRemoveItem(uint64_t storageMapSeed, const String& key);
+ void didClear(uint64_t storageMapSeed);
void resetValues();
void loadValuesIfNeeded();
@@ -89,7 +89,10 @@
RefPtr<WebCore::SecurityOrigin> m_securityOrigin;
RefPtr<WebCore::StorageMap> m_storageMap;
+
+ uint64_t m_currentSeed;
bool m_hasPendingClear;
+ bool m_hasPendingGetValues;
HashCountedSet<String> m_pendingValueChanges;
};
Modified: trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.messages.in (150029 => 150030)
--- trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.messages.in 2013-05-13 19:30:37 UTC (rev 150029)
+++ trunk/Source/WebKit2/WebProcess/Storage/StorageAreaMap.messages.in 2013-05-13 19:31:04 UTC (rev 150030)
@@ -21,10 +21,10 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
messages -> StorageAreaMap {
- DidGetValues()
- DidSetItem(WTF::String key, bool quotaException)
- DidRemoveItem(WTF::String key)
- DidClear()
+ DidGetValues(uint64_t storageMapSeed)
+ DidSetItem(uint64_t storageMapSeed, WTF::String key, bool quotaException)
+ DidRemoveItem(uint64_t storageMapSeed, WTF::String key)
+ DidClear(uint64_t storageMapSeed)
DispatchStorageEvent(uint64_t sourceStorageAreaID, WTF::String key, WTF::String oldValue, WTF::String newValue, WTF::String urlString)
}
