REGRESSION(r158333): http/tests/xmlhttprequest/response-encoding.html and xmlhttprequest-overridemimetype-content-type-header.html are failing
https://bugs.webkit.org/show_bug.cgi?id=123548
Reviewed by Brady Eidson.
Source/WebCore:
We had code that made sure that cached 200 responses weren't used for conditional
requests. But it didn't work the other way - cached 304 responses got reused for
subsequent unconditional requests!
Adding the test uncovered this bug.
* loader/cache/CachedRawResource.cpp: (WebCore::shouldIgnoreHeaderForCacheReuse):
Should never ignore conditional headers. Code in determineRevalidationPolicy
was already undoing this for conditional requests, but we also shouldn't use
WebCore cache if it holds a 304 response to conditional request.
* loader/cache/CachedResourceLoader.cpp: (WebCore::CachedResourceLoader::determineRevalidationPolicy):
Even though the changed code is only for raw resources, I think that we can never
get a conditional request here any more.
LayoutTests:
* TestExpectations: Unskip tests that used to be affected by response-empty-arraybuffer.html
* http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt:
* http/tests/xmlhttprequest/response-empty-arraybuffer.html:
Fix a stupid typo. This test actually fully passes.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@158362 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index a465614..36de5f6 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2013-10-31 Alexey Proskuryakov <ap@apple.com>
+
+ REGRESSION(r158333): http/tests/xmlhttprequest/response-encoding.html and xmlhttprequest-overridemimetype-content-type-header.html are failing
+ https://bugs.webkit.org/show_bug.cgi?id=123548
+
+ Reviewed by Brady Eidson.
+
+ * TestExpectations: Unskip tests that used to be affected by response-empty-arraybuffer.html
+
+ * http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt:
+ * http/tests/xmlhttprequest/response-empty-arraybuffer.html:
+ Fix a stupid typo. This test actually fully passes.
+
2013-10-31 Krzysztof Wolanski <k.wolanski@samsung.com>
[EFL] Rebaselining after r158186
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index e72828b..193f036 100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -76,8 +76,5 @@
webkit.org/b/122679 security/crypto-subtle-gc-2.html [ Skip ]
webkit.org/b/122679 security/crypto-subtle-gc-3.html [ Skip ]
-webkit.org/b/123548 http/tests/xmlhttprequest/response-encoding.html [ Failure ]
-webkit.org/b/123548 http/tests/xmlhttprequest/xmlhttprequest-overridemimetype-content-type-header.html [ Failure ]
-
webkit.org/b/123555 [ Debug ] media/media-fragments/TC0054.html [ Crash ]
webkit.org/b/123555 [ Debug ] media/media-fragments/TC0061.html [ Crash ]
diff --git a/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt b/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt
index 41e786e..8e49dbe 100644
--- a/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt
+++ b/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt
@@ -11,9 +11,9 @@
PASS request.status is 200
PASS Object.prototype.toString.call(request.response) is '[object ArrayBuffer]'
PASS request.response.byteLength is 68
-FAIL request2.status should be 304. Was 200.
+PASS request2.status is 304
PASS Object.prototype.toString.call(request2.response) is '[object ArrayBuffer]'
-FAIL request2.response.byteLength should be 0. Was 68.
+PASS request2.response.byteLength is 0
PASS successfullyParsed is true
TEST COMPLETE
diff --git a/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer.html b/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer.html
index 04a7b37..cd1bfc3 100644
--- a/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer.html
+++ b/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer.html
@@ -58,7 +58,7 @@
if (req2.readyState != 4)
return;
- request2 = req;
+ request2 = req2;
shouldBe("request2.status", "304");
shouldBe("Object.prototype.toString.call(request2.response)", "'[object ArrayBuffer]'");
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index ab7e052..86e801c 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2013-10-31 Alexey Proskuryakov <ap@apple.com>
+
+ REGRESSION(r158333): http/tests/xmlhttprequest/response-encoding.html and xmlhttprequest-overridemimetype-content-type-header.html are failing
+ https://bugs.webkit.org/show_bug.cgi?id=123548
+
+ Reviewed by Brady Eidson.
+
+ We had code that made sure that cached 200 responses weren't used for conditional
+ requests. But it didn't work the other way - cached 304 responses got reused for
+ subsequent unconditional requests!
+
+ Adding the test uncovered this bug.
+
+ * loader/cache/CachedRawResource.cpp: (WebCore::shouldIgnoreHeaderForCacheReuse):
+ Should never ignore conditional headers. Code in determineRevalidationPolicy
+ was already undoing this for conditional requests, but we also shouldn't use
+ WebCore cache if it holds a 304 response to conditional request.
+
+ * loader/cache/CachedResourceLoader.cpp: (WebCore::CachedResourceLoader::determineRevalidationPolicy):
+ Even though the changed code is only for raw resources, I think that we can never
+ get a conditional request here any more.
+
2013-10-30 Alexey Proskuryakov <ap@apple.com>
CryptoAlgorithmDescriptionBuilder should support producing nested algorithms
diff --git a/Source/WebCore/loader/cache/CachedRawResource.cpp b/Source/WebCore/loader/cache/CachedRawResource.cpp
index 44128d1..a6eff5e 100644
--- a/Source/WebCore/loader/cache/CachedRawResource.cpp
+++ b/Source/WebCore/loader/cache/CachedRawResource.cpp
@@ -203,8 +203,6 @@
if (m_headers.isEmpty()) {
m_headers.add("Accept");
m_headers.add("Cache-Control");
- m_headers.add("If-Modified-Since");
- m_headers.add("If-None-Match");
m_headers.add("Origin");
m_headers.add("Pragma");
m_headers.add("Purpose");
diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.cpp b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
index b5b2fcf..9f9e457 100644
--- a/Source/WebCore/loader/cache/CachedResourceLoader.cpp
+++ b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
@@ -584,11 +584,8 @@
if (!existingResource->canReuse(request))
return Reload;
- // Certain requests (e.g., XHRs) might have manually set headers that require revalidation.
- // FIXME: In theory, this should be a Revalidate case. In practice, the MemoryCache revalidation path assumes a whole bunch
- // of things about how revalidation works that manual headers violate, so punt to Reload instead.
- if (request.isConditional())
- return Reload;
+ // Conditional requests should have failed canReuse check.
+ ASSERT(!request.isConditional());
// Do not load from cache if images are not enabled. The load for this image will be blocked
// in CachedImage::load.
