- Revision
- 110608
- Author
- [email protected]
- Date
- 2012-03-13 14:18:25 -0700 (Tue, 13 Mar 2012)
Log Message
Source/WebCore: Rework how a CachedRawResource decides if it can be reused
for a given ResourceRequest. Ensure method, body, cookie policy,
and all headers match.
http://bugs.webkit.org/show_bug.cgi?id=79325
Reviewed by Oliver Hunt.
Test: http/tests/cache/xhr-body.html
* loader/cache/CachedRawResource.cpp:
(WebCore::CachedRawResource::canReuse):
* loader/cache/CachedRawResource.h: Take a ResourceRequest in canReuse().
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::determineRevalidationPolicy): Remove Range header
check, since it is now redundant.
LayoutTests: Test for http://bugs.webkit.org/show_bug.cgi?id=79325.
Reviewed by Oliver Hunt.
* http/tests/cache/resources/body.php: Added.
* http/tests/cache/xhr-body-expected.txt: Added.
* http/tests/cache/xhr-body.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (110607 => 110608)
--- trunk/LayoutTests/ChangeLog 2012-03-13 21:15:00 UTC (rev 110607)
+++ trunk/LayoutTests/ChangeLog 2012-03-13 21:18:25 UTC (rev 110608)
@@ -1,3 +1,13 @@
+2012-03-13 Nate Chapin <[email protected]>
+
+ Test for http://bugs.webkit.org/show_bug.cgi?id=79325.
+
+ Reviewed by Oliver Hunt.
+
+ * http/tests/cache/resources/body.php: Added.
+ * http/tests/cache/xhr-body-expected.txt: Added.
+ * http/tests/cache/xhr-body.html: Added.
+
2012-03-13 Alexey Proskuryakov <[email protected]>
Updating some Mac results in media directory.
Added: trunk/LayoutTests/http/tests/cache/resources/body.php (0 => 110608)
--- trunk/LayoutTests/http/tests/cache/resources/body.php (rev 0)
+++ trunk/LayoutTests/http/tests/cache/resources/body.php 2012-03-13 21:18:25 UTC (rev 110608)
@@ -0,0 +1,4 @@
+<?php
+header('Etag: 123456789');
+echo file_get_contents('php://input');
+?>
Added: trunk/LayoutTests/http/tests/cache/xhr-body-expected.txt (0 => 110608)
--- trunk/LayoutTests/http/tests/cache/xhr-body-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/cache/xhr-body-expected.txt 2012-03-13 21:18:25 UTC (rev 110608)
@@ -0,0 +1,8 @@
+Test that we don't hang xhr loads from cache when the resposne body was empty.
+
+load
+progress
+load
+This http body should be visible
+DONE
+
Added: trunk/LayoutTests/http/tests/cache/xhr-body.html (0 => 110608)
--- trunk/LayoutTests/http/tests/cache/xhr-body.html (rev 0)
+++ trunk/LayoutTests/http/tests/cache/xhr-body.html 2012-03-13 21:18:25 UTC (rev 110608)
@@ -0,0 +1,49 @@
+<body _onload_="test();">
+<p>Test that we don't hang xhr loads from cache when the resposne body was empty.<p>
+<pre id=log></pre>
+<script type="text/_javascript_">
+
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+}
+
+function log(msg) {
+ document.getElementById("log").innerHTML += msg + "\n";
+}
+
+function test() {
+ var request = new XMLHttpRequest();
+ request.addEventListener("progress", function() { log("progress") }, false);
+ request.addEventListener("load", function() { log("load"); repeat(); }, false);
+ request.addEventListener("error", function() { log("error"); repeat(); }, false);
+ request.addEventListener("abort", function() { log("abort"); repeat(); }, false);
+ request.open("GET", "resources/body.php", true);
+ request.send();
+}
+
+var secondRequest;
+
+function secondRequestFinished() {
+ log("load");
+ log(secondRequest.response);
+ end();
+}
+
+function repeat() {
+ secondRequest = new XMLHttpRequest();
+ secondRequest.addEventListener("progress", function() { log("progress") }, false);
+ secondRequest.addEventListener("load", function() { secondRequestFinished(); }, false);
+ secondRequest.addEventListener("error", function() { log("error"); end(); }, false);
+ secondRequest.addEventListener("abort", function() { log("abort"); end(); }, false);
+ secondRequest.open("POST", "resources/body.php", true);
+ secondRequest.send("This http body should be visible");
+}
+
+function end() {
+ log("DONE");
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+}
+</script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (110607 => 110608)
--- trunk/Source/WebCore/ChangeLog 2012-03-13 21:15:00 UTC (rev 110607)
+++ trunk/Source/WebCore/ChangeLog 2012-03-13 21:18:25 UTC (rev 110608)
@@ -1,3 +1,21 @@
+2012-03-13 Nate Chapin <[email protected]>
+
+ Rework how a CachedRawResource decides if it can be reused
+ for a given ResourceRequest. Ensure method, body, cookie policy,
+ and all headers match.
+ http://bugs.webkit.org/show_bug.cgi?id=79325
+
+ Reviewed by Oliver Hunt.
+
+ Test: http/tests/cache/xhr-body.html
+
+ * loader/cache/CachedRawResource.cpp:
+ (WebCore::CachedRawResource::canReuse):
+ * loader/cache/CachedRawResource.h: Take a ResourceRequest in canReuse().
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::determineRevalidationPolicy): Remove Range header
+ check, since it is now redundant.
+
2012-03-13 Rob Buis <[email protected]>
Fix cast-align GCC warning
Modified: trunk/Source/WebCore/loader/cache/CachedRawResource.cpp (110607 => 110608)
--- trunk/Source/WebCore/loader/cache/CachedRawResource.cpp 2012-03-13 21:15:00 UTC (rev 110607)
+++ trunk/Source/WebCore/loader/cache/CachedRawResource.cpp 2012-03-13 21:18:25 UTC (rev 110608)
@@ -122,14 +122,36 @@
m_loader->setDefersLoading(defers);
}
-bool CachedRawResource::canReuse() const
+bool CachedRawResource::canReuse(const ResourceRequest& newRequest) const
{
if (m_options.shouldBufferData == DoNotBufferData)
return false;
- if (m_resourceRequest.httpMethod() != "GET")
+ if (m_resourceRequest.httpMethod() != newRequest.httpMethod())
return false;
+ if (m_resourceRequest.httpBody() != newRequest.httpBody())
+ return false;
+
+ if (m_resourceRequest.allowCookies() != newRequest.allowCookies())
+ return false;
+
+ // Ensure all headers match the existing headers before continuing.
+ // Note that only headers set by our client will be present in either
+ // ResourceRequest, since SubresourceLoader creates a separate copy
+ // for its purposes.
+ // FIXME: There might be some headers that shouldn't block reuse.
+ const HTTPHeaderMap& newHeaders = newRequest.httpHeaderFields();
+ const HTTPHeaderMap& oldHeaders = m_resourceRequest.httpHeaderFields();
+ if (newHeaders.size() != oldHeaders.size())
+ return false;
+
+ HTTPHeaderMap::const_iterator end = newHeaders.end();
+ for (HTTPHeaderMap::const_iterator i = newHeaders.begin(); i != end; ++i) {
+ AtomicString headerName = i->first;
+ if (i->second != oldHeaders.get(headerName))
+ return false;
+ }
return true;
}
Modified: trunk/Source/WebCore/loader/cache/CachedRawResource.h (110607 => 110608)
--- trunk/Source/WebCore/loader/cache/CachedRawResource.h 2012-03-13 21:15:00 UTC (rev 110607)
+++ trunk/Source/WebCore/loader/cache/CachedRawResource.h 2012-03-13 21:18:25 UTC (rev 110608)
@@ -42,7 +42,7 @@
// FIXME: This is exposed for the InpsectorInstrumentation for preflights in DocumentThreadableLoader. It's also really lame.
unsigned long identifier() const { return m_identifier; }
- bool canReuse() const;
+ bool canReuse(const ResourceRequest&) const;
private:
virtual void didAddClient(CachedResourceClient*);
Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (110607 => 110608)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2012-03-13 21:15:00 UTC (rev 110607)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2012-03-13 21:18:25 UTC (rev 110608)
@@ -537,7 +537,7 @@
return Reload;
}
- if (existingResource->type() == CachedResource::RawResource && !static_cast<CachedRawResource*>(existingResource)->canReuse())
+ if (existingResource->type() == CachedResource::RawResource && !static_cast<CachedRawResource*>(existingResource)->canReuse(request))
return Reload;
// Certain requests (e.g., XHRs) might have manually set headers that require revalidation.
@@ -545,11 +545,6 @@
// of things about how revalidation works that manual headers violate, so punt to Reload instead.
if (request.isConditional())
return Reload;
-
- // Re-using resources in the case of a Range header is very simple if the headers are identical and
- // much tougher if they aren't.
- if (existingResource->resourceRequest().httpHeaderField("Range") != request.httpHeaderField("Range"))
- return Reload;
// Don't reload resources while pasting.
if (m_allowStaleResources)
