[BlackBerry] Cookie database isn't loaded into memory in some rare cases

https://bugs.webkit.org/show_bug.cgi?id=109202
PR 286189

Patch by Otto Derek Cheung <otcheung@rim.com> on 2013-02-07
Reviewed by Yong Li.
Internally Reviewed by Konrad Piascik.

If a get/setCookie call is made before the database is loaded, or if there's some
kind of error that causes the loading of the database to fail in the constructor
of CookieManager, the browser will get into a state where it seems like cookie is
permanenty disabled.

Instead of logging the errors and redispatching the setCookie, we should do a force sync
to load the cookie database before continuing.

Since the bug is so difficult to reproduce (I never did so myself), I did the follow test
to make sure the code path is correct:
1) Make sure original implementation is retained - open and loading done in the constructor
2) Removed opening and loading in constructor, the new calls in get/setcookies loaded the db just fine (although with
an initial lag because we are blocking WKT while performing SQLite options).
3) Removed loading in constructor, the new calls loaded the db just fine.

* platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.cpp:
(WebCore::CookieDatabaseBackingStore::openAndLoadDatabaseSynchronously):
(WebCore):
* platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.h:
(CookieDatabaseBackingStore):
* platform/blackberry/CookieManager.cpp:
(WebCore::CookieManager::setCookies):
(WebCore::CookieManager::getCookie):
(WebCore::CookieManager::generateHtmlFragmentForCookies):
(WebCore::CookieManager::getRawCookies):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142150 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 2b060e7123b19544e8a94b4cfee1c7b864e3be46..879e47c77f3f1dc783a6356a06ce530a21bc0afd 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,38 @@
+2013-02-07  Otto Derek Cheung  <otcheung@rim.com>
+
+        [BlackBerry] Cookie database isn't loaded into memory in some rare cases
+        https://bugs.webkit.org/show_bug.cgi?id=109202
+        PR 286189
+
+        Reviewed by Yong Li.
+        Internally Reviewed by Konrad Piascik.
+
+        If a get/setCookie call is made before the database is loaded, or if there's some
+        kind of error that causes the loading of the database to fail in the constructor
+        of CookieManager, the browser will get into a state where it seems like cookie is
+        permanenty disabled.
+
+        Instead of logging the errors and redispatching the setCookie, we should do a force sync
+        to load the cookie database before continuing.
+
+        Since the bug is so difficult to reproduce (I never did so myself), I did the follow test
+        to make sure the code path is correct:
+        1) Make sure original implementation is retained - open and loading done in the constructor
+        2) Removed opening and loading in constructor, the new calls in get/setcookies loaded the db just fine (although with
+        an initial lag because we are blocking WKT while performing SQLite options).
+        3) Removed loading in constructor, the new calls loaded the db just fine.
+
+        * platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.cpp:
+        (WebCore::CookieDatabaseBackingStore::openAndLoadDatabaseSynchronously):
+        (WebCore):
+        * platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.h:
+        (CookieDatabaseBackingStore):
+        * platform/blackberry/CookieManager.cpp:
+        (WebCore::CookieManager::setCookies):
+        (WebCore::CookieManager::getCookie):
+        (WebCore::CookieManager::generateHtmlFragmentForCookies):
+        (WebCore::CookieManager::getRawCookies):
+
 2013-02-07  Max Vujovic  <mvujovic@adobe.com>
 
         [CSS Shaders] Add WebKitCSSFilterRule to DOMWindow.idl

diff --git a/Source/WebCore/platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.cpp b/Source/WebCore/platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.cpp
index ef0fcc8309e222aeb712d805bf28a00a250b0d2c..9076146a23ae30217a5f24021af1dd703671c02c 100644 (file)
--- a/Source/WebCore/platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.cpp
+++ b/Source/WebCore/platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.cpp
@@ -311,6 +311,23 @@ Vector<ParsedCookie*>* CookieDatabaseBackingStore::invokeGetCookiesWithLimit(uns
     return cookies;
 }
 
+void CookieDatabaseBackingStore::openAndLoadDatabaseSynchronously(const String& cookieJar)
+{
+    CookieLog("CookieBackingStore - loading database into CookieManager immediately");
+
+    if (m_db.isOpen()) {
+        if (isCurrentThread())
+            BlackBerry::Platform::webKitThreadMessageClient()->dispatchSyncMessage(createMethodCallMessage(&CookieManager::getBackingStoreCookies, &cookieManager()));
+        else
+            cookieManager().getBackingStoreCookies();
+    } else {
+        if (isCurrentThread())
+            invokeOpen(cookieJar);
+        else
+            dispatchSyncMessage(createMethodCallMessage(&CookieDatabaseBackingStore::invokeOpen, this, cookieJar));
+    }
+}
+
 void CookieDatabaseBackingStore::sendChangesToDatabaseSynchronously()
 {
     CookieLog("CookieBackingStore - sending to database immediately");

diff --git a/Source/WebCore/platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.h b/Source/WebCore/platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.h
index 539e29da9fbcbc8b4710bb6ad1d3f7a2e7738667..8834f4ef34bde334a1941af10543f114a439c4b1 100644 (file)
--- a/Source/WebCore/platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.h
+++ b/Source/WebCore/platform/blackberry/CookieDatabaseBackingStore/CookieDatabaseBackingStore.h
@@ -59,6 +59,7 @@ public:
     // If a limit is not set, the method will return all cookies in the database
     void getCookiesFromDatabase(Vector<ParsedCookie*>& stackOfCookies, unsigned int limit = 0);
 
+    void openAndLoadDatabaseSynchronously(const String& cookieJar);
     void sendChangesToDatabaseSynchronously();
 
     // MessageClient methods

diff --git a/Source/WebCore/platform/blackberry/CookieManager.cpp b/Source/WebCore/platform/blackberry/CookieManager.cpp
index accd3b634e424baf65a76479777765b42f4b7d8a..bdb65f8780c064ce190750101b8de64e99b6734a 100644 (file)
--- a/Source/WebCore/platform/blackberry/CookieManager.cpp
+++ b/Source/WebCore/platform/blackberry/CookieManager.cpp
@@ -126,15 +126,9 @@ static bool shouldIgnoreScheme(const String& protocol)
 
 void CookieManager::setCookies(const KURL& url, const String& value, CookieFilter filter)
 {
-    // Dispatch the message because the database cookies are not loaded in memory yet.
-    if (!m_syncedWithDatabase && !m_privateMode) {
-        typedef void (WebCore::CookieManager::*FunctionType)(const KURL&, const String&, CookieFilter);
-
-        BlackBerry::Platform::webKitThreadMessageClient()->dispatchMessage(
-            BlackBerry::Platform::createMethodCallMessage<FunctionType, CookieManager, const KURL, const String, CookieFilter>(
-                &CookieManager::setCookies, this, url, value, filter));
-        return;
-    }
+    // If the database hasn't been sync-ed at this point, force a sync load
+    if (!m_syncedWithDatabase && !m_privateMode)
+        m_cookieBackingStore->openAndLoadDatabaseSynchronously(cookieJar());
 
     CookieLog("CookieManager - Setting cookies");
     CookieParser parser(url);
@@ -148,14 +142,9 @@ void CookieManager::setCookies(const KURL& url, const String& value, CookieFilte
 
 void CookieManager::setCookies(const KURL& url, const Vector<String>& cookies, CookieFilter filter)
 {
-    // Dispatch the message because the database cookies are not loaded in memory yet.
-    if (!m_syncedWithDatabase && !m_privateMode) {
-        typedef void (WebCore::CookieManager::*FunctionType)(const KURL&, const Vector<String>&, CookieFilter);
-        BlackBerry::Platform::webKitThreadMessageClient()->dispatchMessage(
-            BlackBerry::Platform::createMethodCallMessage<FunctionType, CookieManager, const KURL, const Vector<String>, CookieFilter>(
-                &CookieManager::setCookies, this, url, cookies, filter));
-        return;
-    }
+    // If the database hasn't been sync-ed at this point, force a sync load
+    if (!m_syncedWithDatabase && !m_privateMode)
+        m_cookieBackingStore->openAndLoadDatabaseSynchronously(cookieJar());
 
     CookieLog("CookieManager - Setting cookies");
     CookieParser parser(url);
@@ -168,10 +157,9 @@ void CookieManager::setCookies(const KURL& url, const Vector<String>& cookies, C
 
 String CookieManager::getCookie(const KURL& url, CookieFilter filter) const
 {
-    if (!m_syncedWithDatabase && !m_privateMode) {
-        LOG_ERROR("CookieManager is calling getCookies before database values are loaded.");
-        return String();
-    }
+    // If the database hasn't been sync-ed at this point, force a sync load
+    if (!m_syncedWithDatabase && !m_privateMode)
+        m_cookieBackingStore->openAndLoadDatabaseSynchronously(cookieJar());
 
     Vector<ParsedCookie*> rawCookies;
     rawCookies.reserveInitialCapacity(s_maxCookieCountPerHost);
@@ -198,10 +186,9 @@ String CookieManager::getCookie(const KURL& url, CookieFilter filter) const
 
 String CookieManager::generateHtmlFragmentForCookies()
 {
-    if (!m_syncedWithDatabase && !m_privateMode) {
-        LOG_ERROR("CookieManager is calling generateHtmlFragmentForCookies before database values are loaded.");
-        return String();
-    }
+    // If the database hasn't been sync-ed at this point, force a sync load
+    if (!m_syncedWithDatabase && !m_privateMode)
+        m_cookieBackingStore->openAndLoadDatabaseSynchronously(cookieJar());
 
     CookieLog("CookieManager - generateHtmlFragmentForCookies\n");
 
@@ -238,10 +225,9 @@ String CookieManager::generateHtmlFragmentForCookies()
 
 void CookieManager::getRawCookies(Vector<ParsedCookie*> &stackOfCookies, const KURL& requestURL, CookieFilter filter) const
 {
-    if (!m_syncedWithDatabase && !m_privateMode) {
-        LOG_ERROR("CookieManager is calling getRawCookies before database values are loaded.");
-        return;
-    }
+    // Force a sync load of the database
+    if (!m_syncedWithDatabase && !m_privateMode)
+        m_cookieBackingStore->openAndLoadDatabaseSynchronously(cookieJar());
 
     CookieLog("CookieManager - getRawCookies - processing url with domain - %s & protocol: %s & path: %s\n", requestURL.host().utf8().data(), requestURL.protocol().utf8().data(), requestURL.path().utf8().data());