Use a lock to deal with concurrent access to the m_evictedBackings

cc/prioritized_resource_manager.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/prioritized_resource_manager.h"
 
 #include "base/debug/trace_event.h"
 #include "base/stl_util.h"
 #include "cc/prioritized_resource.h"
 #include "cc/priority_calculator.h"
@@ skipping 18 matching lines @@
     , m_memoryVisibleLastPushedBytes(0)
     , m_memoryVisibleAndNearbyLastPushedBytes(0)
 {
 }
 
 PrioritizedResourceManager::~PrioritizedResourceManager()
 {
     while (m_textures.size() > 0)
         unregisterTexture(*m_textures.begin());
 
-    deleteAllEvictedBackings();
+    unlinkAndClearEvictedBackings();
     DCHECK(m_evictedBackings.empty());
 
     // Each remaining backing is a leaked opengl texture. There should be none.
     DCHECK(m_backings.empty());
 }
 
 size_t PrioritizedResourceManager::memoryVisibleBytes() const
 {
     DCHECK(m_proxy->isImplThread());
     return m_memoryVisibleLastPushedBytes;
@@ skipping 249 matching lines @@
             break;
         wastedMemory += (*it)->bytes();
     }
     size_t tenPercentOfMemory = m_memoryAvailableBytes / 10;
     if (wastedMemory > tenPercentOfMemory)
         evictBackingsToReduceMemory(memoryUseBytes() - (wastedMemory - tenPercentOfMemory),
                                     PriorityCalculator::allowEverythingCutoff(),
                                     EvictOnlyRecyclable,
                                     UnlinkBackings,
                                     resourceProvider);
-
-    // Unlink all evicted backings
-    for (BackingList::const_iterator it = m_evictedBackings.begin(); it != m_evictedBackings.end(); ++it) {
-        if ((*it)->owner())
-            (*it)->owner()->unlink();
-    }
-
-    // And clear the list of evicted backings
-    deleteAllEvictedBackings();
 }
 
 void PrioritizedResourceManager::clearAllMemory(ResourceProvider* resourceProvider)
 {
     DCHECK(m_proxy->isImplThread() && m_proxy->isMainThreadBlocked());
     DCHECK(resourceProvider);
     evictBackingsToReduceMemory(0,
                                 PriorityCalculator::allowEverythingCutoff(),
                                 EvictAnything,
                                 DoNotUnlinkBackings,
                                 resourceProvider);
 }
 
 bool PrioritizedResourceManager::reduceMemoryOnImplThread(size_t limitBytes, int priorityCutoff, ResourceProvider* resourceProvider)
 {
     DCHECK(m_proxy->isImplThread());
     DCHECK(resourceProvider);
     // If we are in the process of uploading a new frame then the backings at the very end of
     // the list are not sorted by priority. Sort them before doing the eviction.
     if (m_backingsTailNotSorted)
         sortBackings();
     return evictBackingsToReduceMemory(limitBytes,
                                        priorityCutoff,
                                        EvictAnything,
                                        DoNotUnlinkBackings,
                                        resourceProvider);
 }
 
-void PrioritizedResourceManager::getEvictedBackings(BackingList& evictedBackings)
-{
-    DCHECK(m_proxy->isImplThread());
-    evictedBackings.clear();
-    evictedBackings.insert(evictedBackings.begin(), m_evictedBackings.begin(), m_evictedBackings.end());
-    for (BackingList::const_iterator it = evictedBackings.begin(); it != evictedBackings.end(); ++it) {
-        PrioritizedResource::Backing* backing = (*it);
-        CHECK(!backing->m_inMainThreadEvictedList);
-        backing->m_inMainThreadEvictedList = true;
-    }
-}
-
-void PrioritizedResourceManager::unlinkEvictedBackings(const BackingList& evictedBackings)
+void PrioritizedResourceManager::unlinkAndClearEvictedBackings()
 {
     DCHECK(m_proxy->isMainThread());
-    for (BackingList::const_iterator it = evictedBackings.begin(); it != evictedBackings.end(); ++it) {
-        PrioritizedResource::Backing* backing = (*it);
-        CHECK(backing->m_inMainThreadEvictedList);
-        backing->m_inMainThreadEvictedList = false;
-        if (backing->owner()) {
-            CHECK(backing->owner()->backing());
-            CHECK(backing->owner()->backing() == backing);
-            backing->owner()->unlink();
-        }
-    }
-}
-
-void PrioritizedResourceManager::deleteAllEvictedBackings()
-{
-    DCHECK(m_proxy->isMainThread() || (m_proxy->isImplThread() && m_proxy->isMainThreadBlocked()));
+    base::AutoLock scoped_lock(m_evictedBackingsLock);
     for (BackingList::const_iterator it = m_evictedBackings.begin(); it != m_evictedBackings.end(); ++it) {
         PrioritizedResource::Backing* backing = (*it);
-        CHECK(!backing->owner());
+        if (backing->owner())
+            backing->owner()->unlink();
         delete backing;
     }
     m_evictedBackings.clear();
 }
 
-bool PrioritizedResourceManager::linkedEvictedBackingsExist() const
+bool PrioritizedResourceManager::linkedEvictedBackingsExist()

[piman, 2012/11/29 22:34:11]

nit: you can keep this const by making m_evictedBackingsLock mutable - it's a
common idiom I think.

[ccameron, 2012/11/30 00:02:17]

Good idea. Done.

 {
+    DCHECK(m_proxy->isImplThread() && m_proxy->isMainThreadBlocked());
+    base::AutoLock scoped_lock(m_evictedBackingsLock);
     for (BackingList::const_iterator it = m_evictedBackings.begin(); it != m_evictedBackings.end(); ++it) {
         if ((*it)->owner())
             return true;
     }
     return false;
 }
 
 void PrioritizedResourceManager::registerTexture(PrioritizedResource* texture)
 {
     DCHECK(m_proxy->isMainThread());
@@ skipping 43 matching lines @@
     DCHECK(!m_backings.empty());
     PrioritizedResource::Backing* backing = m_backings.front();
 
     // Note that we create a backing and its resource at the same time, but we
     // delete the backing structure and its resource in two steps. This is because
     // we can delete the resource while the main thread is running, but we cannot
     // unlink backings while the main thread is running.
     backing->deleteResource(resourceProvider);
     m_memoryUseBytes -= backing->bytes();
     m_backings.pop_front();
+    base::AutoLock scoped_lock(m_evictedBackingsLock);
     m_evictedBackings.push_back(backing);
 }
 
 void PrioritizedResourceManager::assertInvariants()
 {
 #ifndef NDEBUG
     DCHECK(m_proxy->isImplThread() && m_proxy->isMainThreadBlocked());
 
     // If we hit any of these asserts, there is a bug in this class. To see
     // where the bug is, call this function at the beginning and end of
     // every public function.
 
     // Backings/textures must be doubly-linked and only to other backings/textures in this manager.
     for (BackingList::iterator it = m_backings.begin(); it != m_backings.end(); ++it) {
         if ((*it)->owner()) {
             DCHECK(ContainsKey(m_textures, (*it)->owner()));
             DCHECK((*it)->owner()->backing() == (*it));
         }
     }
     for (TextureSet::iterator it = m_textures.begin(); it != m_textures.end(); ++it) {
         PrioritizedResource* texture = (*it);
         PrioritizedResource::Backing* backing = texture->backing();
+        base::AutoLock scoped_lock(m_evictedBackingsLock);
         if (backing) {
             if (backing->resourceHasBeenDeleted()) {
                 DCHECK(std::find(m_backings.begin(), m_backings.end(), backing) == m_backings.end());
                 DCHECK(std::find(m_evictedBackings.begin(), m_evictedBackings.end(), backing) != m_evictedBackings.end());
             } else {
                 DCHECK(std::find(m_backings.begin(), m_backings.end(), backing) != m_backings.end());
                 DCHECK(std::find(m_evictedBackings.begin(), m_evictedBackings.end(), backing) == m_evictedBackings.end());
             }
             DCHECK(backing->owner() == texture);
         }
@@ skipping 18 matching lines @@
     }
 #endif
 }
 
 const Proxy* PrioritizedResourceManager::proxyForDebug() const
 {
     return m_proxy;
 }
 
 }  // namespace cc