Attachment #333224 for bug #181392

View Details Raw Unified Return to Bug 181392

| Differences between



2018-02-06  Chris Dumez  <cdumez@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
        https://bugs.webkit.org/show_bug.cgi?id=181392
        <rdar://problem/36384136>

        Reviewed by NOBODY (OOPS!).

        All tasks from the StorageProcess to the WebContent process to update registrations
        and service workers state are posted to the runloop. However, the fetch callbacks
        do not do so. This means that fetch results might come in out of order with regards
        to the registration / service worker state updates. The test was flaky because an
        intercepted load would sometimes finish before the task to update the service worker
        state to "activated" was processed by the runloop. We address the issue by having
        the ServiceWorkerClientFetch callbacks schedule tasks to the runloop too.

        * WebProcess/Storage/ServiceWorkerClientFetch.cpp:
        (WebKit::ServiceWorkerClientFetch::didReceiveResponse):
        (WebKit::ServiceWorkerClientFetch::didReceiveData):
        (WebKit::ServiceWorkerClientFetch::didFinish):
        (WebKit::ServiceWorkerClientFetch::didFail):
        (WebKit::ServiceWorkerClientFetch::didNotHandle):

2018-02-06  Youenn Fablet  <youenn@apple.com>

        HasServiceWorkerRegistration bit should be sent when creating a new page




void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
{
    callOnMainThread([this, protectedThis = makeRef(*this), response = WTFMove(response)]() mutable {
        if (!m_loader)
            return;

        if (auto error = validateResponse(response)) {
            m_loader->didFail(error.value());
            if (auto callback = WTFMove(m_callback))
                callback(Result::Succeeded);
            return;
        }
        response.setSource(ResourceResponse::Source::ServiceWorker);

        if (response.isRedirection() && response.httpHeaderFields().contains(HTTPHeaderName::Location)) {
            m_redirectionStatus = RedirectionStatus::Receiving;
            m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [protectedThis = makeRef(*this), this](ResourceRequest&& request) {
                if (request.isNull() || !m_callback)
                    return;

                ASSERT(request == m_loader->request());
                if (m_redirectionStatus == RedirectionStatus::Received) {
                    start();
                    return;
                }
                m_redirectionStatus = RedirectionStatus::Following;
            });
            return;
        }

        // In case of main resource and mime type is the default one, we set it to text/html to pass more service worker WPT tests.
        // FIXME: We should refine our MIME type sniffing strategy for synthetic responses.
        if (m_loader->originalRequest().requester() == ResourceRequest::Requester::Main) {
            if (response.mimeType() == defaultMIMEType()) {
                response.setMimeType(ASCIILiteral("text/html"));
                response.setTextEncodingName(ASCIILiteral("UTF-8"));
    response.setSource(ResourceResponse::Source::ServiceWorker);

    if (response.isRedirection() && response.httpHeaderFields().contains(HTTPHeaderName::Location)) {
        m_redirectionStatus = RedirectionStatus::Receiving;
        m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [protectedThis = makeRef(*this), this](ResourceRequest&& request) {
            if (request.isNull() || !m_callback)
                return;

            ASSERT(request == m_loader->request());
            if (m_redirectionStatus == RedirectionStatus::Received) {
                start();
                return;
            }
            m_redirectionStatus = RedirectionStatus::Following;
        });
        return;
    }

    // In case of main resource and mime type is the default one, we set it to text/html to pass more service worker WPT tests.
    // FIXME: We should refine our MIME type sniffing strategy for synthetic responses.
    if (m_loader->originalRequest().requester() == ResourceRequest::Requester::Main) {
        if (response.mimeType() == defaultMIMEType()) {
            response.setMimeType(ASCIILiteral("text/html"));
            response.setTextEncodingName(ASCIILiteral("UTF-8"));
        }
    }

        // As per https://fetch.spec.whatwg.org/#main-fetch step 9, copy request's url list in response's url list if empty.
        if (response.url().isNull())
            response.setURL(m_loader->request().url());

        m_loader->didReceiveResponse(response);
        if (auto callback = WTFMove(m_callback))
            callback(Result::Succeeded);
    });
}

void ServiceWorkerClientFetch::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)
{
    callOnMainThread([this, protectedThis = makeRef(*this), data = data.vector(), encodedDataLength] {
        if (!m_loader)
            return;

        m_loader->didReceiveData(reinterpret_cast<const char*>(data.data()), data.size(), encodedDataLength, DataPayloadBytes);
    });
}

void ServiceWorkerClientFetch::didReceiveFormData(const IPC::FormDataReference&)


void ServiceWorkerClientFetch::didFinish()
{
    callOnMainThread([this, protectedThis = makeRef(*this)] {
        if (!m_loader)
            return;

        switch (m_redirectionStatus) {
        case RedirectionStatus::None:
            break;
        case RedirectionStatus::Receiving:
            m_redirectionStatus = RedirectionStatus::Received;
            return;
        case RedirectionStatus::Following:
            start();
            return;
        case RedirectionStatus::Received:
            ASSERT_NOT_REACHED();
            m_redirectionStatus = RedirectionStatus::None;
        }

        ASSERT(!m_callback);

        m_loader->didFinishLoading(NetworkLoadMetrics { });
        m_serviceWorkerProvider.fetchFinished(m_identifier);
    });
}

void ServiceWorkerClientFetch::didFail()
{
    callOnMainThread([this, protectedThis = makeRef(*this)] {
        if (!m_loader)
            return;

        m_loader->didFail({ ResourceError::Type::General });
        callback(Result::Succeeded);

        if (auto callback = WTFMove(m_callback))
            callback(Result::Succeeded);

        m_serviceWorkerProvider.fetchFinished(m_identifier);
    });
}

void ServiceWorkerClientFetch::didNotHandle()
{
    callOnMainThread([this, protectedThis = makeRef(*this)] {
        if (!m_loader)
            return;

        if (auto callback = WTFMove(m_callback))
            callback(Result::Unhandled);

        m_serviceWorkerProvider.fetchFinished(m_identifier);
    });
}

void ServiceWorkerClientFetch::cancel()
{
    if (auto callback = WTFMove(m_callback))
        callback(Result::Cancelled);
    m_loader = nullptr;
}

} // namespace WebKit

Lines 67-73 private: a/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h_sec1
- a/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h -1 / +1 lines
67	void didNotHandle();	67	void didNotHandle();
68		68
69	WebServiceWorkerProvider& m_serviceWorkerProvider;	69	WebServiceWorkerProvider& m_serviceWorkerProvider;
70	Ref<WebCore::ResourceLoader> m_loader;	70	RefPtr<WebCore::ResourceLoader> m_loader;
71	uint64_t m_identifier { 0 };	71	uint64_t m_identifier { 0 };
72	Ref<WebSWClientConnection> m_connection;	72	Ref<WebSWClientConnection> m_connection;
73	Callback m_callback;	73	Callback m_callback;



2018-02-06  Chris Dumez  <cdumez@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
        https://bugs.webkit.org/show_bug.cgi?id=181392
        <rdar://problem/36384136>

        Reviewed by NOBODY (OOPS!).

        Unskip test that is no longer flaky.

        * platform/mac-wk2/TestExpectations:

2018-02-06  Daniel Bates  <dabates@apple.com>

        Make mixed content basic auth tests more robust




webkit.org/b/181107 http/wpt/cache-storage/cache-put-stream.https.any.html [ Pass Failure ]

webkit.org/b/181392 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-waits-for-activate.https.html [ Pass Failure ]

webkit.org/b/172879 webrtc/video-unmute.html [ Skip ]

webkit.org/b/181502 swipe/pushstate-with-manual-scrollrestoration.html [ Failure ]

Return to Bug 181392

Lines 100-152 std::optional<ResourceError> ServiceWorkerClientFetch::validateResponse(const Re a/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp_sec1
- a/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp -65 / +87 lines
100		100
101	void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)	101	void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
102	{	102	{
103	auto protectedThis = makeRef(*this);	103	callOnMainThread([this, protectedThis = makeRef(*this), response = WTFMove(response)]() mutable {
		104	if (!m_loader)
		105	return;
		106
		107	if (auto error = validateResponse(response)) {
		108	m_loader->didFail(error.value());
		109	if (auto callback = WTFMove(m_callback))
		110	callback(Result::Succeeded);
		111	return;
		112	}
		113	response.setSource(ResourceResponse::Source::ServiceWorker);
		114
		115	if (response.isRedirection() && response.httpHeaderFields().contains(HTTPHeaderName::Location)) {
		116	m_redirectionStatus = RedirectionStatus::Receiving;
		117	m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [protectedThis = makeRef(*this), this](ResourceRequest&& request) {
		118	if (request.isNull() \|\| !m_callback)
		119	return;
		120
		121	ASSERT(request == m_loader->request());
		122	if (m_redirectionStatus == RedirectionStatus::Received) {
		123	start();
		124	return;
		125	}
		126	m_redirectionStatus = RedirectionStatus::Following;
		127	});
		128	return;
		129	}
104		130
105	if (auto error = validateResponse(response)) {	131	// In case of main resource and mime type is the default one, we set it to text/html to pass more service worker WPT tests.
106	m_loader->didFail(error.value());	132	// FIXME: We should refine our MIME type sniffing strategy for synthetic responses.
107	if (auto callback = WTFMove(m_callback))	133	if (m_loader->originalRequest().requester() == ResourceRequest::Requester::Main) {
108	callback(Result::Succeeded);	134	if (response.mimeType() == defaultMIMEType()) {
109	return;	135	response.setMimeType(ASCIILiteral("text/html"));
110	}	136	response.setTextEncodingName(ASCIILiteral("UTF-8"));
111	response.setSource(ResourceResponse::Source::ServiceWorker);
112
113	if (response.isRedirection() && response.httpHeaderFields().contains(HTTPHeaderName::Location)) {
114	m_redirectionStatus = RedirectionStatus::Receiving;
115	m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [protectedThis = makeRef(*this), this](ResourceRequest&& request) {
116	if (request.isNull() \|\| !m_callback)
117	return;
118
119	ASSERT(request == m_loader->request());
120	if (m_redirectionStatus == RedirectionStatus::Received) {
121	start();
122	return;
123	}	137	}
124	m_redirectionStatus = RedirectionStatus::Following;
125	});
126	return;
127	}
128
129	// In case of main resource and mime type is the default one, we set it to text/html to pass more service worker WPT tests.
130	// FIXME: We should refine our MIME type sniffing strategy for synthetic responses.
131	if (m_loader->originalRequest().requester() == ResourceRequest::Requester::Main) {
132	if (response.mimeType() == defaultMIMEType()) {
133	response.setMimeType(ASCIILiteral("text/html"));
134	response.setTextEncodingName(ASCIILiteral("UTF-8"));
135	}	138	}
136	}
137		139
138	// As per https://fetch.spec.whatwg.org/#main-fetch step 9, copy request's url list in response's url list if empty.	140	// As per https://fetch.spec.whatwg.org/#main-fetch step 9, copy request's url list in response's url list if empty.
139	if (response.url().isNull())	141	if (response.url().isNull())
140	response.setURL(m_loader->request().url());	142	response.setURL(m_loader->request().url());
141		143
142	m_loader->didReceiveResponse(response);	144	m_loader->didReceiveResponse(response);
143	if (auto callback = WTFMove(m_callback))	145	if (auto callback = WTFMove(m_callback))
144	callback(Result::Succeeded);	146	callback(Result::Succeeded);
		147	});
145	}	148	}
146		149
147	void ServiceWorkerClientFetch::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)	150	void ServiceWorkerClientFetch::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)
148	{	151	{
149	m_loader->didReceiveData(reinterpret_cast<const char*>(data.data()), data.size(), encodedDataLength, DataPayloadBytes);	152	callOnMainThread([this, protectedThis = makeRef(*this), data = data.vector(), encodedDataLength] {
		153	if (!m_loader)
		154	return;
		155
		156	m_loader->didReceiveData(reinterpret_cast<const char*>(data.data()), data.size(), encodedDataLength, DataPayloadBytes);
		157	});
150	}	158	}
151		159
152	void ServiceWorkerClientFetch::didReceiveFormData(const IPC::FormDataReference&)	160	void ServiceWorkerClientFetch::didReceiveFormData(const IPC::FormDataReference&)
Lines 156-205 void ServiceWorkerClientFetch::didReceiveFormData(const IPC::FormDataReference&) a/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp_sec2
156		164
157	void ServiceWorkerClientFetch::didFinish()	165	void ServiceWorkerClientFetch::didFinish()
158	{	166	{
159	switch (m_redirectionStatus) {	167	callOnMainThread([this, protectedThis = makeRef(*this)] {
160	case RedirectionStatus::None:	168	if (!m_loader)
161	break;	169	return;
162	case RedirectionStatus::Receiving:	170
163	m_redirectionStatus = RedirectionStatus::Received;	171	switch (m_redirectionStatus) {
164	return;	172	case RedirectionStatus::None:
165	case RedirectionStatus::Following:	173	break;
166	start();	174	case RedirectionStatus::Receiving:
167	return;	175	m_redirectionStatus = RedirectionStatus::Received;
168	case RedirectionStatus::Received:	176	return;
169	ASSERT_NOT_REACHED();	177	case RedirectionStatus::Following:
170	m_redirectionStatus = RedirectionStatus::None;	178	start();
171	}	179	return;
172		180	case RedirectionStatus::Received:
173	ASSERT(!m_callback);	181	ASSERT_NOT_REACHED();
174		182	m_redirectionStatus = RedirectionStatus::None;
175	auto protectedThis = makeRef(*this);	183	}
176	m_loader->didFinishLoading(NetworkLoadMetrics { });	184
177	m_serviceWorkerProvider.fetchFinished(m_identifier);	185	ASSERT(!m_callback);
		186
		187	m_loader->didFinishLoading(NetworkLoadMetrics { });
		188	m_serviceWorkerProvider.fetchFinished(m_identifier);
		189	});
178	}	190	}
179		191
180	void ServiceWorkerClientFetch::didFail()	192	void ServiceWorkerClientFetch::didFail()
181	{	193	{
182	auto protectedThis = makeRef(*this);	194	callOnMainThread([this, protectedThis = makeRef(*this)] {
183	m_loader->didFail({ ResourceError::Type::General });	195	if (!m_loader)
		196	return;
184		197
185	if (auto callback = WTFMove(m_callback))	198	m_loader->didFail({ ResourceError::Type::General });
186	callback(Result::Succeeded);
187		199
188	m_serviceWorkerProvider.fetchFinished(m_identifier);	200	if (auto callback = WTFMove(m_callback))
		201	callback(Result::Succeeded);
		202
		203	m_serviceWorkerProvider.fetchFinished(m_identifier);
		204	});
189	}	205	}
190		206
191	void ServiceWorkerClientFetch::didNotHandle()	207	void ServiceWorkerClientFetch::didNotHandle()
192	{	208	{
193	if (auto callback = WTFMove(m_callback))	209	callOnMainThread([this, protectedThis = makeRef(*this)] {
194	callback(Result::Unhandled);	210	if (!m_loader)
		211	return;
		212
		213	if (auto callback = WTFMove(m_callback))
		214	callback(Result::Unhandled);
195		215
196	m_serviceWorkerProvider.fetchFinished(m_identifier);	216	m_serviceWorkerProvider.fetchFinished(m_identifier);
		217	});
197	}	218	}
198		219
199	void ServiceWorkerClientFetch::cancel()	220	void ServiceWorkerClientFetch::cancel()
200	{	221	{
201	if (auto callback = WTFMove(m_callback))	222	if (auto callback = WTFMove(m_callback))
202	callback(Result::Cancelled);	223	callback(Result::Cancelled);
		224	m_loader = nullptr;
203	}	225	}
204		226
205	} // namespace WebKit	227	} // namespace WebKit

Lines 858-865 webkit.org/b/181069 fast/mediastream/MediaStream-MediaElement-setObject-null.htm a/LayoutTests/platform/mac-wk2/TestExpectations_sec1
- a/LayoutTests/platform/mac-wk2/TestExpectations -2 lines
858		858
859	webkit.org/b/181107 http/wpt/cache-storage/cache-put-stream.https.any.html [ Pass Failure ]	859	webkit.org/b/181107 http/wpt/cache-storage/cache-put-stream.https.any.html [ Pass Failure ]
860		860
861	webkit.org/b/181392 imported/w3c/web-platform-tests/service-workers/service-worker/fetch-waits-for-activate.https.html [ Pass Failure ]
862
863	webkit.org/b/172879 webrtc/video-unmute.html [ Skip ]	861	webkit.org/b/172879 webrtc/video-unmute.html [ Skip ]
864		862
865	webkit.org/b/181502 swipe/pushstate-with-manual-scrollrestoration.html [ Failure ]	863	webkit.org/b/181502 swipe/pushstate-with-manual-scrollrestoration.html [ Failure ]

Lines 1-3 a/Source/WebKit/ChangeLog_sec1
- a/Source/WebKit/ChangeLog +23 lines
		1	2018-02-06 Chris Dumez <cdumez@apple.com>
		2
		3	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
		4	https://bugs.webkit.org/show_bug.cgi?id=181392
		5	<rdar://problem/36384136>
		6
		7	Reviewed by NOBODY (OOPS!).
		8
		9	All tasks from the StorageProcess to the WebContent process to update registrations
		10	and service workers state are posted to the runloop. However, the fetch callbacks
		11	do not do so. This means that fetch results might come in out of order with regards
		12	to the registration / service worker state updates. The test was flaky because an
		13	intercepted load would sometimes finish before the task to update the service worker
		14	state to "activated" was processed by the runloop. We address the issue by having
		15	the ServiceWorkerClientFetch callbacks schedule tasks to the runloop too.
		16
		17	* WebProcess/Storage/ServiceWorkerClientFetch.cpp:
		18	(WebKit::ServiceWorkerClientFetch::didReceiveResponse):
		19	(WebKit::ServiceWorkerClientFetch::didReceiveData):
		20	(WebKit::ServiceWorkerClientFetch::didFinish):
		21	(WebKit::ServiceWorkerClientFetch::didFail):
		22	(WebKit::ServiceWorkerClientFetch::didNotHandle):
		23
1	2018-02-06 Youenn Fablet <youenn@apple.com>	24	2018-02-06 Youenn Fablet <youenn@apple.com>
2		25
3	HasServiceWorkerRegistration bit should be sent when creating a new page	26	HasServiceWorkerRegistration bit should be sent when creating a new page

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +12 lines
		1	2018-02-06 Chris Dumez <cdumez@apple.com>
		2
		3	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
		4	https://bugs.webkit.org/show_bug.cgi?id=181392
		5	<rdar://problem/36384136>
		6
		7	Reviewed by NOBODY (OOPS!).
		8
		9	Unskip test that is no longer flaky.
		10
		11	* platform/mac-wk2/TestExpectations:
		12
1	2018-02-06 Daniel Bates <dabates@apple.com>	13	2018-02-06 Daniel Bates <dabates@apple.com>
2		14
3	Make mixed content basic auth tests more robust	15	Make mixed content basic auth tests more robust