Allow XHR timeout attribute to be overridden after send(), per spec

Source/core/loader/DocumentThreadableLoader.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2013, Intel Corporation
  *
  * 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.
@@ skipping 61 matching lines @@
     : m_client(client)
     , m_document(document)
     , m_options(options)
     , m_resourceLoaderOptions(resourceLoaderOptions)
     , m_forceDoNotAllowStoredCredentials(false)
     , m_securityOrigin(m_resourceLoaderOptions.securityOrigin)
     , m_sameOriginRequest(securityOrigin()->canRequest(request.url()))
     , m_simpleRequest(true)
     , m_async(blockingBehavior == LoadAsynchronously)
     , m_timeoutTimer(this, &DocumentThreadableLoader::didTimeout)
+    , m_requestStartedSeconds(0.0)
 {
     ASSERT(client);
     // Setting an outgoing referer is only supported in the async code path.
     ASSERT(m_async || request.httpReferrer().isEmpty());
 
+    m_requestStartedSeconds = monotonicallyIncreasingTime();
+
     // Save any CORS simple headers on the request here. If this request redirects cross-origin, we cancel the old request
     // create a new one, and copy these headers.
     const HTTPHeaderMap& headerMap = request.httpHeaderFields();
     HTTPHeaderMap::const_iterator end = headerMap.end();
     for (HTTPHeaderMap::const_iterator it = headerMap.begin(); it != end; ++it) {
         if (isOnAccessControlSimpleRequestHeaderWhitelist(it->key, it->value))
             m_simpleRequestHeaders.add(it->key, it->value);
     }
 
     if (m_sameOriginRequest || m_options.crossOriginRequestPolicy == AllowCrossOriginRequests) {
@@ skipping 43 matching lines @@
             preflightOptions.allowCredentials = DoNotAllowStoredCredentials;
             loadRequest(preflightRequest, preflightOptions);
         }
     }
 }
 
 DocumentThreadableLoader::~DocumentThreadableLoader()
 {
 }
 
+void DocumentThreadableLoader::overrideTimeout(unsigned long timeoutMilliseconds)
+{
+    if (!m_async)
+        return;
+    m_timeoutTimer.stop();
+    // At the time of this method's implementation, it is only ever called by
+    // XMLHttpRequest, when the timeout attribute is set after sending the
+    // request.
+    //
+    // The XHR request says to resolve the time relative to when the request
+    // was initially sent, however other uses of this method may need to
+    // behave differently, in which case this should be re-arranged somehow.
+    if (timeoutMilliseconds && m_requestStartedSeconds > 0.0) {
+        double elapsedTime = monotonicallyIncreasingTime() - m_requestStartedSeconds;
+        double nextFire = timeoutMilliseconds / 1000.0;
+        double resolvedTime = std::max(nextFire - elapsedTime, 0.0);
+        m_timeoutTimer.startOneShot(resolvedTime, FROM_HERE);
+    }
+}
+
 void DocumentThreadableLoader::cancel()
 {
     cancelWithError(ResourceError());
 }
 
 void DocumentThreadableLoader::cancelWithError(const ResourceError& error)
 {
     RefPtr<DocumentThreadableLoader> protect(this);
 
     // Cancel can re-enter and m_resource might be null here as a result.
     if (m_client && resource()) {
         ResourceError errorForCallback = error;
         if (errorForCallback.isNull()) {
             // FIXME: This error is sent to the client in didFail(), so it should not be an internal one. Use FrameLoaderClient::cancelledError() instead.
             errorForCallback = ResourceError(errorDomainBlinkInternal, 0, resource()->url().string(), "Load cancelled");
             errorForCallback.setIsCancellation(true);
         }
         m_client->didFail(errorForCallback);
     }
     clearResource();
     m_client = 0;
+    m_requestStartedSeconds = 0.0;
 }
 
 void DocumentThreadableLoader::setDefersLoading(bool value)
 {
     if (resource())
         resource()->setDefersLoading(value);
 }
 
 void DocumentThreadableLoader::redirectReceived(Resource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse)
 {
     ASSERT(m_client);
     ASSERT_UNUSED(resource, resource == this->resource());
 
     RefPtr<DocumentThreadableLoader> protect(this);
     if (!isAllowedByPolicy(request.url())) {
         m_client->didFailRedirectCheck();
         request = ResourceRequest();
+        m_requestStartedSeconds = 0.0;
         return;
     }
 
     // Allow same origin requests to continue after allowing clients to audit the redirect.
     if (isAllowedRedirect(request.url())) {
         if (m_client->isDocumentThreadableLoaderClient())
             static_cast<DocumentThreadableLoaderClient*>(m_client)->willSendRequest(request, redirectResponse);
         return;
     }
 
@@ skipping 46 matching lines @@
             makeCrossOriginAccessRequest(request);
             return;
         }
 
         ResourceError error(errorDomainBlinkInternal, 0, redirectResponse.url().string(), accessControlErrorDescription);
         m_client->didFailAccessControlCheck(error);
     } else {
         m_client->didFailRedirectCheck();
     }
     request = ResourceRequest();
+    m_requestStartedSeconds = 0.0;
 }
 
 void DocumentThreadableLoader::dataSent(Resource* resource, unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
 {
     ASSERT(m_client);
     ASSERT_UNUSED(resource, resource == this->resource());
     m_client->didSendData(bytesSent, totalBytesToBeSent);
 }
 
 void DocumentThreadableLoader::dataDownloaded(Resource* resource, int dataLength)
@@ skipping 91 matching lines @@
     else
         handleSuccessfulFinish(resource->identifier(), resource->loadFinishTime());
 }
 
 void DocumentThreadableLoader::handleSuccessfulFinish(unsigned long identifier, double finishTime)
 {
     if (m_actualRequest) {
         ASSERT(!m_sameOriginRequest);
         ASSERT(m_options.crossOriginRequestPolicy == UseAccessControl);
         loadActualRequest();
-    } else
+    } else {
         m_client->didFinishLoading(identifier, finishTime);
+        m_requestStartedSeconds = 0.0;

[abarth-chromium, 2014/06/22 00:22:48]

There's no reason to believe |this| is still alive at this point.

Reading the review thread, folks asked for me to take a look at this CL before
it landed, which didn't happen.

[caitp (gmail), 2014/06/22 00:56:14]

Given that this is a non-static method, there probably is a good reason to
assume |this| is still alive. I see that it's not, but it's not clear that
expecting 'this' to exist in an instance method is the root cause. Especially
since we're referencing m_client.

[caitp (gmail), 2014/06/22 00:58:19]

Unless you mean that `didFinishLoading()` might kill 'this', okay.

[tyoshino (SeeGerritForStatus), 2014/06/23 08:00:02]

Sorry that I didn't catch this. Right, didFinishLoading() may kill this. You can
just put m_requestStartedSeconds = 0.0 before calling didFinishLoading().

[caitp (gmail), 2014/06/23 15:55:44]

It would have been hard to notice =)

I'm not sure there's a good reason to reset m_requestStarted if there's a
guarantee the loader will die (I guess it could be done in the destructor to
handle all of these cases?) --- but it may be useful for `didFinishLoading()`,
etc, to know when the entire request was started. Or it might not, I'm not sure.
I could see that potentially being useful later on, though.

[tyoshino (SeeGerritForStatus), 2014/06/24 05:55:34]

Feel free to postpone this and leave FIXME explaining discussion we had. I.e.
I'm fine with not having this line here and L431 for now.

It's not guaranteed that the loader dies on didFinishLoading, but it's not the
right way to call overrideTimeout on the ThreadableLoader after receiving
didFinishLoading callback and I guess no one does it.

+    }
 }
 
 void DocumentThreadableLoader::didTimeout(Timer<DocumentThreadableLoader>* timer)
 {
     ASSERT_UNUSED(timer, timer == &m_timeoutTimer);
 
     // Using values from net/base/net_error_list.h ERR_TIMED_OUT,
     // Same as existing FIXME above - this error should be coming from FrameLoaderClient to be identifiable.
     static const int timeoutError = -7;
     ResourceError error("net", timeoutError, resource()->url(), String());
@@ skipping 16 matching lines @@
 }
 
 void DocumentThreadableLoader::handlePreflightFailure(const String& url, const String& errorDescription)
 {
     ResourceError error(errorDomainBlinkInternal, 0, url, errorDescription);
 
     // Prevent handleSuccessfulFinish() from bypassing access check.
     m_actualRequest = nullptr;
 
     m_client->didFailAccessControlCheck(error);
+    m_requestStartedSeconds = 0.0;

[tyoshino (SeeGerritForStatus), 2014/06/23 08:00:02]

ditto

 }
 
 void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, ResourceLoaderOptions resourceLoaderOptions)
 {
     // Any credential should have been removed from the cross-site requests.
     const KURL& requestURL = request.url();
     ASSERT(m_sameOriginRequest || requestURL.user().isEmpty());
     ASSERT(m_sameOriginRequest || requestURL.pass().isEmpty());
 
     // Update resourceLoaderOptions with enforced values.
@@ skipping 25 matching lines @@
     FetchRequest fetchRequest(request, m_options.initiator, resourceLoaderOptions);
     ResourcePtr<Resource> resource = m_document.fetcher()->fetchSynchronously(fetchRequest);
     ResourceResponse response = resource ? resource->response() : ResourceResponse();
     unsigned long identifier = resource ? resource->identifier() : std::numeric_limits<unsigned long>::max();
     ResourceError error = resource ? resource->resourceError() : ResourceError();
 
     InspectorInstrumentation::documentThreadableLoaderStartedLoadingForClient(&m_document, identifier, m_client);
 
     if (!resource) {
         m_client->didFail(error);
+        m_requestStartedSeconds = 0.0; // Unnecessary for synchronous request, but for API consistency.
         return;
     }
 
     // No exception for file:/// resources, see <rdar://problem/4962298>.
     // Also, if we have an HTTP response, then it wasn't a network error in fact.
     if (!error.isNull() && !requestURL.isLocalFile() && response.httpStatusCode() <= 0) {
         m_client->didFail(error);
         return;
     }
 
     // FIXME: A synchronous request does not tell us whether a redirect happened or not, so we guess by comparing the
     // request and response URLs. This isn't a perfect test though, since a server can serve a redirect to the same URL that was
     // requested. Also comparing the request and response URLs as strings will fail if the requestURL still has its credentials.
     if (requestURL != response.url() && (!isAllowedByPolicy(response.url()) || !isAllowedRedirect(response.url()))) {
         m_client->didFailRedirectCheck();
+        m_requestStartedSeconds = 0.0; // Unnecessary for synchronous request, but for API consistency.
         return;
     }
 
     handleResponse(identifier, response);
 
     SharedBuffer* data = resource->resourceBuffer();
     if (data)
         handleReceivedData(data->data(), data->size());
 
     handleSuccessfulFinish(identifier, 0.0);
@@ skipping 20 matching lines @@
         return DoNotAllowStoredCredentials;
     return m_resourceLoaderOptions.allowCredentials;
 }
 
 SecurityOrigin* DocumentThreadableLoader::securityOrigin() const
 {
     return m_securityOrigin ? m_securityOrigin.get() : m_document.securityOrigin();
 }
 
 } // namespace WebCore