Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

third_party/WebKit/Source/core/fetch/Resource.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
@@ skipping 533 matching lines @@
     m_redirectChain.append(RedirectPair(newRequest, redirectResponse));
 }
 
 void Resource::setResponse(const ResourceResponse& response)
 {
     m_response = response;
     if (m_response.wasFetchedViaServiceWorker())
         m_cacheHandler = ServiceWorkerResponseCachedMetadataHandler::create(this, m_fetcherSecurityOrigin.get());
 }
 
-bool Resource::unlock()
-{
-    if (!m_data)
-        return false;
-
-    if (!m_data->isLocked())
-        return true;
-
-    if (!memoryCache()->contains(this) || hasClientsOrObservers() || !isLoaded() || !isSafeToUnlock())
-        return false;
-
-    if (RuntimeEnabledFeatures::doNotUnlockSharedBufferEnabled())
-        return false;
-
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(EnumerationHistogram, unlockHistogram, new EnumerationHistogram("Blink.SharedBuffer.Unlock", kLastResourceType));
-    unlockHistogram.count(getType());
-
-    m_data->unlock();
-    return true;
-}
-
 void Resource::responseReceived(const ResourceResponse& response, std::unique_ptr<WebDataConsumerHandle>)
 {
     m_responseTimestamp = currentTime();
     if (m_preloadDiscoveryTime) {
         int timeSinceDiscovery = static_cast<int>(1000 * (monotonicallyIncreasingTime() - m_preloadDiscoveryTime));
         DEFINE_STATIC_LOCAL(CustomCountHistogram, preloadDiscoveryToFirstByteHistogram, ("PreloadScanner.TTFB", 0, 10000, 50));
         preloadDiscoveryToFirstByteHistogram.count(timeSinceDiscovery);
     }
 
     if (m_isRevalidating) {
@@ skipping 84 matching lines @@
         return true;
     if (type == Resource::Script)
         return true;
     if (type == Resource::Font)
         return true;
     return false;
 }
 
 void Resource::willAddClientOrObserver()
 {
-    ASSERT(!isPurgeable());
     if (m_preloadResult == PreloadNotReferenced) {
         if (isLoaded())
             m_preloadResult = PreloadReferencedWhileComplete;
         else if (isLoading())
             m_preloadResult = PreloadReferencedWhileLoading;
         else
             m_preloadResult = PreloadReferenced;
 
         if (m_preloadDiscoveryTime) {
             int timeSinceDiscovery = static_cast<int>(1000 * (monotonicallyIncreasingTime() - m_preloadDiscoveryTime));
@@ skipping 64 matching lines @@
 }
 
 void Resource::allClientsAndObserversRemoved()
 {
     if (!m_loader)
         return;
     if (m_type == Raw)
         cancelTimerFired(&m_cancelTimer);
     else if (!m_cancelTimer.isActive())
         m_cancelTimer.startOneShot(0, BLINK_FROM_HERE);
-
-    unlock();
 }
 
 void Resource::cancelTimerFired(TimerBase* timer)
 {
     ASSERT_UNUSED(timer, timer == &m_cancelTimer);
     if (!hasClientsOrObservers() && m_loader)
         m_loader->cancel();
 }
 
 void Resource::setDecodedSize(size_t decodedSize)
@@ skipping 49 matching lines @@
     if (scheduled && m_clientsAwaitingCallback.isEmpty())
         ResourceCallback::callbackHandler().cancel(this);
 
     // Prevent the case when there are clients waiting but no callback scheduled.
     ASSERT(m_clientsAwaitingCallback.isEmpty() || scheduled);
 }
 
 void Resource::prune()
 {
     destroyDecodedDataIfPossible();
-    unlock();
 }
 
 void Resource::onMemoryDump(WebMemoryDumpLevelOfDetail levelOfDetail, WebProcessMemoryDump* memoryDump) const
 {
     static const size_t kMaxURLReportLength = 128;
     static const int kMaxResourceClientToShowInMemoryInfra = 10;
 
     const String dumpName = getMemoryDumpName();
     WebMemoryAllocatorDump* dump = memoryDump->createMemoryAllocatorDump(dumpName);
     dump->addScalar("encoded_size", "bytes", m_encodedSize);
-    if (m_data && m_data->isLocked())
+    if (m_data)
         dump->addScalar("live_size", "bytes", m_encodedSize);

[yhirano, 2016/08/18 09:28:27]

Do these live/dead concepts make sense without isLocked()?

[hiroshige, 2016/08/19 05:37:43]

Nice catch.
In fact, there seems multiple definitions of "Live":
1. hasClientsOrObservers() is true (Most of Resource, e.g. callsites of
MemoryCache::makeDead() and makeLive(), allClientsAndObserversRemoved())
2. the Resource is on MemoryCache's live list (i.e. hasClientsOrObservers() is
true and decodedSize() > 0 and some other conditions...)
3. isLocked() (here; 1. is a subset of 3.)

I replaced this here with hasClientsOrObservers() to unify 1. and 3.
(and let unifying 1. and 2. a separate issue)

     else
         dump->addScalar("dead_size", "bytes", m_encodedSize);
 
     if (m_data) {
-        dump->addScalar("purgeable_size", "bytes", isPurgeable() ? encodedSize() + overheadSize() : 0);
         m_data->onMemoryDump(dumpName, memoryDump);
     }
 
     if (levelOfDetail == WebMemoryDumpLevelOfDetail::Detailed) {
         String urlToReport = url().getString();
         if (urlToReport.length() > kMaxURLReportLength) {
             urlToReport.truncate(kMaxURLReportLength);
             urlToReport = urlToReport + "...";
         }
         dump->addString("url", "", urlToReport);
@@ skipping 113 matching lines @@
     if (hasCacheControlNoStoreHeader())
         return false;
 
     // Do not revalidate Resource with redirects. https://crbug.com/613971
     if (!redirectChain().isEmpty())
         return false;
 
     return m_response.hasCacheValidatorFields() || m_resourceRequest.hasCacheValidatorFields();
 }
 
-bool Resource::isPurgeable() const
-{
-    return m_data && !m_data->isLocked();
-}
-
-bool Resource::lock()
-{
-    if (!m_data)
-        return true;
-    if (m_data->isLocked())
-        return true;
-
-    ASSERT(!hasClientsOrObservers());
-
-    // If locking fails, our buffer has been purged. There's no point
-    // in leaving a purged resource in MemoryCache.
-    if (!m_data->lock()) {
-        DEFINE_THREAD_SAFE_STATIC_LOCAL(EnumerationHistogram, failedLockHistogram, new EnumerationHistogram("Blink.SharedBuffer.FailedLock", kLastResourceType));
-        failedLockHistogram.count(getType());
-
-        m_data.clear();
-        setEncodedSize(0);
-        memoryCache()->remove(this);
-        return false;
-    }
-
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(EnumerationHistogram, successfulLockHistogram, new EnumerationHistogram("Blink.SharedBuffer.SuccessfulLock", kLastResourceType));
-    successfulLockHistogram.count(getType());
-
-    return true;
-}
-
 size_t Resource::calculateOverheadSize() const
 {
     static const int kAverageClientsHashMapSize = 384;
     return sizeof(Resource) + m_response.memoryUsage() + kAverageClientsHashMapSize + m_resourceRequest.url().getString().length() * 2;
 }
 
 void Resource::didChangePriority(ResourceLoadPriority loadPriority, int intraPriorityValue)
 {
     m_resourceRequest.setPriority(loadPriority, intraPriorityValue);
     if (m_loader)
@@ skipping 83 matching lines @@
     case Resource::TextTrack:
     case Resource::Media:
     case Resource::Manifest:
         return false;
     }
     ASSERT_NOT_REACHED();
     return false;
 }
 
 } // namespace blink