⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 181480 in webkit


Ignore:
Timestamp:
Mar 13, 2015, 10:58:50 AM (11 years ago)
Author:
Chris Dumez
Message:

XMLHttpRequests should not prevent a page from entering PageCache
​https://bugs.webkit.org/show_bug.cgi?id=142612
<rdar://problem/19923085>

Reviewed by Alexey Proskuryakov.

Source/WebCore:

Make XMLHttpRequest ActiveDOMObjects suspendable in most cases to
drastically improve the likelihood of pages using them to enter
PageCache. XMLHttpRequest used to be only suspendable when not
loading. After this patch, if the XMLHttpRequest is loading when
navigating away from the page, it will be aborted and the page
will enter the PageCache. Upon restoring the page from PageCache,
the XMLHttpRequests' error handlers will be executed to give them
a chance to reload if they want to.

Test: http/tests/navigation/page-cache-xhr.html

  • history/PageCache.cpp:

(WebCore::logCanCacheFrameDecision):
(WebCore::PageCache::canCachePageContainingThisFrame):
Do not prevent a page to enter the page cache ff the main document has
an error that is a cancellation and all remaining subresource loaders
are for XHR. We extend the pre-existing mechanism used on iOS, which
allowed PageCaching if the remaining resource loads are for images.

  • loader/DocumentLoader.cpp:

(WebCore::areAllLoadersPageCacheAcceptable):
Mark XHR loaders as PageCache acceptable.

  • loader/DocumentThreadableLoader.cpp:

(WebCore::DocumentThreadableLoader::isXMLHttpRequest):

  • loader/DocumentThreadableLoader.h:
  • loader/ThreadableLoader.h:
  • loader/cache/CachedResource.cpp:

(WebCore::CachedResource::areAllClientsXMLHttpRequests):

  • loader/cache/CachedResource.h:
  • loader/cache/CachedResourceClient.h:

(WebCore::CachedResourceClient::isXMLHttpRequest):

  • xml/XMLHttpRequest.cpp:

(WebCore::XMLHttpRequest::XMLHttpRequest):
(WebCore::XMLHttpRequest::createRequest):
(WebCore::XMLHttpRequest::canSuspend):
Report that we can suspend XMLHttpRequests as long as the window load
event has already fired. If the window load event has not fired yet,
it would be unsafe to cancel the load in suspend() as it would
potentially cause arbitrary JS execution while suspending.

(WebCore::XMLHttpRequest::suspend):
If suspending for PageCache and the request is currently loading, abort
the load and mark that we should fire the error event upon restoring
from PageCache.

(WebCore::XMLHttpRequest::resume):
(WebCore::XMLHttpRequest::resumeTimerFired):
Upon resuming, fire the error event in a timer if the load was aborted
for suspending. We need to do this in a timer because we are not allowed
to execute arbitrary JS inside resume().

(WebCore::XMLHttpRequest::stop):
Add a assertion to make sure we are not firing event inside stop() as
this would potentially cause arbitrary JS execution and it would be
unsafe. It seems to me that our code is currently unsafe but the
assertion does not seem to be hit by our current layout tests. I am
adding the assertion as it would make it clear we have a bug and we
need to fix it.

  • xml/XMLHttpRequest.h:

LayoutTests:

Add a tests to make sure that loading XMLHttpRequests do not prevent a
page from entering PageCache.

  • http/tests/navigation/page-cache-xhr-expected.txt: Added.
  • http/tests/navigation/page-cache-xhr.html: Added.
  • http/tests/navigation/resources/page-cache-helper.html: Added.
Location:
trunk
Files:
3 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r181478 r181480  
     12015-03-13  Chris Dumez  <cdumez@apple.com>
     2
     3        XMLHttpRequests should not prevent a page from entering PageCache
     4        https://bugs.webkit.org/show_bug.cgi?id=142612
     5        <rdar://problem/19923085>
     6
     7        Reviewed by Alexey Proskuryakov.
     8
     9        Add a tests to make sure that loading XMLHttpRequests do not prevent a
     10        page from entering PageCache.
     11
     12        * http/tests/navigation/page-cache-xhr-expected.txt: Added.
     13        * http/tests/navigation/page-cache-xhr.html: Added.
     14        * http/tests/navigation/resources/page-cache-helper.html: Added.
     15
    1162015-03-13  Marcos Chavarría Teijeiro  <chavarria1991@gmail.com>
    217

  • trunk/Source/WebCore/ChangeLog

    r181475 r181480  
     12015-03-13  Chris Dumez  <cdumez@apple.com>
     2
     3        XMLHttpRequests should not prevent a page from entering PageCache
     4        https://bugs.webkit.org/show_bug.cgi?id=142612
     5        <rdar://problem/19923085>
     6
     7        Reviewed by Alexey Proskuryakov.
     8
     9        Make XMLHttpRequest ActiveDOMObjects suspendable in most cases to
     10        drastically improve the likelihood of pages using them to enter
     11        PageCache. XMLHttpRequest used to be only suspendable when not
     12        loading. After this patch, if the XMLHttpRequest is loading when
     13        navigating away from the page, it will be aborted and the page
     14        will enter the PageCache. Upon restoring the page from PageCache,
     15        the XMLHttpRequests' error handlers will be executed to give them
     16        a chance to reload if they want to.
     17
     18        Test: http/tests/navigation/page-cache-xhr.html
     19
     20        * history/PageCache.cpp:
     21        (WebCore::logCanCacheFrameDecision):
     22        (WebCore::PageCache::canCachePageContainingThisFrame):
     23        Do not prevent a page to enter the page cache ff the main document has
     24        an error that is a cancellation and all remaining subresource loaders
     25        are for XHR. We extend the pre-existing mechanism used on iOS, which
     26        allowed PageCaching if the remaining resource loads are for images.
     27
     28        * loader/DocumentLoader.cpp:
     29        (WebCore::areAllLoadersPageCacheAcceptable):
     30        Mark XHR loaders as PageCache acceptable.
     31
     32        * loader/DocumentThreadableLoader.cpp:
     33        (WebCore::DocumentThreadableLoader::isXMLHttpRequest):
     34        * loader/DocumentThreadableLoader.h:
     35        * loader/ThreadableLoader.h:
     36        * loader/cache/CachedResource.cpp:
     37        (WebCore::CachedResource::areAllClientsXMLHttpRequests):
     38        * loader/cache/CachedResource.h:
     39        * loader/cache/CachedResourceClient.h:
     40        (WebCore::CachedResourceClient::isXMLHttpRequest):
     41        * xml/XMLHttpRequest.cpp:
     42        (WebCore::XMLHttpRequest::XMLHttpRequest):
     43        (WebCore::XMLHttpRequest::createRequest):
     44        (WebCore::XMLHttpRequest::canSuspend):
     45        Report that we can suspend XMLHttpRequests as long as the window load
     46        event has already fired. If the window load event has not fired yet,
     47        it would be unsafe to cancel the load in suspend() as it would
     48        potentially cause arbitrary JS execution while suspending.
     49
     50        (WebCore::XMLHttpRequest::suspend):
     51        If suspending for PageCache and the request is currently loading, abort
     52        the load and mark that we should fire the error event upon restoring
     53        from PageCache.
     54
     55        (WebCore::XMLHttpRequest::resume):
     56        (WebCore::XMLHttpRequest::resumeTimerFired):
     57        Upon resuming, fire the error event in a timer if the load was aborted
     58        for suspending. We need to do this in a timer because we are not allowed
     59        to execute arbitrary JS inside resume().
     60
     61        (WebCore::XMLHttpRequest::stop):
     62        Add a assertion to make sure we are not firing event inside stop() as
     63        this would potentially cause arbitrary JS execution and it would be
     64        unsafe. It seems to me that our code is currently unsafe but the
     65        assertion does not seem to be hit by our current layout tests. I am
     66        adding the assertion as it would make it clear we have a bug and we
     67        need to fix it.
     68
     69        * xml/XMLHttpRequest.h:
     70
    1712015-03-13  Joonghun Park  <jh718.park@samsung.com>
    272

  • trunk/Source/WebCore/history/PageCache.cpp

    r180770 r181480  
    129129        PCLOG("   -Main document has an error");
    130130        logPageCacheFailureDiagnosticMessage(diagnosticLoggingClient, DiagnosticLoggingKeys::mainDocumentErrorKey());
    131 #if !PLATFORM(IOS)
    132         rejectReasons |= 1 << MainDocumentError;
    133 #else
     131
    134132        if (frame.loader().documentLoader()->mainDocumentError().isCancellation() && frame.loader().documentLoader()->subresourceLoadersArePageCacheAcceptable())
    135             PCLOG("    -But, it was a cancellation and all loaders during the cancel were loading images.");
     133            PCLOG("    -But, it was a cancellation and all loaders during the cancelation were loading images or XHR.");
    136134        else
    137135            rejectReasons |= 1 << MainDocumentError;
    138 #endif
    139136    }
    140137    if (frame.loader().documentLoader()->substituteData().isValid() && frame.loader().documentLoader()->substituteData().failingURL().isEmpty()) {
    … …  
    305302   
    306303    return documentLoader
    307 #if !PLATFORM(IOS)
    308         && documentLoader->mainDocumentError().isNull()
    309 #else
    310304        && (documentLoader->mainDocumentError().isNull() || (documentLoader->mainDocumentError().isCancellation() && documentLoader->subresourceLoadersArePageCacheAcceptable()))
    311 #endif
    312305        // Do not cache error pages (these can be recognized as pages with substitute data or unreachable URLs).
    313306        && !(documentLoader->substituteData().isValid() && !documentLoader->substituteData().failingURL().isEmpty())

  • trunk/Source/WebCore/loader/DocumentLoader.cpp

    r181409 r181480  
    100100    copyValuesToVector(loaders, loadersCopy);
    101101    for (auto& loader : loadersCopy) {
    102         ResourceHandle* handle = loader->handle();
    103         if (!handle)
    104             return false;
    105 
    106102        if (!loader->frameLoader())
    107103            return false;
    108104
    109         CachedResource* cachedResource = MemoryCache::singleton().resourceForURL(handle->firstRequest().url(), loader->frameLoader()->frame().page()->sessionID());
     105        CachedResource* cachedResource = MemoryCache::singleton().resourceForURL(loader->request().url(), loader->frameLoader()->frame().page()->sessionID());
    110106        if (!cachedResource)
    111107            return false;
    112108
     109        // Only image and XHR loads do prevent the page from entering the PageCache.
    113110        // All non-image loads will prevent the page from entering the PageCache.
    114         if (!cachedResource->isImage())
     111        if (!cachedResource->isImage() && !cachedResource->areAllClientsXMLHttpRequests())
    115112            return false;
    116113    }

  • trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp

    r180132 r181480  
    3535#include "CachedResourceLoader.h"
    3636#include "CachedResourceRequest.h"
     37#include "CachedResourceRequestInitiators.h"
    3738#include "CrossOriginAccessControl.h"
    3839#include "CrossOriginPreflightResultCache.h"
    … …  
    419420}
    420421
     422bool DocumentThreadableLoader::isXMLHttpRequest() const
     423{
     424    return m_options.initiator == cachedResourceRequestInitiators().xmlhttprequest;
     425}
     426
    421427SecurityOrigin* DocumentThreadableLoader::securityOrigin() const
    422428{

  • trunk/Source/WebCore/loader/DocumentThreadableLoader.h

    r176172 r181480  
    9797        bool isAllowedRedirect(const URL&);
    9898
     99        bool isXMLHttpRequest() const override final;
     100
    99101        SecurityOrigin* securityOrigin() const;
    100102

  • trunk/Source/WebCore/loader/ThreadableLoader.h

    r149876 r181480  
    3737#include <wtf/RefPtr.h>
    3838#include <wtf/Vector.h>
    39 
    40 #if ENABLE(RESOURCE_TIMING)
    4139#include <wtf/text/AtomicString.h>
    42 #endif
    4340
    4441namespace WebCore {
    … …  
    7067        CrossOriginRequestPolicy crossOriginRequestPolicy;
    7168        RefPtr<SecurityOrigin> securityOrigin;
    72 #if ENABLE(RESOURCE_TIMING)
    7369        AtomicString initiator;
    74 #endif
    7570    };
    7671

  • trunk/Source/WebCore/loader/cache/CachedResource.cpp

    r180517 r181480  
    736736}
    737737
     738bool CachedResource::areAllClientsXMLHttpRequests() const
     739{
     740    if (type() != RawResource)
     741        return false;
     742
     743    for (auto& client : m_clients) {
     744        if (!client.key->isXMLHttpRequest())
     745            return false;
     746    }
     747    return true;
     748}
     749
    738750void CachedResource::setLoadPriority(const Optional<ResourceLoadPriority>& loadPriority)
    739751{

  • trunk/Source/WebCore/loader/cache/CachedResource.h

    r180207 r181480  
    159159    SubresourceLoader* loader() { return m_loader.get(); }
    160160
     161    bool areAllClientsXMLHttpRequests() const;
     162
    161163    bool isImage() const { return type() == ImageResource; }
    162164    // FIXME: CachedRawResource could be either a main resource or a raw XHR resource.

  • trunk/Source/WebCore/loader/cache/CachedResourceClient.h

    r175549 r181480  
    4444    virtual void notifyFinished(CachedResource*) { }
    4545    virtual void deprecatedDidReceiveCachedResource(CachedResource*) { }
     46    virtual bool isXMLHttpRequest() const { return false; }
    4647
    4748    static CachedResourceClientType expectedType() { return BaseResourceType; }

  • trunk/Source/WebCore/xml/XMLHttpRequest.cpp

    r181411 r181480  
    2525
    2626#include "Blob.h"
     27#include "CachedResourceRequestInitiators.h"
    2728#include "ContentSecurityPolicy.h"
    2829#include "CrossOriginAccessControl.h"
    … …  
    6566#include <wtf/text/CString.h>
    6667
    67 #if ENABLE(RESOURCE_TIMING)
    68 #include "CachedResourceRequestInitiators.h"
    69 #endif
    70 
    7168namespace WebCore {
    7269
    … …  
    140137    , m_responseTypeCode(ResponseTypeDefault)
    141138    , m_responseCacheIsValid(false)
     139    , m_resumeTimer(*this, &XMLHttpRequest::resumeTimerFired)
     140    , m_dispatchErrorOnResuming(false)
    142141{
    143142#ifndef NDEBUG
    … …  
    765764    options.crossOriginRequestPolicy = UseAccessControl;
    766765    options.securityOrigin = securityOrigin();
    767 #if ENABLE(RESOURCE_TIMING)
    768766    options.initiator = cachedResourceRequestInitiators().xmlhttprequest;
    769 #endif
    770767
    771768#if ENABLE(XHR_TIMEOUT)
    … …  
    12561253bool XMLHttpRequest::canSuspend() const
    12571254{
    1258     return !m_loader;
     1255    // If the load event has not fired yet, cancelling the load in suspend() may cause
     1256    // the load event to be fired and arbitrary JS execution, which would be unsafe.
     1257    // Therefore, we prevent suspending in this case.
     1258    return document()->loadEventFinished();
    12591259}
    12601260
    … …  
    12641264}
    12651265
    1266 void XMLHttpRequest::suspend(ReasonForSuspension)
    1267 {
     1266void XMLHttpRequest::suspend(ReasonForSuspension reason)
     1267{
     1268    NoEventDispatchAssertion assertNoEventDispatch;
     1269
    12681270    m_progressEventThrottle.suspend();
     1271
     1272    if (m_resumeTimer.isActive()) {
     1273        m_resumeTimer.stop();
     1274        m_dispatchErrorOnResuming = true;
     1275    }
     1276
     1277    if (reason == ActiveDOMObject::DocumentWillBecomeInactive && m_loader) {
     1278        // Going into PageCache, abort the request and dispatch a network error on resuming.
     1279        genericError();
     1280        m_dispatchErrorOnResuming = true;
     1281        bool aborted = internalAbort();
     1282        // It should not be possible to restart the load when aborting in suspend() because
     1283        // we are not allowed to execute in JS in suspend().
     1284        ASSERT_UNUSED(aborted, aborted);
     1285    }
    12691286}
    12701287
    12711288void XMLHttpRequest::resume()
    12721289{
     1290    NoEventDispatchAssertion assertNoEventDispatch;
     1291
    12731292    m_progressEventThrottle.resume();
     1293
     1294    // We are not allowed to execute arbitrary JS in resume() so dispatch
     1295    // the error event in a timer.
     1296    if (m_dispatchErrorOnResuming && !m_resumeTimer.isActive())
     1297        m_resumeTimer.startOneShot(0);
     1298}
     1299
     1300void XMLHttpRequest::resumeTimerFired()
     1301{
     1302    ASSERT(m_dispatchErrorOnResuming);
     1303    m_dispatchErrorOnResuming = false;
     1304    dispatchErrorEvents(eventNames().errorEvent);
    12741305}
    12751306
    12761307void XMLHttpRequest::stop()
    12771308{
     1309    NoEventDispatchAssertion assertNoEventDispatch;
    12781310    internalAbort();
    12791311}

  • trunk/Source/WebCore/xml/XMLHttpRequest.h

    r181169 r181480  
    206206    void dispatchErrorEvents(const AtomicString&);
    207207
     208    void resumeTimerFired();
     209
    208210    std::unique_ptr<XMLHttpRequestUpload> m_upload;
    209211
    … …  
    255257    ResponseTypeCode m_responseTypeCode;
    256258    bool m_responseCacheIsValid;
     259
     260    Timer m_resumeTimer;
     261    bool m_dispatchErrorOnResuming;
    257262};
    258263
Note: See TracChangeset for help on using the changeset viewer.