Fix aggressive RELEASE_ASSERTs in Resource

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 460 matching lines @@
 
     m_clients.add(client);
     return true;
 }
 
 void Resource::removeClient(ResourceClient* client)
 {
     if (m_clientsAwaitingCallback.contains(client)) {
         ASSERT(!m_clients.contains(client));
         m_clientsAwaitingCallback.remove(client);
-        if (m_clientsAwaitingCallback.isEmpty())
-            ResourceCallback::callbackHandler()->cancel(this);
     } else {
-        RELEASE_ASSERT(m_clients.contains(client));
+        ASSERT(m_clients.contains(client));
         m_clients.remove(client);
         didRemoveClient(client);
     }
 
+    if (m_clientsAwaitingCallback.isEmpty())
+        ResourceCallback::callbackHandler()->cancel(this);
+
     bool deleted = deleteIfPossible();
     if (!deleted && !hasClients()) {
         memoryCache()->makeDead(this);
         if (!m_switchingClientsToRevalidatedResource)
             allClientsRemoved();
 
         // RFC2616 14.9.2:
         // "no-store: ... MUST make a best-effort attempt to remove the information from volatile storage as promptly as possible"
         // "... History buffers MAY store such responses as part of their normal operation."
         // We allow non-secure content to be reused in history, but we do not allow secure content to be reused.
@@ skipping 82 matching lines @@
     for (size_t i = 0; i < clientsToNotify.size(); ++i) {
         ResourceClient* client = clientsToNotify[i];
 
         // Handle case (2) to skip removed clients.
         if (!m_clientsAwaitingCallback.remove(client))
             continue;
         m_clients.add(client);
         didAddClient(client);
     }
 
-    bool scheduled = ResourceCallback::callbackHandler()->isScheduled(this);
-    // It is a critical problem if a callback is scheduled but there is no client waiting for it.
-    // Such a callback cannot be cancelled. It is better to crash the renderer now.
-    RELEASE_ASSERT(!scheduled || !m_clientsAwaitingCallback.isEmpty());
+    // It is still possible for the above loop to finish a new client synchronously.
+    // If there's no client waiting we should deschedule.
+    if (ResourceCallback::callbackHandler()->isScheduled(this) && m_clientsAwaitingCallback.isEmpty()) {

[Nate Chapin, 2014/04/18 18:43:46]

Style nit: the curly braces are unnecessary.

+        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();
 }
@@ skipping 374 matching lines @@
         return "ImportResource";
     case Resource::Media:
         return "Media";
     }
     ASSERT_NOT_REACHED();
     return "Unknown";
 }
 #endif // !LOG_DISABLED
 
 }