Title: [158362] trunk
- Revision
- 158362
- Author
- [email protected]
- Date
- 2013-10-31 09:50:03 -0700 (Thu, 31 Oct 2013)
Log Message
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.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (158361 => 158362)
--- trunk/LayoutTests/ChangeLog 2013-10-31 16:28:14 UTC (rev 158361)
+++ trunk/LayoutTests/ChangeLog 2013-10-31 16:50:03 UTC (rev 158362)
@@ -1,3 +1,16 @@
+2013-10-31 Alexey Proskuryakov <[email protected]>
+
+ 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 <[email protected]>
[EFL] Rebaselining after r158186
Modified: trunk/LayoutTests/TestExpectations (158361 => 158362)
--- trunk/LayoutTests/TestExpectations 2013-10-31 16:28:14 UTC (rev 158361)
+++ trunk/LayoutTests/TestExpectations 2013-10-31 16:50:03 UTC (rev 158362)
@@ -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 ]
Modified: trunk/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt (158361 => 158362)
--- trunk/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt 2013-10-31 16:28:14 UTC (rev 158361)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer-expected.txt 2013-10-31 16:50:03 UTC (rev 158362)
@@ -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
Modified: trunk/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer.html (158361 => 158362)
--- trunk/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer.html 2013-10-31 16:28:14 UTC (rev 158361)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/response-empty-arraybuffer.html 2013-10-31 16:50:03 UTC (rev 158362)
@@ -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]'");
Modified: trunk/Source/WebCore/ChangeLog (158361 => 158362)
--- trunk/Source/WebCore/ChangeLog 2013-10-31 16:28:14 UTC (rev 158361)
+++ trunk/Source/WebCore/ChangeLog 2013-10-31 16:50:03 UTC (rev 158362)
@@ -1,3 +1,25 @@
+2013-10-31 Alexey Proskuryakov <[email protected]>
+
+ 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 <[email protected]>
CryptoAlgorithmDescriptionBuilder should support producing nested algorithms
Modified: trunk/Source/WebCore/loader/cache/CachedRawResource.cpp (158361 => 158362)
--- trunk/Source/WebCore/loader/cache/CachedRawResource.cpp 2013-10-31 16:28:14 UTC (rev 158361)
+++ trunk/Source/WebCore/loader/cache/CachedRawResource.cpp 2013-10-31 16:50:03 UTC (rev 158362)
@@ -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");
Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (158361 => 158362)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2013-10-31 16:28:14 UTC (rev 158361)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2013-10-31 16:50:03 UTC (rev 158362)
@@ -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.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
