Prevent calling didReceiveData()/didFinishLoading() after didFailAccessControlCheck()

Source/core/loader/WorkerLoaderClientBridgeSyncHelper.cpp

 /*
  * Copyright (C) 2014 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 48 matching lines @@
 {
     // This must be called only after m_event is signalled.
     MutexLocker lock(m_lock);
     ASSERT(m_done);
     for (size_t i = 0; i < m_clientTasks.size(); ++i)
         (*m_clientTasks[i])();
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;

[kinuko, 2015/02/03 02:35:36]

nit: I am kind of supportive for this style, but chromium style usually doesn't
like this (assert then handle exceptions).

http://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-

[hiroshige, 2015/02/03 08:19:44]

Hmm.
Then, how about use RELEASE_ASSERT here and omit if statement?
(because just omitting if statement results in a thread-safe bug in Release
builds)

[kinuko, 2015/02/03 08:51:22]

That works for me, assuming that leaving a thread-safe bug could lead to a
security problem.

[hiroshige, 2015/02/03 09:21:14]

Done.

+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     m_clientTasks.append(bind(&ThreadableLoaderClient::didSendData, &m_client, bytesSent, totalBytesToBeSent));
 }
 
 static void didReceiveResponseAdapter(ThreadableLoaderClient* client, unsigned long identifier, PassOwnPtr<CrossThreadResourceResponseData> responseData, PassOwnPtr<WebDataConsumerHandle> handle)
 {
     OwnPtr<ResourceResponse> response(ResourceResponse::adopt(responseData));
     client->didReceiveResponse(identifier, *response, handle);
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didReceiveResponse(unsigned long identifier, const ResourceResponse& response, PassOwnPtr<WebDataConsumerHandle> handle)
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;
+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     m_clientTasks.append(bind(&didReceiveResponseAdapter, &m_client, identifier, response.copyData(), handle));
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didReceiveData(const char* data, unsigned dataLength)
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;
+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     Vector<char>* buffer = new Vector<char>(dataLength);
     memcpy(buffer->data(), data, dataLength);
     m_receivedData.append(buffer);
     m_clientTasks.append(bind(&ThreadableLoaderClient::didReceiveData, &m_client, static_cast<const char*>(buffer->data()), dataLength));
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didDownloadData(int dataLength)
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;
+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     m_clientTasks.append(bind(&ThreadableLoaderClient::didDownloadData, &m_client, dataLength));
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didReceiveCachedMetadata(const char* data, int dataLength)
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;
+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     Vector<char>* buffer = new Vector<char>(dataLength);
     memcpy(buffer->data(), data, dataLength);
     m_receivedData.append(buffer);
     m_clientTasks.append(bind(&ThreadableLoaderClient::didReceiveCachedMetadata, &m_client, static_cast<const char*>(buffer->data()), dataLength));
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didFinishLoading(unsigned long identifier, double finishTime)
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;
+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     m_clientTasks.append(bind(&ThreadableLoaderClient::didFinishLoading, &m_client, identifier, finishTime));
     m_done = true;
     m_event->signal();
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didFail(const ResourceError& error)
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;
+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     m_clientTasks.append(bind(&ThreadableLoaderClient::didFail, &m_client, error.copy()));
     m_done = true;
     m_event->signal();
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didFailAccessControlCheck(const ResourceError& error)
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;
+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     m_clientTasks.append(bind(&ThreadableLoaderClient::didFailAccessControlCheck, &m_client, error.copy()));
     m_done = true;
     m_event->signal();
 }
 
 void WorkerLoaderClientBridgeSyncHelper::didFailRedirectCheck()
 {
+    ASSERT(!m_done);
+    if (m_done)
+        return;
+
     MutexLocker lock(m_lock);
     ASSERT(isMainThread());
     m_clientTasks.append(bind(&ThreadableLoaderClient::didFailRedirectCheck, &m_client));
     m_done = true;
     m_event->signal();
 }
 
 WorkerLoaderClientBridgeSyncHelper::WorkerLoaderClientBridgeSyncHelper(ThreadableLoaderClient& client, PassOwnPtr<blink::WebWaitableEvent> event)
     : m_done(false)
     , m_client(client)
     , m_event(event)
 {
     ASSERT(m_event);
 }
 
 } // namespace blink