- Revision
- 228199
- Author
- commit-qu...@webkit.org
- Date
- 2018-02-06 16:09:21 -0800 (Tue, 06 Feb 2018)
Log Message
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-respond-with-response-body-with-invalid-chunk.https.html is flaky
https://bugs.webkit.org/show_bug.cgi?id=182541
Patch by Youenn Fablet <you...@apple.com> on 2018-02-06
Reviewed by Chris Dumez.
Source/WebCore:
Covered by test being no longer flaky.
In case of loading error when getting the response body, we were only reporting
the error if there was a callback set or a ReadableStream already created.
Otherwise, we were just stopping loading and if creating a ReadableStream, we were just returning an empty body.
FetchBodyOwner now stores a loading error.
In case a readable stream is created, it will error it if there is a loading error.
If there is not and the loading failed later on, the stream will be errored using the current code path.
* Modules/cache/DOMCache.cpp:
(WebCore::DOMCache::put):
* Modules/fetch/FetchResponse.cpp:
(WebCore::FetchResponse::clone):
(WebCore::FetchResponse::BodyLoader::didFail):
* Modules/fetch/FetchResponse.h:
* workers/service/context/ServiceWorkerFetch.cpp:
(WebCore::ServiceWorkerFetch::processResponse):
LayoutTests:
* TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (228198 => 228199)
--- trunk/LayoutTests/ChangeLog 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/LayoutTests/ChangeLog 2018-02-07 00:09:21 UTC (rev 228199)
@@ -1,3 +1,12 @@
+2018-02-06 Youenn Fablet <you...@apple.com>
+
+ imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-respond-with-response-body-with-invalid-chunk.https.html is flaky
+ https://bugs.webkit.org/show_bug.cgi?id=182541
+
+ Reviewed by Chris Dumez.
+
+ * TestExpectations:
+
2018-02-06 Chris Dumez <cdu...@apple.com>
Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/fetch-waits-for-activate.https.html is a flaky failure on macOS and iOS
Modified: trunk/LayoutTests/TestExpectations (228198 => 228199)
--- trunk/LayoutTests/TestExpectations 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/LayoutTests/TestExpectations 2018-02-07 00:09:21 UTC (rev 228199)
@@ -177,7 +177,6 @@
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-request-css-images.https.html [ Skip ]
webkit.org/b/179248 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-within-sw.https.html [ Pass Failure ]
-webkit.org/b/182541 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-respond-with-response-body-with-invalid-chunk.https.html [ Pass Failure ]
imported/w3c/web-platform-tests/service-workers/service-worker/websocket.https.html [ Pass Failure ]
webkit.org/b/181901 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-cors-xhr.https.html [ DumpJSConsoleLogInStdErr Pass Failure ]
Modified: trunk/Source/WebCore/ChangeLog (228198 => 228199)
--- trunk/Source/WebCore/ChangeLog 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/Source/WebCore/ChangeLog 2018-02-07 00:09:21 UTC (rev 228199)
@@ -1,3 +1,28 @@
+2018-02-06 Youenn Fablet <you...@apple.com>
+
+ imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-respond-with-response-body-with-invalid-chunk.https.html is flaky
+ https://bugs.webkit.org/show_bug.cgi?id=182541
+
+ Reviewed by Chris Dumez.
+
+ Covered by test being no longer flaky.
+ In case of loading error when getting the response body, we were only reporting
+ the error if there was a callback set or a ReadableStream already created.
+ Otherwise, we were just stopping loading and if creating a ReadableStream, we were just returning an empty body.
+
+ FetchBodyOwner now stores a loading error.
+ In case a readable stream is created, it will error it if there is a loading error.
+ If there is not and the loading failed later on, the stream will be errored using the current code path.
+
+ * Modules/cache/DOMCache.cpp:
+ (WebCore::DOMCache::put):
+ * Modules/fetch/FetchResponse.cpp:
+ (WebCore::FetchResponse::clone):
+ (WebCore::FetchResponse::BodyLoader::didFail):
+ * Modules/fetch/FetchResponse.h:
+ * workers/service/context/ServiceWorkerFetch.cpp:
+ (WebCore::ServiceWorkerFetch::processResponse):
+
2018-02-06 Andy Estes <aes...@apple.com>
[Payment Request] show() should take an optional PaymentDetailsUpdate promise
Modified: trunk/Source/WebCore/Modules/cache/DOMCache.cpp (228198 => 228199)
--- trunk/Source/WebCore/Modules/cache/DOMCache.cpp 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/Source/WebCore/Modules/cache/DOMCache.cpp 2018-02-07 00:09:21 UTC (rev 228199)
@@ -320,6 +320,11 @@
}
auto request = requestOrException.releaseReturnValue();
+ if (response->loadingError()) {
+ promise.reject(Exception { TypeError, String { response->loadingError()->localizedDescription() } });
+ return;
+ }
+
if (hasResponseVaryStarHeaderValue(response.get())) {
promise.reject(Exception { TypeError, ASCIILiteral("Response has a '*' Vary header value") });
return;
Modified: trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.cpp (228198 => 228199)
--- trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.cpp 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.cpp 2018-02-07 00:09:21 UTC (rev 228199)
@@ -312,6 +312,12 @@
{
ASSERT(m_readableStreamSource);
+ if (m_loadingError) {
+ auto errorMessage = m_loadingError->localizedDescription();
+ m_readableStreamSource->error(errorMessage.isEmpty() ? ASCIILiteral("Loading failed") : errorMessage);
+ return;
+ }
+
body().consumeAsStream(*this, *m_readableStreamSource);
if (!m_readableStreamSource->isPulling())
m_readableStreamSource = nullptr;
Modified: trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.h (228198 => 228199)
--- trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.h 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.h 2018-02-07 00:09:21 UTC (rev 228199)
@@ -34,6 +34,7 @@
#include "FetchHeaders.h"
#include "FetchLoader.h"
#include "FetchLoaderClient.h"
+#include "ResourceError.h"
namespace WebCore {
@@ -114,6 +115,7 @@
RefPtr<FetchBodySource> m_readableStreamSource;
#endif
Ref<FetchHeaders> m_headers;
+ std::optional<ResourceError> m_loadingError;
private:
std::optional<BlobLoader> m_blobLoader;
Modified: trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp (228198 => 228199)
--- trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp 2018-02-07 00:09:21 UTC (rev 228199)
@@ -176,6 +176,7 @@
m_internalResponse.setHTTPHeaderFields(HTTPHeaderMap { headers().internalHeaders() });
auto clone = FetchResponse::create(context, std::nullopt, headers().guard(), ResourceResponse { m_internalResponse });
+ clone->m_loadingError = m_loadingError;
clone->cloneBody(*this);
clone->m_opaqueLoadIdentifier = m_opaqueLoadIdentifier;
clone->m_bodySizeWithPadding = m_bodySizeWithPadding;
@@ -236,6 +237,9 @@
void FetchResponse::BodyLoader::didFail(const ResourceError& error)
{
ASSERT(m_response.hasPendingActivity());
+
+ m_response.m_loadingError = error;
+
if (auto responseCallback = WTFMove(m_responseCallback))
responseCallback(Exception { TypeError, String(error.localizedDescription()) });
Modified: trunk/Source/WebCore/Modules/fetch/FetchResponse.h (228198 => 228199)
--- trunk/Source/WebCore/Modules/fetch/FetchResponse.h 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/Source/WebCore/Modules/fetch/FetchResponse.h 2018-02-07 00:09:21 UTC (rev 228199)
@@ -105,6 +105,8 @@
void initializeOpaqueLoadIdentifierForTesting() { m_opaqueLoadIdentifier = 1; }
+ const std::optional<ResourceError>& loadingError() const { return m_loadingError; }
+
private:
FetchResponse(ScriptExecutionContext&, std::optional<FetchBody>&&, Ref<FetchHeaders>&&, ResourceResponse&&);
Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp (228198 => 228199)
--- trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp 2018-02-07 00:03:06 UTC (rev 228198)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp 2018-02-07 00:09:21 UTC (rev 228199)
@@ -53,6 +53,11 @@
client->didReceiveResponse(response->resourceResponse());
+ if (response->loadingError()) {
+ client->didFail();
+ return;
+ }
+
if (response->isBodyReceivedByChunk()) {
response->consumeBodyReceivedByChunk([client = WTFMove(client)] (auto&& result) mutable {
if (result.hasException()) {
