Changeset 116969 in webkit

Timestamp:

May 14, 2012 11:04:17 AM (12 years ago)

Author:

kling@webkit.org

Message:

REGRESSION(r116796): Assertion failure in API tests
<​http://webkit.org/b/86378>
<rdar://problem/10777218>

Reviewed by Anders Carlsson.

Invalidate the plugin work queue in ~WebContext to avoid hitting
the !m_isValid assertion in ~WorkQueue later on.

• UIProcess/WebContext.cpp:

(WebKit::WebContext::~WebContext):

Location:

trunk/Source

Files:

• WebCore/loader/icon/IconDatabase.cpp (modified) (9 diffs)
• WebCore/loader/icon/IconDatabase.h (modified) (4 diffs)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/UIProcess/WebContext.cpp (modified) (1 diff)

trunk/Source/WebCore/loader/icon/IconDatabase.cpp

@@ -223 +223 @@
 
     MutexLocker locker(m_urlAndIconLock);
+
+    if (m_retainOrReleaseIconRequested)
+        performPendingRetainAndReleaseOperations();
     
     String pageURLCopy; // Creates a null string for easy testing
@@ -390 +393 @@
 }
 
-
-void IconDatabase::retainIconForPageURL(const String& pageURLOriginal)
-{    
+void IconDatabase::retainIconForPageURL(const String& pageURL)
+{
     ASSERT_NOT_SYNC_THREAD();
-    
-    // Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
-    
-    if (!isEnabled() || !documentCanHaveIcon(pageURLOriginal))
+
+    if (!isEnabled() || !documentCanHaveIcon(pageURL))
         return;
        
-    MutexLocker locker(m_urlAndIconLock);
-
+    MutexLocker locker(m_urlsToRetainOrReleaseLock);
+    m_urlsToRetain.append(pageURL);
+    scheduleOrDeferSyncTimer();
+}
+
+void IconDatabase::performRetainIconForPageURL(const String& pageURLOriginal)
+{
     PageURLRecord* record = m_pageURLToRecordMap.get(pageURLOriginal);
     
@@ -435 +440 @@
 }
 
-void IconDatabase::releaseIconForPageURL(const String& pageURLOriginal)
+void IconDatabase::releaseIconForPageURL(const String& pageURL)
 {
     ASSERT_NOT_SYNC_THREAD();
@@ -441 +446 @@
     // Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
     
-    if (!isEnabled() || !documentCanHaveIcon(pageURLOriginal))
-        return;
-    
-    MutexLocker locker(m_urlAndIconLock);
-
+    if (!isEnabled() || !documentCanHaveIcon(pageURL))
+        return;
+
+    MutexLocker locker(m_urlsToRetainOrReleaseLock);
+    m_urlsToRelease.append(pageURL);
+    m_retainOrReleaseIconRequested = true;
+    scheduleOrDeferSyncTimer();
+}
+
+void IconDatabase::performReleaseIconForPageURL(const String& pageURLOriginal)
+{
     // Check if this pageURL is actually retained
     if (!m_retainedPageURLs.contains(pageURLOriginal)) {
@@ -499 +510 @@
     
     delete pageRecord;
-
-    if (isOpen())
-        scheduleOrDeferSyncTimer();
 }
 
@@ -737 +745 @@
 {
     MutexLocker locker(m_urlAndIconLock);
+
+    if (m_retainOrReleaseIconRequested)
+        performPendingRetainAndReleaseOperations();
+
     return m_retainedPageURLs.size();
 }
@@ -1307 +1319 @@
     {
         MutexLocker locker(m_urlAndIconLock);
-        
+
+        if (m_retainOrReleaseIconRequested)
+            performPendingRetainAndReleaseOperations();
+
         for (unsigned i = 0; i < urls.size(); ++i) {
             if (!m_retainedPageURLs.contains(urls[i])) {
@@ -1388 +1403 @@
         if (m_threadTerminationRequested)
             break;
+
+        if (m_retainOrReleaseIconRequested) {
+            MutexLocker locker(m_urlAndIconLock);
+            performPendingRetainAndReleaseOperations();
+        }
         
         bool didAnyWork = true;
@@ -1470 +1490 @@
         m_disabledSuddenTerminationForSyncThread = false;
     }
+}
+
+void IconDatabase::performPendingRetainAndReleaseOperations()
+{
+    ASSERT(m_retainOrReleaseIconRequested);
+
+    // NOTE: The caller is assumed to hold m_urlAndIconLock.
+
+    MutexLocker vectorLocker(m_urlsToRetainOrReleaseLock);
+
+    for (unsigned i = 0; i < m_urlsToRetain.size(); ++i)
+        performRetainIconForPageURL(m_urlsToRetain[i]);
+    for (unsigned i = 0; i < m_urlsToRelease.size(); ++i)
+        performReleaseIconForPageURL(m_urlsToRelease[i]);
+    printf("Retained %lu, released %lu\n", m_urlsToRetain.size(), m_urlsToRelease.size());
+
+    m_urlsToRetain.clear();
+    m_urlsToRelease.clear();
+    m_retainOrReleaseIconRequested = false;
 }
 

trunk/Source/WebCore/loader/icon/IconDatabase.h

@@ -160 +160 @@
     bool m_syncThreadHasWorkToDo;
     bool m_disabledSuddenTerminationForSyncThread;
+    bool m_retainOrReleaseIconRequested;
 
     Mutex m_urlAndIconLock;
@@ -177 +178 @@
     HashSet<String> m_pageURLsInterestedInIcons;
     HashSet<IconRecord*> m_iconsPendingReading;
+
+    Mutex m_urlsToRetainOrReleaseLock;
+    // Holding m_urlsToRetainOrReleaseLock is required when accessing any of the following data structures.
+    Vector<String> m_urlsToRetain;
+    Vector<String> m_urlsToRelease;
 
 // *** Sync Thread Only ***
@@ -203 +209 @@
     void deleteAllPreparedStatements();
     void* cleanupSyncThread();
+    void performRetainIconForPageURL(const String&);
+    void performReleaseIconForPageURL(const String&);
 
     // Record (on disk) whether or not Safari 2-style icons were imported (once per dataabse)
@@ -221 +229 @@
     void removeIconFromSQLDatabase(const String& iconURL);
     void writeIconSnapshotToSQLDatabase(const IconSnapshot&);    
-    
+
+    void performPendingRetainAndReleaseOperations();
+
     // Methods to dispatch client callbacks on the main thread
     void dispatchDidImportIconURLForPageURLOnMainThread(const String&);

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2012-05-14  Andreas Kling  <kling@webkit.org>
+
+        REGRESSION(r116796): Assertion failure in API tests
+        <http://webkit.org/b/86378>
+        <rdar://problem/10777218>
+
+        Reviewed by Anders Carlsson.
+
+        Invalidate the plugin work queue in ~WebContext to avoid hitting
+        the !m_isValid assertion in ~WorkQueue later on.
+
+        * UIProcess/WebContext.cpp:
+        (WebKit::WebContext::~WebContext):
+
 2012-05-14  Csaba Osztrogonác  <ossy@webkit.org>
 

trunk/Source/WebKit2/UIProcess/WebContext.cpp

@@ -167 +167 @@
 WebContext::~WebContext()
 {
+    m_pluginWorkQueue.invalidate();
+
     if (m_process && m_process->isValid())
         m_process->connection()->removeQueueClient(this);