Changeset 229558 in webkit


Ignore:
Timestamp:
Mar 12, 2018 3:58:54 PM (6 years ago)
Author:
ajuma@chromium.org
Message:

http/tests/workers/service/service-worker-download.https.html times out with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183479

Reviewed by Youenn Fablet.

Source/WebKit:

Ensure that ServiceWorkerFetchClient::m_isCheckingResponse is set before code that depends on it
executes. This bit was set by code that's posted to the runloop using 'callOnMainThread' in
ServiceWorkerFetchClient::didReceiveResponse. But when didReceiveResponse is executing, tasks for
handling didReceiveData, didFail, or didFinish may already have been posted to the runloop, and in
that case would execute before m_isCheckingResponse gets set, and then incorrectly fail to
early-out. Fix this by directly setting m_isCheckingResponse in didReceiveResponse.

  • WebProcess/Storage/ServiceWorkerClientFetch.cpp:

(WebKit::ServiceWorkerClientFetch::start):
(WebKit::ServiceWorkerClientFetch::didReceiveResponse):

LayoutTests:

Add layout test coverage.

  • http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt: Added.
  • http/tests/workers/service/service-worker-download-async-delegates.https.html: Added.
Location:
trunk
Files:
2 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r229539 r229558  
     12018-03-12  Ali Juma  <ajuma@chromium.org>
     2
     3        http/tests/workers/service/service-worker-download.https.html times out with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183479
     5
     6        Reviewed by Youenn Fablet.
     7
     8        Add layout test coverage.
     9
     10        * http/tests/workers/service/service-worker-download-async-delegates.https-expected.txt: Added.
     11        * http/tests/workers/service/service-worker-download-async-delegates.https.html: Added.
     12
    1132018-03-12  Chris Dumez  <cdumez@apple.com>
    214
  • trunk/Source/WebKit/ChangeLog

    r229542 r229558  
     12018-03-12  Ali Juma  <ajuma@chromium.org>
     2
     3        http/tests/workers/service/service-worker-download.https.html times out with async policy delegates
     4        https://bugs.webkit.org/show_bug.cgi?id=183479
     5
     6        Reviewed by Youenn Fablet.
     7
     8        Ensure that ServiceWorkerFetchClient::m_isCheckingResponse is set before code that depends on it
     9        executes. This bit was set by code that's posted to the runloop using 'callOnMainThread' in
     10        ServiceWorkerFetchClient::didReceiveResponse. But when didReceiveResponse is executing, tasks for
     11        handling didReceiveData, didFail, or didFinish may already have been posted to the runloop, and in
     12        that case would execute before m_isCheckingResponse gets set, and then incorrectly fail to
     13        early-out. Fix this by directly setting m_isCheckingResponse in didReceiveResponse.
     14
     15        * WebProcess/Storage/ServiceWorkerClientFetch.cpp:
     16        (WebKit::ServiceWorkerClientFetch::start):
     17        (WebKit::ServiceWorkerClientFetch::didReceiveResponse):
     18
    1192018-03-12  Wenson Hsieh  <wenson_hsieh@apple.com>
    220
  • trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp

    r229477 r229558  
    6969    auto referrer = request.httpReferrer();
    7070
     71    m_didFail = false;
     72    m_didFinish = false;
     73
    7174    // We are intercepting fetch calls after going through the HTTP layer, which may add some specific headers.
    7275    cleanHTTPRequestHeadersForAccessControl(request, options.httpHeadersToKeep);
     
    101104void ServiceWorkerClientFetch::didReceiveResponse(ResourceResponse&& response)
    102105{
     106    m_isCheckingResponse = true;
    103107    callOnMainThread([this, protectedThis = makeRef(*this), response = WTFMove(response)]() mutable {
    104         if (!m_loader)
    105             return;
     108        if (!m_loader) {
     109            m_isCheckingResponse = false;
     110            return;
     111        }
    106112
    107113        if (auto error = validateResponse(response)) {
     114            m_isCheckingResponse = false;
    108115            m_loader->didFail(error.value());
    109116            ASSERT(!m_loader);
     
    115122
    116123        if (response.isRedirection() && response.httpHeaderFields().contains(HTTPHeaderName::Location)) {
     124            m_isCheckingResponse = false;
     125            continueLoadingAfterCheckingResponse();
    117126            m_redirectionStatus = RedirectionStatus::Receiving;
    118127            m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [protectedThis = makeRef(*this), this](ResourceRequest&& request) {
     
    143152            response.setURL(m_loader->request().url());
    144153
    145         m_isCheckingResponse = true;
    146154        m_loader->didReceiveResponse(response, [this, protectedThis = WTFMove(protectedThis)] {
    147155            m_isCheckingResponse = false;
Note: See TracChangeset for help on using the changeset viewer.