Implementing timeout support for XHR

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

Index: Source/core/platform/network/chromium/ResourceHandle.cpp
diff --git a/Source/core/platform/network/chromium/ResourceHandle.cpp b/Source/core/platform/network/chromium/ResourceHandle.cpp
index 54175ef216086b78905b6a56f4874d017cb3f4f1..d5d5420990d9f4a9105c1f2a5e8c640d6f182cf1 100644
--- a/Source/core/platform/network/chromium/ResourceHandle.cpp
+++ b/Source/core/platform/network/chromium/ResourceHandle.cpp
@@ -1,5 +1,6 @@
 /*
  * 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
@@ -58,6 +59,7 @@ ResourceHandleInternal::ResourceHandleInternal(NetworkingContext* context, const
     , m_owner(0)
     , m_client(client)
     , m_state(ConnectionStateNew)
+    , m_timeoutTimer(this, &ResourceHandleInternal::timerFired)
 {
 }
 
@@ -73,13 +75,21 @@ void ResourceHandleInternal::start()
     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;
 }
@@ -177,6 +187,18 @@ ResourceHandleInternal* ResourceHandleInternal::FromResourceHandle(ResourceHandl
     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)