Changeset 241317 in webkit


Ignore:
Timestamp:
Feb 12, 2019, 1:49:39 PM (6 years ago)
Author:
achristensen@apple.com
Message:

Remove setDefersLoading infrastructure from WebKit2
​https://bugs.webkit.org/show_bug.cgi?id=194506

Reviewed by Brady Eidson.

Source/WebCore:

setDefersLoading is inherently racy from WebCore to the NetworkProcess,
it adds unwanted complexity to the initialization and use of network objects,
and it has led to many unrecoverable hang bugs over the years.
We needed to force it into WebKit2 to transition some existing clients who relied on it,
but we have recently finished transitioning those clients to other solutions, mostly
completion handlers.

  • inspector/PageScriptDebugServer.cpp:

(WebCore::PageScriptDebugServer::setJavaScriptPaused):

Source/WebKit:

  • NetworkProcess/NetworkConnectionToWebProcess.cpp:

(WebKit::NetworkConnectionToWebProcess::setDefersLoading): Deleted.

  • NetworkProcess/NetworkConnectionToWebProcess.h:
  • NetworkProcess/NetworkConnectionToWebProcess.messages.in:
  • NetworkProcess/NetworkDataTask.h:
  • NetworkProcess/NetworkDataTaskBlob.cpp:

(WebKit::NetworkDataTaskBlob::suspend): Deleted.

  • NetworkProcess/NetworkDataTaskBlob.h:
  • NetworkProcess/NetworkLoad.cpp:

(WebKit::NetworkLoad::initialize):
(WebKit::NetworkLoad::setDefersLoading): Deleted.

  • NetworkProcess/NetworkLoad.h:
  • NetworkProcess/NetworkLoadParameters.h:
  • NetworkProcess/NetworkResourceLoadParameters.cpp:

(WebKit::NetworkResourceLoadParameters::encode const):
(WebKit::NetworkResourceLoadParameters::decode):

  • NetworkProcess/NetworkResourceLoader.cpp:

(WebKit::NetworkResourceLoader::start):
(WebKit::NetworkResourceLoader::startNetworkLoad):
(WebKit::NetworkResourceLoader::setDefersLoading): Deleted.

  • NetworkProcess/NetworkResourceLoader.h:
  • NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
  • NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:

(WebKit::NetworkDataTaskCocoa::suspend): Deleted.

  • NetworkProcess/curl/NetworkDataTaskCurl.cpp:

(WebKit::NetworkDataTaskCurl::suspend): Deleted.

  • NetworkProcess/curl/NetworkDataTaskCurl.h:
  • NetworkProcess/soup/NetworkDataTaskSoup.cpp:

(WebKit::NetworkDataTaskSoup::suspend): Deleted.

  • NetworkProcess/soup/NetworkDataTaskSoup.h:
  • WebProcess/Network/WebLoaderStrategy.cpp:

(WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
(WebKit::WebLoaderStrategy::setDefersLoading):

Tools:

  • WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
  • WebKitTestRunner/InjectedBundle/TestRunner.cpp:

(WTR::TestRunner::setDefersLoading): Deleted.

  • WebKitTestRunner/InjectedBundle/TestRunner.h:

LayoutTests:

  • platform/wk2/TestExpectations:
Location:
trunk
Files:
28 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r241310 r241317  
     12019-02-12  Alex Christensen  <achristensen@webkit.org>
     2
     3        Remove setDefersLoading infrastructure from WebKit2
     4        https://bugs.webkit.org/show_bug.cgi?id=194506
     5
     6        Reviewed by Brady Eidson.
     7
     8        * platform/wk2/TestExpectations:
     9
    1102019-02-12  Justin Fan  <justin_fan@apple.com>
    211

  • trunk/LayoutTests/platform/wk2/TestExpectations

    r241127 r241317  
    537537
    538538# setDefersLoading is not supported in WK2.
    539 loader/load-defer.html [ WontFix ]
     539loader/load-defer.html [ Skip ]
     540loader/load-defer-resume-crash.html [ Skip ]
     541loader/navigation-while-deferring-loads.html [ Skip ]
    540542
    541543# WebKitTestRunner doesn't have objCController

  • trunk/Source/WebCore/ChangeLog

    r241316 r241317  
     12019-02-12  Alex Christensen  <achristensen@webkit.org>
     2
     3        Remove setDefersLoading infrastructure from WebKit2
     4        https://bugs.webkit.org/show_bug.cgi?id=194506
     5
     6        Reviewed by Brady Eidson.
     7
     8        setDefersLoading is inherently racy from WebCore to the NetworkProcess,
     9        it adds unwanted complexity to the initialization and use of network objects,
     10        and it has led to many unrecoverable hang bugs over the years.
     11        We needed to force it into WebKit2 to transition some existing clients who relied on it,
     12        but we have recently finished transitioning those clients to other solutions, mostly
     13        completion handlers.
     14
     15        * inspector/PageScriptDebugServer.cpp:
     16        (WebCore::PageScriptDebugServer::setJavaScriptPaused):
     17
    1182019-02-12  Michael Catanzaro  <mcatanzaro@igalia.com>
    219

  • trunk/Source/WebCore/inspector/PageScriptDebugServer.cpp

    r237266 r241317  
    139139
    140140    for (auto& page : pageGroup.pages()) {
    141         page->setDefersLoading(paused);
    142 
    143141        for (Frame* frame = &page->mainFrame(); frame; frame = frame->tree().traverseNext())
    144142            setJavaScriptPaused(*frame, paused);

  • trunk/Source/WebKit/ChangeLog

    r241316 r241317  
     12019-02-12  Alex Christensen  <achristensen@webkit.org>
     2
     3        Remove setDefersLoading infrastructure from WebKit2
     4        https://bugs.webkit.org/show_bug.cgi?id=194506
     5
     6        Reviewed by Brady Eidson.
     7
     8        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
     9        (WebKit::NetworkConnectionToWebProcess::setDefersLoading): Deleted.
     10        * NetworkProcess/NetworkConnectionToWebProcess.h:
     11        * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
     12        * NetworkProcess/NetworkDataTask.h:
     13        * NetworkProcess/NetworkDataTaskBlob.cpp:
     14        (WebKit::NetworkDataTaskBlob::suspend): Deleted.
     15        * NetworkProcess/NetworkDataTaskBlob.h:
     16        * NetworkProcess/NetworkLoad.cpp:
     17        (WebKit::NetworkLoad::initialize):
     18        (WebKit::NetworkLoad::setDefersLoading): Deleted.
     19        * NetworkProcess/NetworkLoad.h:
     20        * NetworkProcess/NetworkLoadParameters.h:
     21        * NetworkProcess/NetworkResourceLoadParameters.cpp:
     22        (WebKit::NetworkResourceLoadParameters::encode const):
     23        (WebKit::NetworkResourceLoadParameters::decode):
     24        * NetworkProcess/NetworkResourceLoader.cpp:
     25        (WebKit::NetworkResourceLoader::start):
     26        (WebKit::NetworkResourceLoader::startNetworkLoad):
     27        (WebKit::NetworkResourceLoader::setDefersLoading): Deleted.
     28        * NetworkProcess/NetworkResourceLoader.h:
     29        * NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
     30        * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
     31        (WebKit::NetworkDataTaskCocoa::suspend): Deleted.
     32        * NetworkProcess/curl/NetworkDataTaskCurl.cpp:
     33        (WebKit::NetworkDataTaskCurl::suspend): Deleted.
     34        * NetworkProcess/curl/NetworkDataTaskCurl.h:
     35        * NetworkProcess/soup/NetworkDataTaskSoup.cpp:
     36        (WebKit::NetworkDataTaskSoup::suspend): Deleted.
     37        * NetworkProcess/soup/NetworkDataTaskSoup.h:
     38        * WebProcess/Network/WebLoaderStrategy.cpp:
     39        (WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
     40        (WebKit::WebLoaderStrategy::setDefersLoading):
     41
    1422019-02-12  Michael Catanzaro  <mcatanzaro@igalia.com>
    243

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp

    r241008 r241317  
    387387}
    388388
    389 void NetworkConnectionToWebProcess::setDefersLoading(ResourceLoadIdentifier identifier, bool defers)
    390 {
    391     RELEASE_ASSERT(identifier);
    392     RELEASE_ASSERT(RunLoop::isMain());
    393 
    394     RefPtr<NetworkResourceLoader> loader = m_networkResourceLoaders.get(identifier);
    395     if (!loader)
    396         return;
    397 
    398     loader->setDefersLoading(defers);
    399 }
    400 
    401389void NetworkConnectionToWebProcess::prefetchDNS(const String& hostname)
    402390{

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h

    r241008 r241317  
    150150    void removeLoadIdentifier(ResourceLoadIdentifier);
    151151    void pageLoadCompleted(uint64_t webPageID);
    152     void setDefersLoading(ResourceLoadIdentifier, bool);
    153152    void crossOriginRedirectReceived(ResourceLoadIdentifier, const URL& redirectURL);
    154153    void startDownload(PAL::SessionID, DownloadID, const WebCore::ResourceRequest&, const String& suggestedName = { });

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in

    r240931 r241317  
    2828    RemoveLoadIdentifier(uint64_t resourceLoadIdentifier)
    2929    PageLoadCompleted(uint64_t webPageID)
    30     SetDefersLoading(uint64_t resourceLoadIdentifier, bool defers)
    3130    PrefetchDNS(String hostname)
    3231    PreconnectTo(uint64_t preconnectionIdentifier, WebKit::NetworkResourceLoadParameters loadParameters);

  • trunk/Source/WebKit/NetworkProcess/NetworkDataTask.h

    r239007 r241317  
    8484    virtual ~NetworkDataTask();
    8585
    86     virtual void suspend() = 0;
    8786    virtual void cancel() = 0;
    8887    virtual void resume() = 0;

  • trunk/Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp

    r241008 r241317  
    145145}
    146146
    147 void NetworkDataTaskBlob::suspend()
    148 {
    149     // FIXME: can this happen?
    150 }
    151 
    152147void NetworkDataTaskBlob::cancel()
    153148{

  • trunk/Source/WebKit/NetworkProcess/NetworkDataTaskBlob.h

    r241008 r241317  
    6060    NetworkDataTaskBlob(NetworkSession&, WebCore::BlobRegistryImpl&, NetworkDataTaskClient&, const WebCore::ResourceRequest&, WebCore::ContentSniffingPolicy, const Vector<RefPtr<WebCore::BlobDataFileReference>>&);
    6161
    62     void suspend() override;
    6362    void cancel() override;
    6463    void resume() override;

  • trunk/Source/WebKit/NetworkProcess/NetworkLoad.cpp

    r241183 r241317  
    7070        m_task = NetworkDataTask::create(networkSession, *this, m_parameters);
    7171
    72     if (!m_parameters.defersLoading)
    73         m_task->resume();
     72    m_task->resume();
    7473}
    7574
    … …  
    8180    if (m_task)
    8281        m_task->clearClient();
    83 }
    84 
    85 void NetworkLoad::setDefersLoading(bool defers)
    86 {
    87     if (m_task) {
    88         if (defers)
    89             m_task->suspend();
    90         else
    91             m_task->resume();
    92     }
    9382}
    9483

  • trunk/Source/WebKit/NetworkProcess/NetworkLoad.h

    r241008 r241317  
    4848    ~NetworkLoad();
    4949
    50     void setDefersLoading(bool);
    5150    void cancel();
    5251

  • trunk/Source/WebKit/NetworkProcess/NetworkLoadParameters.h

    r239622 r241317  
    4949    WebCore::ClientCredentialPolicy clientCredentialPolicy { WebCore::ClientCredentialPolicy::CannotAskClientForCredentials };
    5050    bool shouldClearReferrerOnHTTPSToHTTPRedirect { true };
    51     bool defersLoading { false };
    5251    bool needsCertificateInfo { false };
    5352    bool isMainFrameNavigation { false };

  • trunk/Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.cpp

    r239622 r241317  
    7777    encoder.encodeEnum(shouldPreconnectOnly);
    7878    encoder << shouldClearReferrerOnHTTPSToHTTPRedirect;
    79     encoder << defersLoading;
    8079    encoder << needsCertificateInfo;
    8180    encoder << isMainFrameNavigation;
    … …  
    164163    if (!decoder.decode(result.shouldClearReferrerOnHTTPSToHTTPRedirect))
    165164        return false;
    166     if (!decoder.decode(result.defersLoading))
    167         return false;
    168165    if (!decoder.decode(result.needsCertificateInfo))
    169166        return false;

  • trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp

    r241244 r241317  
    9595    , m_connection { connection }
    9696    , m_fileReferences(connection.resolveBlobReferences(m_parameters))
    97     , m_defersLoading { parameters.defersLoading }
    9897    , m_isAllowedToAskUserForCredentials { m_parameters.clientCredentialPolicy == ClientCredentialPolicy::MayAskClientForCredentials }
    9998    , m_bufferingTimer { *this, &NetworkResourceLoader::bufferingTimerFired }
    … …  
    166165
    167166    m_networkActivityTracker = m_connection->startTrackingResourceLoad(m_parameters.webPageID, m_parameters.identifier, isMainResource(), sessionID());
    168 
    169     if (m_defersLoading) {
    170         RELEASE_LOG_IF_ALLOWED("start: Loading is deferred (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d, parentPID = %d)", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous(), m_parameters.parentPID);
    171         return;
    172     }
    173167
    174168    ASSERT(!m_wasStarted);
    … …  
    273267
    274268    NetworkLoadParameters parameters = m_parameters;
    275     parameters.defersLoading = m_defersLoading;
    276269    parameters.networkActivityTracker = m_networkActivityTracker;
    277270    if (m_networkLoadChecker)
    … …  
    298291
    299292    RELEASE_LOG_IF_ALLOWED("startNetworkLoad: (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", description = %{public}s)", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier, m_networkLoad->description().utf8().data());
    300 
    301     if (m_defersLoading) {
    302         RELEASE_LOG_IF_ALLOWED("startNetworkLoad: Created, but deferred (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")",
    303             m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier);
    304     }
    305 }
    306 
    307 void NetworkResourceLoader::setDefersLoading(bool defers)
    308 {
    309     if (m_defersLoading == defers)
    310         return;
    311     m_defersLoading = defers;
    312 
    313     if (defers)
    314         RELEASE_LOG_IF_ALLOWED("setDefersLoading: Deferring resource load (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier);
    315     else
    316         RELEASE_LOG_IF_ALLOWED("setDefersLoading: Resuming deferred resource load (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier);
    317 
    318     if (m_networkLoad) {
    319         m_networkLoad->setDefersLoading(defers);
    320         return;
    321     }
    322 
    323     if (!m_defersLoading && !m_wasStarted)
    324         start();
    325     else
    326         RELEASE_LOG_IF_ALLOWED("setDefersLoading: defers = %d, but nothing to do (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ")", m_defersLoading, m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier);
    327293}
    328294

  • trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.h

    r240955 r241317  
    7373    void abort();
    7474
    75     void setDefersLoading(bool);
    76 
    7775    // Message handlers.
    7876    void didReceiveNetworkResourceLoaderMessage(IPC::Connection&, IPC::Decoder&);
    … …  
    192190    bool m_wasStarted { false };
    193191    bool m_didConsumeSandboxExtensions { false };
    194     bool m_defersLoading { false };
    195192    bool m_isAllowedToAskUserForCredentials { false };
    196193    size_t m_numBytesReceived { 0 };

  • trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h

    r240973 r241317  
    6161    void transferSandboxExtensionToDownload(Download&);
    6262
    63     void suspend() override;
    6463    void cancel() override;
    6564    void resume() override;

  • trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm

    r240973 r241317  
    477477}
    478478
    479 void NetworkDataTaskCocoa::suspend()
    480 {
    481     if (m_failureTimer.isActive())
    482         m_failureTimer.stop();
    483     [m_task suspend];
    484 }
    485 
    486479NetworkDataTask::State NetworkDataTaskCocoa::state() const
    487480{

  • trunk/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp

    r240029 r241317  
    9797}
    9898
    99 void NetworkDataTaskCurl::suspend()
    100 {
    101     ASSERT(m_state != State::Suspended);
    102     if (m_state == State::Canceling || m_state == State::Completed)
    103         return;
    104 
    105     m_state = State::Suspended;
    106 
    107     if (m_curlRequest)
    108         m_curlRequest->suspend();
    109 }
    110 
    11199void NetworkDataTaskCurl::cancel()
    112100{

  • trunk/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h

    r236073 r241317  
    5858    NetworkDataTaskCurl(NetworkSession&, NetworkDataTaskClient&, const WebCore::ResourceRequest&, WebCore::StoredCredentialsPolicy, WebCore::ContentSniffingPolicy, WebCore::ContentEncodingSniffingPolicy, bool shouldClearReferrerOnHTTPSToHTTPRedirect, bool dataTaskIsForMainFrameNavigation);
    5959
    60     void suspend() override;
    6160    void cancel() override;
    6261    void resume() override;

  • trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp

    r240925 r241317  
    234234            ASSERT_NOT_REACHED();
    235235    }
    236 }
    237 
    238 void NetworkDataTaskSoup::suspend()
    239 {
    240     ASSERT(m_state != State::Suspended);
    241     if (m_state == State::Canceling || m_state == State::Completed)
    242         return;
    243     m_state = State::Suspended;
    244 
    245     stopTimeout();
    246236}
    247237

  • trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h

    r231008 r241317  
    4747    NetworkDataTaskSoup(NetworkSession&, NetworkDataTaskClient&, const WebCore::ResourceRequest&, WebCore::StoredCredentialsPolicy, WebCore::ContentSniffingPolicy, WebCore::ContentEncodingSniffingPolicy, bool shouldClearReferrerOnHTTPSToHTTPRedirect, bool dataTaskIsForMainFrameNavigation);
    4848
    49     void suspend() override;
    5049    void cancel() override;
    5150    void resume() override;

  • trunk/Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp

    r241308 r241317  
    278278    loadParameters.clientCredentialPolicy = (loadParameters.webFrameID && loadParameters.webPageID && resourceLoader.isAllowedToAskUserForCredentials()) ? ClientCredentialPolicy::MayAskClientForCredentials : ClientCredentialPolicy::CannotAskClientForCredentials;
    279279    loadParameters.shouldClearReferrerOnHTTPSToHTTPRedirect = shouldClearReferrerOnHTTPSToHTTPRedirect;
    280     loadParameters.defersLoading = resourceLoader.defersLoading();
    281280    loadParameters.needsCertificateInfo = resourceLoader.shouldIncludeCertificateInfo();
    282281    loadParameters.maximumBufferingTime = maximumBufferingTime;
    … …  
    427426}
    428427
    429 void WebLoaderStrategy::setDefersLoading(ResourceLoader& resourceLoader, bool defers)
    430 {
    431     ResourceLoadIdentifier identifier = resourceLoader.identifier();
    432     WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::SetDefersLoading(identifier, defers), 0);
     428void WebLoaderStrategy::setDefersLoading(ResourceLoader&, bool)
     429{
     430    ASSERT_NOT_REACHED();
    433431}
    434432

  • trunk/Tools/ChangeLog

    r241306 r241317  
     12019-02-12  Alex Christensen  <achristensen@webkit.org>
     2
     3        Remove setDefersLoading infrastructure from WebKit2
     4        https://bugs.webkit.org/show_bug.cgi?id=194506
     5
     6        Reviewed by Brady Eidson.
     7
     8        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
     9        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
     10        (WTR::TestRunner::setDefersLoading): Deleted.
     11        * WebKitTestRunner/InjectedBundle/TestRunner.h:
     12
    1132019-02-12  Alex Christensen  <achristensen@webkit.org>
    214

  • trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl

    r240928 r241317  
    170170    void setShouldStayOnPageAfterHandlingBeforeUnload(boolean flag);
    171171
    172     void setDefersLoading(boolean flag);
    173172    void setStopProvisionalFrameLoads();
    174173

  • trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp

    r241127 r241317  
    742742}
    743743
    744 void TestRunner::setDefersLoading(bool)
    745 {
    746 }
    747 
    748744bool TestRunner::didReceiveServerRedirectForProvisionalNavigation() const
    749745{

  • trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h

    r240928 r241317  
    263263
    264264    void setShouldStayOnPageAfterHandlingBeforeUnload(bool);
    265 
    266     void setDefersLoading(bool);
    267265
    268266    void setStopProvisionalFrameLoads() { m_shouldStopProvisionalFrameLoads = true; }
Note: See TracChangeset for help on using the changeset viewer.