Replace ASSERT*() with DCHECK*() in core/fetch/ and core/loader/

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 119 matching lines @@
 {
     visitor->trace(m_resource);
     CachedMetadataHandler::trace(visitor);
 }
 
 void Resource::CachedMetadataHandlerImpl::setCachedMetadata(uint32_t dataTypeID, const char* data, size_t size, CachedMetadataHandler::CacheType cacheType)
 {
     // Currently, only one type of cached metadata per resource is supported.
     // If the need arises for multiple types of metadata per resource this could
     // be enhanced to store types of metadata in a map.
-    ASSERT(!m_cachedMetadata);
+    DCHECK(!m_cachedMetadata);
     m_cachedMetadata = CachedMetadata::create(dataTypeID, data, size);
     if (cacheType == CachedMetadataHandler::SendToPlatform)
         sendToPlatform();
 }
 
 void Resource::CachedMetadataHandlerImpl::clearCachedMetadata(CachedMetadataHandler::CacheType cacheType)
 {
     m_cachedMetadata.clear();
     if (cacheType == CachedMetadataHandler::SendToPlatform)
         sendToPlatform();
 }
 
 PassRefPtr<CachedMetadata> Resource::CachedMetadataHandlerImpl::cachedMetadata(uint32_t dataTypeID) const
 {
     if (!m_cachedMetadata || m_cachedMetadata->dataTypeID() != dataTypeID)
         return nullptr;
     return m_cachedMetadata.get();
 }
 
 String Resource::CachedMetadataHandlerImpl::encoding() const
 {
     return m_resource->encoding();
 }
 
 void Resource::CachedMetadataHandlerImpl::setSerializedCachedMetadata(const char* data, size_t size)
 {
     // We only expect to receive cached metadata from the platform once.
     // If this triggers, it indicates an efficiency problem which is most
     // likely unexpected in code designed to improve performance.
-    ASSERT(!m_cachedMetadata);
+    DCHECK(!m_cachedMetadata);
     m_cachedMetadata = CachedMetadata::createFromSerializedData(data, size);
 }
 
 void Resource::CachedMetadataHandlerImpl::sendToPlatform()
 {
     if (m_cachedMetadata) {
         const Vector<char>& serializedData = m_cachedMetadata->serializedData();
         Platform::current()->cacheMetadata(response().url(), response().responseTime(), serializedData.data(), serializedData.size());
     } else {
         Platform::current()->cacheMetadata(response().url(), response().responseTime(), nullptr, 0);
@@ skipping 118 matching lines @@
     , m_status(NotStarted)
     , m_needsSynchronousCacheHit(false)
     , m_linkPreload(false)
     , m_isRevalidating(false)
     , m_isAlive(false)
     , m_options(options)
     , m_responseTimestamp(currentTime())
     , m_cancelTimer(this, &Resource::cancelTimerFired)
     , m_resourceRequest(request)
 {
-    ASSERT(m_type == unsigned(type)); // m_type is a bitfield, so this tests careless updates of the enum.
+    DCHECK(m_type == unsigned(type)); // m_type is a bitfield, so this tests careless updates of the enum.

[yhirano, 2016/09/12 04:33:12]

DCHECK_EQ

[hiroshige, 2016/09/13 08:43:19]

Done.

     InstanceCounters::incrementCounter(InstanceCounters::ResourceCounter);
 
     // Currently we support the metadata caching only for HTTP family.
     if (m_resourceRequest.url().protocolIsInHTTPFamily())
         m_cacheHandler = CachedMetadataHandlerImpl::create(this);
     MemoryCoordinator::instance().registerClient(this);
 }
 
 Resource::~Resource()
 {
     InstanceCounters::decrementCounter(InstanceCounters::ResourceCounter);
 }
 
 DEFINE_TRACE(Resource)
 {
     visitor->trace(m_loader);
     visitor->trace(m_cacheHandler);
     visitor->trace(m_clients);
     visitor->trace(m_clientsAwaitingCallback);
     visitor->trace(m_finishedClients);
     MemoryCoordinatorClient::trace(visitor);
 }
 
 void Resource::setLoader(ResourceLoader* loader)
 {
-    RELEASE_ASSERT(!m_loader);
-    ASSERT(stillNeedsLoad());
+    CHECK(!m_loader);
+    DCHECK(stillNeedsLoad());
     m_loader = loader;
     m_status = Pending;
 }
 
 void Resource::checkNotify()
 {
     notifyClientsInternal(MarkFinishedOption::ShouldMarkFinished);
 }
 
 void Resource::notifyClientsInternal(MarkFinishedOption markFinishedOption)
@@ skipping 14 matching lines @@
     if (m_clients.contains(client)) {
         m_finishedClients.add(client);
         m_clients.remove(client);
     }
 }
 
 void Resource::appendData(const char* data, size_t length)
 {
     TRACE_EVENT0("blink", "Resource::appendData");
     DCHECK(!m_isRevalidating);
-    ASSERT(!errorOccurred());
+    DCHECK(!errorOccurred());
     if (m_options.dataBufferingPolicy == DoNotBufferData)
         return;
     if (m_data)
         m_data->append(data, length);
     else
         m_data = SharedBuffer::create(data, length);
     setEncodedSize(m_data->size());
 }
 
 void Resource::setResourceBuffer(PassRefPtr<SharedBuffer> resourceBuffer)
 {
     DCHECK(!m_isRevalidating);
-    ASSERT(!errorOccurred());
-    ASSERT(m_options.dataBufferingPolicy == BufferData);
+    DCHECK(!errorOccurred());
+    DCHECK(m_options.dataBufferingPolicy == BufferData);

[yhirano, 2016/09/12 04:33:11]

DCHECK_EQ

[hiroshige, 2016/09/13 08:43:19]

Done.

     m_data = resourceBuffer;
     setEncodedSize(m_data->size());
 }
 
 void Resource::setDataBufferingPolicy(DataBufferingPolicy dataBufferingPolicy)
 {
     m_options.dataBufferingPolicy = dataBufferingPolicy;
     m_data.clear();
     setEncodedSize(0);
 }
 
 void Resource::error(const ResourceError& error)
 {
-    ASSERT(!error.isNull());
+    DCHECK(!error.isNull());
     m_error = error;
     m_isRevalidating = false;
 
     if (m_error.isCancellation() || !isPreloaded())
         memoryCache()->remove(this);
 
     if (!errorOccurred())
         setStatus(LoadError);
-    ASSERT(errorOccurred());
+    DCHECK(errorOccurred());
     m_data.clear();
     m_loader = nullptr;
     checkNotify();
 }
 
 void Resource::finish(double loadFinishTime)
 {
     DCHECK(!m_isRevalidating);
     m_loadFinishTime = loadFinishTime;
     if (!errorOccurred())
@@ skipping 154 matching lines @@
     }
     setResponse(response);
     String encoding = response.textEncodingName();
     if (!encoding.isNull())
         setEncoding(encoding);
 }
 
 void Resource::setSerializedCachedMetadata(const char* data, size_t size)
 {
     DCHECK(!m_isRevalidating);
-    ASSERT(!m_response.isNull());
+    DCHECK(!m_response.isNull());
     if (m_cacheHandler)
         m_cacheHandler->setSerializedCachedMetadata(data, size);
 }
 
 CachedMetadataHandler* Resource::cacheHandler()
 {
     return m_cacheHandler.get();
 }
 
 
@@ skipping 150 matching lines @@
     if (!m_loader)
         return;
     if (m_type == Raw)
         cancelTimerFired(&m_cancelTimer);
     else if (!m_cancelTimer.isActive())
         m_cancelTimer.startOneShot(0, BLINK_FROM_HERE);
 }
 
 void Resource::cancelTimerFired(TimerBase* timer)
 {
-    ASSERT_UNUSED(timer, timer == &m_cancelTimer);
+    ALLOW_UNUSED_LOCAL(timer);

[yhirano, 2016/09/12 04:33:12]

Is this needed? DCHECK requires its expectation as well-formed even if
DCHECK_IS_ON is false, and that means the expression is evaluated by the
compiler, I guess?

[hiroshige, 2016/09/13 08:43:19]

I put ALLOW_UNUSED_LOCAL() based on a comment in Assertions.h:
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/wtf/As...

I removed ALLOW_UNUSED_LOCAL()s in Patch Set 5 and 6, and they passed on bots.

tkent@, how do you think about yhirano@'s comment?
Do we have specific bots/builders where compilation fails without
ALLOW_UNUSED_LOCAL()?

[tkent, 2016/09/13 08:50:52]

AFAIK we don't need to add ALLOW_UNUSED_LOCAL() on replacing ASSERT() with
DCHECK*(). The comment in Assertions.h is wrong.

[hiroshige, 2016/09/13 09:30:51]

Thanks! I'll create a separate CL to fix Assertions.h.

+    DCHECK(timer == &m_cancelTimer);

[yhirano, 2016/09/12 04:33:12]

DCHECK_EQ

[hiroshige, 2016/09/13 08:43:19]

Done.

     if (!hasClientsOrObservers() && m_loader)
         m_loader->cancel();
 }
 
 void Resource::setDecodedSize(size_t decodedSize)
 {
     if (decodedSize == m_decodedSize)
         return;
     size_t oldSize = size();
     m_decodedSize = decodedSize;
@@ skipping 38 matching lines @@
         didAddClient(client);
     }
 
     // It is still possible for the above loop to finish a new client synchronously.
     // If there's no client waiting we should deschedule.
     bool scheduled = ResourceCallback::callbackHandler().isScheduled(this);
     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);
+    DCHECK(m_clientsAwaitingCallback.isEmpty() || scheduled);
 }
 
 void Resource::prune()
 {
     destroyDecodedDataIfPossible();
 }
 
 void Resource::prepareToSuspend()
 {
     prune();
@@ skipping 208 matching lines @@
         return "Link prefetch resource";
     case Resource::TextTrack:
         return "Text track";
     case Resource::ImportResource:
         return "Imported resource";
     case Resource::Media:
         return "Media";
     case Resource::Manifest:
         return "Manifest";
     }
-    ASSERT_NOT_REACHED();
+    NOTREACHED();
     return initatorTypeNameToString(initiatorInfo.name);
 }
 
 bool Resource::shouldBlockLoadEvent() const
 {
     return !m_linkPreload && isLoadEventBlockingResourceType();
 }
 
 bool Resource::isLoadEventBlockingResourceType() const
 {
     switch (m_type) {
     case Resource::MainResource:
     case Resource::Image:
     case Resource::CSSStyleSheet:
     case Resource::Script:
     case Resource::Font:
     case Resource::SVGDocument:
     case Resource::XSLStyleSheet:
     case Resource::ImportResource:
         return true;
     case Resource::Raw:
     case Resource::LinkPrefetch:
     case Resource::TextTrack:
     case Resource::Media:
     case Resource::Manifest:
         return false;
     }
-    ASSERT_NOT_REACHED();
+    NOTREACHED();
     return false;
 }
 
 } // namespace blink