Diff
Added: trunk/LayoutTests/http/tests/cookies/resources/setArraycookies-expected.txt (0 => 109494)
--- trunk/LayoutTests/http/tests/cookies/resources/setArraycookies-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/cookies/resources/setArraycookies-expected.txt 2012-03-02 04:26:40 UTC (rev 109494)
@@ -0,0 +1,5 @@
+This test case to set a Array of Cookie size of 3 and check if it's been correctly set and in order.
+If the Test case was successful, Then it should print Passed below!!
+
+
+Passed
Added: trunk/LayoutTests/http/tests/cookies/resources/setArraycookies-result.php (0 => 109494)
--- trunk/LayoutTests/http/tests/cookies/resources/setArraycookies-result.php (rev 0)
+++ trunk/LayoutTests/http/tests/cookies/resources/setArraycookies-result.php 2012-03-02 04:26:40 UTC (rev 109494)
@@ -0,0 +1,34 @@
+<html>
+<head>
+<script>
+ function run(){
+ // Reading the Cooikes using PHP
+ var cookie = "<?php if (isset($_COOKIE['setArraycookie'])) {
+ foreach ($_COOKIE['setArraycookie'] as $name => $value) {
+ $name = htmlspecialchars($name);
+ $value = htmlspecialchars($value);
+ echo "$name : $value";
+ }
+ }
+ ?>";
+
+ var status = "Fail";
+ if (cookie == "three : cookiethreetwo : cookietwoone : cookieone") {
+ document.getElementById("test").innerHTML = "<b>Passed</b>";
+ status = "Pass";
+ } else
+ document.getElementById("test").innerHTML = "<b>Failed</b>";
+ }
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+</script>
+</head>
+<body _onload_="run()">
+<p>This test case to set a Array of Cookie size of 3 and check if it's been correctly set and in order.<br />If the Test case was successful, Then it should print Passed below!! </p><br/>
+<div id="test">
+Not Working!!
+</div>
+</body>
+</html>
+
Added: trunk/LayoutTests/http/tests/cookies/resources/setArraycookies.php (0 => 109494)
--- trunk/LayoutTests/http/tests/cookies/resources/setArraycookies.php (rev 0)
+++ trunk/LayoutTests/http/tests/cookies/resources/setArraycookies.php 2012-03-02 04:26:40 UTC (rev 109494)
@@ -0,0 +1,8 @@
+<?php
+// Setting Array of cookies
+setcookie("setArraycookie[three]", "cookiethree");
+setcookie("setArraycookie[two]", "cookietwo");
+setcookie("setArraycookie[one]", "cookieone");
+header('Location: setArraycookies-result.php');
+?>
+
Modified: trunk/Source/WebCore/ChangeLog (109493 => 109494)
--- trunk/Source/WebCore/ChangeLog 2012-03-02 04:23:25 UTC (rev 109493)
+++ trunk/Source/WebCore/ChangeLog 2012-03-02 04:26:40 UTC (rev 109494)
@@ -1,3 +1,37 @@
+2012-03-01 Jason Liu <[email protected]>
+
+ [BlackBerry]Array of Cookies in HTTP request header are not in order.
+ https://bugs.webkit.org/show_bug.cgi?id=79870
+
+ Std::sort and HashMap are not stable. So cookies with the same creating
+ time sometimes are sent disorder.
+ Change std::sort with std::stable-sort.
+ We don't need using HashMap to save so few cookies for one domain.
+ It is a wast of time to create HashMap, too.
+ So change it with vector.
+
+ Reviewed by George Staikos.
+
+ Test: http/tests/cookies/resources/setArraycookies.php
+
+ * platform/blackberry/CookieManager.cpp:
+ (WebCore::cookieSorter):
+ (WebCore::CookieManager::getRawCookies):
+ (WebCore::CookieManager::checkAndTreatCookie):
+ (WebCore::CookieManager::addCookieToMap):
+ * platform/blackberry/CookieManager.h:
+ * platform/blackberry/CookieMap.cpp:
+ (WebCore::CookieMap::addOrReplaceCookie):
+ (WebCore::CookieMap::removeCookieAtIndex):
+ (WebCore::CookieMap::removeCookie):
+ (WebCore):
+ (WebCore::CookieMap::getAllCookies):
+ (WebCore::CookieMap::updateOldestCookie):
+ (WebCore::CookieMap::deleteAllCookiesAndDomains):
+ * platform/blackberry/CookieMap.h:
+ (WebCore::CookieMap::count):
+ (CookieMap):
+
2012-03-01 Adam Barth <[email protected]>
Move WebCore/storage/IDB* files into WebCore/Modules/indexeddb
Modified: trunk/Source/WebCore/platform/blackberry/CookieManager.cpp (109493 => 109494)
--- trunk/Source/WebCore/platform/blackberry/CookieManager.cpp 2012-03-02 04:23:25 UTC (rev 109493)
+++ trunk/Source/WebCore/platform/blackberry/CookieManager.cpp 2012-03-02 04:26:40 UTC (rev 109494)
@@ -120,7 +120,7 @@
static bool cookieSorter(ParsedCookie* a, ParsedCookie* b)
{
if (a->path().length() == b->path().length())
- return a->creationTime() <= b->creationTime();
+ return a->creationTime() < b->creationTime();
return a->path().length() > b->path().length();
}
@@ -300,7 +300,7 @@
}
}
- std::sort(stackOfCookies.begin(), stackOfCookies.end(), cookieSorter);
+ std::stable_sort(stackOfCookies.begin(), stackOfCookies.end(), cookieSorter);
}
void CookieManager::removeAllCookies(BackingStoreRemovalPolicy backingStoreRemoval)
@@ -370,32 +370,59 @@
if (postToBackingStore == BackingStoreCookieEntry)
m_cookieBackingStore->remove(candidateCookie);
else if (curMap) {
- bool cookieAlreadyExists = curMap->existsCookie(candidateCookie);
- if (cookieAlreadyExists) {
- CookieLog("CookieManager - expired cookie exists in memory");
- ParsedCookie* expired = curMap->removeCookie(candidateCookie);
- // Cookie is useless, Remove the cookie from the backingstore if it exists
- // Backup check for BackingStoreCookieEntry incase someone incorrectly uses this enum
- if (postToBackingStore != BackingStoreCookieEntry && !expired->isSession()) {
- CookieLog("CookieManager - expired cookie is nonsession, deleting from db");
- m_cookieBackingStore->remove(expired);
- }
- delete expired;
+ // RemoveCookie will return 0 if the cookie doesn't exist.
+ ParsedCookie* expired = curMap->removeCookie(candidateCookie);
+ // Cookie is useless, Remove the cookie from the backingstore if it exists.
+ // Backup check for BackingStoreCookieEntry incase someone incorrectly uses this enum.
+ if (expired && postToBackingStore != BackingStoreCookieEntry && !expired->isSession()) {
+ CookieLog("CookieManager - expired cookie is nonsession, deleting from db");
+ m_cookieBackingStore->remove(expired);
}
+ delete expired;
+
} else
delete candidateCookie;
} else {
ASSERT(curMap);
- bool cookieAlreadyExists = curMap->existsCookie(candidateCookie);
- if (cookieAlreadyExists)
- update(curMap, candidateCookie, postToBackingStore);
- else
- addCookieToMap(curMap, candidateCookie, postToBackingStore);
+ addCookieToMap(curMap, candidateCookie, postToBackingStore);
}
}
void CookieManager::addCookieToMap(CookieMap* targetMap, ParsedCookie* candidateCookie, BackingStoreRemovalPolicy postToBackingStore)
{
+ ParsedCookie* prevCookie = targetMap->addOrReplaceCookie(candidateCookie);
+ if (prevCookie) {
+
+ CookieLog("CookieManager - updating new cookie - %s.\n", candidateCookie->toString().utf8().data());
+
+ // A cookie was replaced in targetMap.
+ // If old cookie is non-session and new one is, we have to delete it from backingstore
+ // If new cookie is non-session and old one is, we have to add it to backingstore
+ // If both sessions are non-session, then we update it in the backingstore
+ bool newIsSession = candidateCookie->isSession();
+ bool oldIsSession = prevCookie->isSession();
+
+ if (postToBackingStore == RemoveFromBackingStore) {
+ if (!newIsSession && !oldIsSession)
+ m_cookieBackingStore->update(candidateCookie);
+ else if (newIsSession && !oldIsSession) {
+ // Must manually decrease the counter because it was not counted when
+ // the cookie was removed in cookieVector.
+ removedCookie();
+ m_cookieBackingStore->remove(prevCookie);
+ } else if (!newIsSession && oldIsSession) {
+ // Must manually increase the counter because it was not counted when
+ // the cookie was added in cookieVector.
+ addedCookie();
+ m_cookieBackingStore->insert(candidateCookie);
+ }
+ }
+ delete prevCookie;
+ return;
+ }
+
+ CookieLog("CookieManager - adding new cookie - %s.\n", candidateCookie->toString().utf8().data());
+
ParsedCookie* oldestCookie = 0;
// Check if we have not reached the per cookie domain limit.
// If that is not true, we check if the global limit has been reached if backingstore mode is on
@@ -408,18 +435,14 @@
// then it means the global count will never exceed the limit
CookieLimitLog("CookieManager - local count: %d global count: %d", targetMap->count(), m_count);
- if (targetMap->count() >= s_maxCookieCountPerHost) {
+ if (targetMap->count() > s_maxCookieCountPerHost) {
CookieLog("CookieManager - deleting oldest cookie from this map due to domain count.\n");
oldestCookie = targetMap->removeOldestCookie();
- } else if (m_count >= s_globalMaxCookieCount && (postToBackingStore != DoNotRemoveFromBackingStore)) {
+ } else if (m_count > s_globalMaxCookieCount && (postToBackingStore != DoNotRemoveFromBackingStore)) {
CookieLimitLog("CookieManager - Global limit reached, initiate cookie limit clean up.");
initiateCookieLimitCleanUp();
}
- CookieLog("CookieManager - adding new cookie - %s.\n", candidateCookie->toString().utf8().data());
-
- targetMap->addCookie(candidateCookie);
-
// Only add non session cookie to the backing store.
if (postToBackingStore == RemoveFromBackingStore) {
if (oldestCookie && !oldestCookie->isSession()) {
@@ -433,38 +456,6 @@
delete oldestCookie;
}
-void CookieManager::update(CookieMap* targetMap, ParsedCookie* newCookie, BackingStoreRemovalPolicy postToBackingStore)
-{
- // If old cookie is non-session and new one is, we have to delete it from backingstore
- // If new cookie is non-session and old one is, we have to add it to backingstore
- // If both sessions are non-session, then we update it in the backingstore
-
- CookieLog("CookieManager - updating new cookie - %s.\n", newCookie->toString().utf8().data());
-
- ParsedCookie* oldCookie = targetMap->updateCookie(newCookie);
-
- ASSERT(oldCookie);
-
- if (postToBackingStore == RemoveFromBackingStore) {
- bool newIsSession = newCookie->isSession();
- bool oldIsSession = oldCookie->isSession();
- if (!newIsSession && !oldIsSession)
- m_cookieBackingStore->update(newCookie);
- else if (newIsSession && !oldIsSession) {
- // Must manually decrease the counter because it was not counted when
- // the cookie was removed in cookieMap.
- removedCookie();
- m_cookieBackingStore->remove(oldCookie);
- } else if (!newIsSession && oldIsSession) {
- // Must manually increase the counter because it was not counted when
- // the cookie was added in cookieMap.
- addedCookie();
- m_cookieBackingStore->insert(newCookie);
- }
- }
- delete oldCookie;
-}
-
void CookieManager::getBackingStoreCookies()
{
// This method should be called just after having created the cookieManager
Modified: trunk/Source/WebCore/platform/blackberry/CookieManager.h (109493 => 109494)
--- trunk/Source/WebCore/platform/blackberry/CookieManager.h 2012-03-02 04:23:25 UTC (rev 109493)
+++ trunk/Source/WebCore/platform/blackberry/CookieManager.h 2012-03-02 04:26:40 UTC (rev 109494)
@@ -120,8 +120,7 @@
bool shouldRejectForSecurityReason(const ParsedCookie*, const KURL&);
- void addCookieToMap(CookieMap*, ParsedCookie*, BackingStoreRemovalPolicy);
- void update(CookieMap*, ParsedCookie*, BackingStoreRemovalPolicy);
+ void addCookieToMap(CookieMap* targetMap, ParsedCookie* candidateCookie, BackingStoreRemovalPolicy postToBackingStore);
CookieMap* findOrCreateCookieMap(CookieMap* protocolMap, const String& domain, bool findOnly);
Modified: trunk/Source/WebCore/platform/blackberry/CookieMap.cpp (109493 => 109494)
--- trunk/Source/WebCore/platform/blackberry/CookieMap.cpp 2012-03-02 04:23:25 UTC (rev 109493)
+++ trunk/Source/WebCore/platform/blackberry/CookieMap.cpp 2012-03-02 04:26:40 UTC (rev 109494)
@@ -54,46 +54,35 @@
deleteAllCookiesAndDomains();
}
-bool CookieMap::existsCookie(const ParsedCookie* cookie) const
+ParsedCookie* CookieMap::addOrReplaceCookie(ParsedCookie* cookie)
{
- String key = cookie->name() + cookie->path();
- return m_cookieMap.contains(key);
-}
+ CookieLog("CookieMap - Attempting to add cookie - %s", cookie->name().utf8().data());
-void CookieMap::addCookie(ParsedCookie* cookie)
-{
- String key = cookie->name() + cookie->path();
+ ParsedCookie* prevCookie = 0;
+ for (int i = 0; i < m_cookieVector.size(); i++) {
+ if (m_cookieVector[i]->name() == cookie->name() && m_cookieVector[i]->path() == cookie->path()) {
+ prevCookie = m_cookieVector[i];
+ m_cookieVector[i] = cookie;
+ if (prevCookie == m_oldestCookie)
+ updateOldestCookie();
+ return prevCookie;
+ }
+ }
- CookieLog("CookieMap - Attempting to add cookie - %s", cookie->name().utf8().data());
-
- ASSERT(!m_cookieMap.contains(key));
- m_cookieMap.add(key, cookie);
+ m_cookieVector.append(cookie);
if (!cookie->isSession())
cookieManager().addedCookie();
if (!m_oldestCookie || m_oldestCookie->lastAccessed() > cookie->lastAccessed())
m_oldestCookie = cookie;
+ return 0;
}
-ParsedCookie* CookieMap::updateCookie(ParsedCookie* newCookie)
+ParsedCookie* CookieMap::removeCookieAtIndex(int position, const ParsedCookie* cookie)
{
- String key = newCookie->name() + newCookie->path();
- ParsedCookie* oldCookie = m_cookieMap.take(key);
- ASSERT(oldCookie);
- m_cookieMap.add(key, newCookie);
- if (oldCookie == m_oldestCookie)
- updateOldestCookie();
- return oldCookie;
-}
+ ASSERT(0 <= position && position < m_cookieVector.size());
+ ParsedCookie* prevCookie = m_cookieVector[position];
+ m_cookieVector.remove(position);
-ParsedCookie* CookieMap::removeCookie(const ParsedCookie* cookie)
-{
- // Find a previous entry for deletion
- String key = cookie->name() + cookie->path();
- ParsedCookie* prevCookie = m_cookieMap.take(key);
-
- if (!prevCookie)
- return 0;
-
if (prevCookie == m_oldestCookie)
updateOldestCookie();
else if (prevCookie != cookie) {
@@ -109,6 +98,16 @@
return prevCookie;
}
+ParsedCookie* CookieMap::removeCookie(const ParsedCookie* cookie)
+{
+ size_t cookieCount = m_cookieVector.size();
+ for (int position = 0; position < cookieCount; ++position) {
+ if (m_cookieVector[position]->name() == cookie->name() && m_cookieVector[position]->path() == cookie->path())
+ return removeCookieAtIndex(position, cookie);
+ }
+ return 0;
+}
+
CookieMap* CookieMap::getSubdomainMap(const String& subdomain)
{
#if ENABLE_COOKIE_DEBUG
@@ -126,19 +125,22 @@
void CookieMap::getAllCookies(Vector<ParsedCookie*>* stackOfCookies)
{
- CookieLog("CookieMap - Attempting to copy Map:%s cookies with %d cookies into vectors", m_name.utf8().data(), m_cookieMap.size());
+ CookieLog("CookieMap - Attempting to copy Map:%s cookies with %d cookies into vectors", m_name.utf8().data(), m_cookieVector.size());
- Vector<ParsedCookie*> newCookies;
- copyValuesToVector(m_cookieMap, newCookies);
- for (size_t i = 0; i < newCookies.size(); i++) {
- ParsedCookie* newCookie = newCookies[i];
+ stackOfCookies->reserveCapacity(stackOfCookies->size() + m_cookieVector.size());
+
+ int position = 0;
+ while (position < m_cookieVector.size()) {
+ ParsedCookie* newCookie = m_cookieVector[position];
if (newCookie->hasExpired()) {
// Notice that we don't delete from backingstore. These expired cookies will be
// deleted when manager loads the backingstore again.
- ParsedCookie* expired = removeCookie(newCookie);
+ ParsedCookie* expired = removeCookieAtIndex(position, newCookie);
delete expired;
- } else
+ } else {
stackOfCookies->append(newCookie);
+ position++;
+ }
}
CookieLog("CookieMap - stack of cookies now have %d cookies in it", (*stackOfCookies).size());
@@ -177,24 +179,25 @@
void CookieMap::updateOldestCookie()
{
- if (!m_cookieMap.size())
+ size_t size = m_cookieVector.size();
+ if (!size) {
m_oldestCookie = 0;
- else {
- HashMap<String, ParsedCookie*>::iterator it = m_cookieMap.begin();
- m_oldestCookie = it->second;
- ++it;
- for (; it != m_cookieMap.end(); ++it)
- if (m_oldestCookie->lastAccessed() > it->second->lastAccessed())
- m_oldestCookie = it->second;
+ return;
}
+
+ m_oldestCookie = m_cookieVector[0];
+ for (int i = 1; i < size; ++i) {
+ if (m_oldestCookie->lastAccessed() > m_cookieVector[i]->lastAccessed())
+ m_oldestCookie = m_cookieVector[i];
+ }
}
void CookieMap::deleteAllCookiesAndDomains()
{
deleteAllValues(m_subdomains);
m_subdomains.clear();
- deleteAllValues(m_cookieMap);
- m_cookieMap.clear();
+ deleteAllValues(m_cookieVector);
+ m_cookieVector.clear();
m_oldestCookie = 0;
}
Modified: trunk/Source/WebCore/platform/blackberry/CookieMap.h (109493 => 109494)
--- trunk/Source/WebCore/platform/blackberry/CookieMap.h 2012-03-02 04:23:25 UTC (rev 109493)
+++ trunk/Source/WebCore/platform/blackberry/CookieMap.h 2012-03-02 04:26:40 UTC (rev 109494)
@@ -51,17 +51,14 @@
CookieMap(const String& name = "");
~CookieMap();
- unsigned int count() const { return m_cookieMap.size(); }
+ unsigned int count() const { return m_cookieVector.size(); }
const String& getName() const { return m_name; }
- void addCookie(ParsedCookie*);
-
// Returning the original cookie object so manager can keep a reference to the updates in the database queue.
- ParsedCookie* updateCookie(ParsedCookie*);
+ ParsedCookie* addOrReplaceCookie(ParsedCookie*);
// Need to return the reference to the removed cookie so manager can deal with it (garbage collect).
ParsedCookie* removeCookie(const ParsedCookie*);
- bool existsCookie(const ParsedCookie*) const;
// Returns a map with that given subdomain.
CookieMap* getSubdomainMap(const String&);
@@ -74,11 +71,9 @@
private:
void updateOldestCookie();
+ ParsedCookie* removeCookieAtIndex(int position, const ParsedCookie*);
- // The key is the tuple (name, path).
- // The spec asks to have also domain, which is implied by choosing the CookieMap relevant to the domain.
- HashMap<String, ParsedCookie*> m_cookieMap;
-
+ Vector<ParsedCookie*> m_cookieVector;
// The key is a subsection of the domain.
// ex: if inserting accounts.google.com & this cookiemap is "com", this subdomain map will contain "google"
// the "google" cookiemap will contain "accounts" in its subdomain map.
