Fix aggressive RELEASE_ASSERTs in Resource (issue 243533002)

[hc...@chromium.org]

Reviewers: Nate Chapin,

Description:
Fix aggressive RELEASE_ASSERTs in Resource

I added RELEASE_ASSERTs in my last patch what are found to be too
aggressive. Change them back to ASSERTs and add preventive measures.

BUG=352043

Please review this at https://codereview.chromium.org/243533002/

SVN Base: svn://svn.chromium.org/blink/trunk

Affected files (+9, -7 lines):
M Source/core/fetch/Resource.cpp


Index: Source/core/fetch/Resource.cpp
diff --git a/Source/core/fetch/Resource.cpp b/Source/core/fetch/Resource.cpp
index
e4096b63282f3c1579f9ff0e637c25deb02f6444..953d70246a2a8b0665f4f75c34bdc0f07a9a83e0
100644
--- a/Source/core/fetch/Resource.cpp
+++ b/Source/core/fetch/Resource.cpp
@@ -478,14 +478,15 @@ 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);
@@ -588,10 +589,11 @@ void Resource::finishPendingClients()
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()) {
+ ResourceCallback::callbackHandler()->cancel(this);
+ }

// Prevent the case when there are clients waiting but no callback
scheduled.
ASSERT(m_clientsAwaitingCallback.isEmpty() || scheduled);

[jap...@chromium.org]

https://codereview.chromium.org/243533002/diff/1/Source/core/fetch/Resource.cpp
File Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/243533002/diff/1/Source/core/fetch/Resource.cpp#newcode594
Source/core/fetch/Resource.cpp:594: if

(ResourceCallback::callbackHandler()->isScheduled(this) &&
m_clientsAwaitingCallback.isEmpty()) {

Style nit: the curly braces are unnecessary.

https://codereview.chromium.org/243533002/

[jap...@chromium.org]

LGTM

https://codereview.chromium.org/243533002/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hc...@chromium.org/243533002/20001


https://codereview.chromium.org/243533002/

[commi...@chromium.org]

Try jobs failed on following builders:
tryserver.blink on linux_blink_dbg

https://codereview.chromium.org/243533002/

[commi...@chromium.org]

[hc...@chromium.org]

Committed patchset #2 manually as r171972 (presubmit successful).

https://codereview.chromium.org/243533002/