Implementing timeout support for XHR

Source/core/platform/network/chromium/ResourceHandle.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
+ * Copyright (C) 2013 Intel Corporation. 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
  * copyright notice, this list of conditions and the following disclaimer
  * in the documentation and/or other materials provided with the
@@ skipping 38 matching lines @@
 
 namespace WebCore {
 
 // ResourceHandleInternal -----------------------------------------------------
 ResourceHandleInternal::ResourceHandleInternal(NetworkingContext* context, const ResourceRequest& request, ResourceHandleClient* client)
     : m_context(context)
     , m_request(request)
     , m_owner(0)
     , m_client(client)
     , m_state(ConnectionStateNew)
+    , m_timeoutTimer(this, &ResourceHandleInternal::timerFired)
 {
 }
 
 void ResourceHandleInternal::start()
 {
     if (m_state != ConnectionStateNew)
         CRASH();
     m_state = ConnectionStateStarted;
 
     m_loader = adoptPtr(Platform::current()->createURLLoader());
     ASSERT(m_loader);
 
     WrappedResourceRequest wrappedRequest(m_request);
     wrappedRequest.setAllowStoredCredentials(allowStoredCredentials());
     m_loader->loadAsynchronously(wrappedRequest, this);
+
+    if (m_request.timeoutInterval() > 0)

[darin (slow to review), 2013/04/22 17:46:39]

I'm also curious if we can remove ResourceRequest::timeoutInterval() if that
won't be implemented by the ResourceHandle implementation now.

+        m_timeoutTimer.startOneShot(m_request.timeoutInterval());
 }
 
-void ResourceHandleInternal::cancel()
+void ResourceHandleInternal::cancel(const ResourceError& error)
 {
     m_state = ConnectionStateCanceled;
     m_loader->cancel();
 
+    if (m_client && !error.isNull()) {
+        m_client->didFail(m_owner, error);

[Dominik Röttsches, 2013/04/22 15:47:45]

didFail() here triggers a WebCore::ResourceLoader::releaseResources() which in
turn deletes the ResourceHandle which this ResourceHandleInternal is owned by.
So, we cannot and don't need to set m_client to 0 after this.

+        return;
+    }
+
     // Do not make any further calls to the client.
     m_client = 0;
 }
 
 void ResourceHandleInternal::setDefersLoading(bool value)
 {
     m_loader->setDefersLoading(value);
 }
 
 void ResourceHandleInternal::didChangePriority(WebURLRequest::Priority newPriority)
@@ skipping 77 matching lines @@
     ASSERT(m_client);
     m_state = ConnectionStateFailed;
     m_client->didFail(m_owner, error);
 }
 
 ResourceHandleInternal* ResourceHandleInternal::FromResourceHandle(ResourceHandle* handle)
 {
     return handle->d.get();
 }
 
+void ResourceHandleInternal::timerFired(Timer<ResourceHandleInternal>* timer)
+{
+    ASSERT_UNUSED(timer, timer == &m_timeoutTimer);
+
+    // Using values from net/base/net_error_list.h ERR_TIMED_OUT.
+    static const int timeoutError = -7;
+    static const char* const errorDomain = "net";

[Dominik Röttsches, 2013/04/22 15:47:45]

Since the cancel() method now checks for isNull(), I need to use the non-trivial
constructor here, initializing it with some values.

Ideally, we should not copy but use the values from net/base/net_error_list.h -
however, the way to retrieve them here while adhering to proper layering would
mean adding a function for retrieving a WebURLError to the WebURLLoader
interface and synthesizing such an error on the Chromium side.

Since the actual values do not have any relevance in this case (only the
isTimeout() flag makes the difference for the didFail call) and a method for
this purpose does not 100% belong into the WebURLLoader class, I was not sure
whether that's worth the interface change. How do you see this?

+    ResourceError error(errorDomain, timeoutError, m_request.url().string(), String());
+    error.setIsTimeout(true);
+    cancel(error);
+ }
+
 // ResourceHandle -------------------------------------------------------------
 
 ResourceHandle::ResourceHandle(NetworkingContext* context, const ResourceRequest& request, ResourceHandleClient* client, bool defersLoading, bool shouldContentSniff)
     : d(adoptPtr(new ResourceHandleInternal(context, request, client)))
 {
     d->setOwner(this);
 
     // FIXME: Figure out what to do with the bool params.
 }
 
@@ skipping 85 matching lines @@
     d->didChangePriority(static_cast<WebURLRequest::Priority>(newPriority));
 }
 
 // static
 void ResourceHandle::cacheMetadata(const ResourceResponse& response, const Vector<char>& data)
 {
     WebKit::Platform::current()->cacheMetadata(response.url(), response.responseTime(), data.data(), data.size());
 }
 
 } // namespace WebCore