Ignore:
Timestamp:
Oct 21, 2019, 8:17:19 AM (6 years ago)
Author:
Chris Dumez
Message:

XMLHttpRequest should not prevent entering the back/forward cache
​https://bugs.webkit.org/show_bug.cgi?id=203107
<rdar://problem/56438647>

Reviewed by Youenn Fablet.

LayoutTests/imported/w3c:

Rebaseline a new WPT tests that are passing now that we properly check that the
Document is fully active in open().

  • web-platform-tests/xhr/open-url-multi-window-2-expected.txt:
  • web-platform-tests/xhr/open-url-multi-window-5-expected.txt:
  • web-platform-tests/xhr/open-url-multi-window-6-expected.txt:

Source/WebCore:

Improve XMLHttpRequest for back/forward cache suspension:

  1. We no longer cancel pending loads in the suspend() method as this may fire events.
  2. Simplify XMLHttpRequestProgressEventThrottle to use SuspendableTimers to dispatch events that are deferred by suspension or throttling.

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

  • xml/XMLHttpRequest.cpp:

(WebCore::XMLHttpRequest::XMLHttpRequest):
(WebCore::XMLHttpRequest::open):
Add check to throw a InvalidStateError if the associated document is not fully active,
as per ​https://xhr.spec.whatwg.org/#dom-xmlhttprequest-open (Step 2). This avoids
dispatching events after ActiveDOMObject::stop() has been called and brings a few more
passes on WPT tests.

(WebCore::XMLHttpRequest::dispatchEvent):
(WebCore::XMLHttpRequest::suspend):
(WebCore::XMLHttpRequest::resume):
(WebCore::XMLHttpRequest::shouldPreventEnteringBackForwardCache_DEPRECATED const): Deleted.
(WebCore::XMLHttpRequest::resumeTimerFired): Deleted.

  • xml/XMLHttpRequest.h:
  • xml/XMLHttpRequestProgressEventThrottle.cpp:

(WebCore::XMLHttpRequestProgressEventThrottle::XMLHttpRequestProgressEventThrottle):
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchThrottledProgressEvent):
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchReadyStateChangeEvent):
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchEventWhenPossible):
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchProgressEvent):
(WebCore::XMLHttpRequestProgressEventThrottle::flushProgressEvent):
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchDeferredEventsAfterResuming):
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchThrottledProgressEventTimerFired):
(WebCore::XMLHttpRequestProgressEventThrottle::suspend):
(WebCore::XMLHttpRequestProgressEventThrottle::resume):
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchEvent): Deleted.
(WebCore::XMLHttpRequestProgressEventThrottle::dispatchDeferredEvents): Deleted.
(WebCore::XMLHttpRequestProgressEventThrottle::fired): Deleted.
(WebCore::XMLHttpRequestProgressEventThrottle::hasEventToDispatch const): Deleted.

  • xml/XMLHttpRequestProgressEventThrottle.h:

LayoutTests:

Add more test coverage.

  • TestExpectations:
  • fast/dom/xmlhttprequest-constructor-in-detached-document-expected.txt:
  • fast/xmlhttprequest/xmlhttprequest-open-after-iframe-onload-remove-self.html:
  • http/tests/navigation/page-cache-xhr-in-loading-iframe-expected.txt: Added.
  • http/tests/navigation/page-cache-xhr-in-loading-iframe.html: Added.
  • http/tests/navigation/resources/page-cache-xhr-in-loading-iframe.html: Added.
File:
1 edited

Legend:

Unmodified
Added
Removed

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

    r246490 r251366  
    3636const Seconds XMLHttpRequestProgressEventThrottle::minimumProgressEventDispatchingInterval { 50_ms }; // 50 ms per specification.
    3737
    38 XMLHttpRequestProgressEventThrottle::XMLHttpRequestProgressEventThrottle(EventTarget* target)
     38XMLHttpRequestProgressEventThrottle::XMLHttpRequestProgressEventThrottle(EventTarget& target)
    3939    : m_target(target)
    40     , m_dispatchDeferredEventsTimer(*this, &XMLHttpRequestProgressEventThrottle::dispatchDeferredEvents)
     40    , m_dispatchThrottledProgressEventTimer(target.scriptExecutionContext(), *this, &XMLHttpRequestProgressEventThrottle::dispatchThrottledProgressEventTimerFired)
     41    , m_dispatchDeferredEventsAfterResumingTimer(target.scriptExecutionContext(), *this, &XMLHttpRequestProgressEventThrottle::dispatchDeferredEventsAfterResuming)
    4142{
    42     ASSERT(target);
     43    m_dispatchThrottledProgressEventTimer.suspendIfNeeded();
     44    m_dispatchDeferredEventsAfterResumingTimer.suspendIfNeeded();
    4345}
    4446
    … …  
    5153    m_total = total;
    5254
    53     if (!m_target->hasEventListeners(eventNames().progressEvent))
     55    if (!m_target.hasEventListeners(eventNames().progressEvent))
    5456        return;
    55    
    56     if (m_deferEvents) {
    57         // Only store the latest progress event while suspended.
    58         m_deferredProgressEvent = XMLHttpRequestProgressEvent::create(eventNames().progressEvent, lengthComputable, loaded, total);
    59         return;
    60     }
    6157
    62     if (!isActive()) {
     58    if (!m_shouldDeferEventsDueToSuspension && !m_dispatchThrottledProgressEventTimer.isActive()) {
    6359        // The timer is not active so the least frequent event for now is every byte. Just dispatch the event.
    6460
    6561        // We should not have any throttled progress event.
    66         ASSERT(!m_hasThrottledProgressEvent);
     62        ASSERT(!m_hasPendingThrottledProgressEvent);
    6763
    68         dispatchEvent(XMLHttpRequestProgressEvent::create(eventNames().progressEvent, lengthComputable, loaded, total));
    69         startRepeating(minimumProgressEventDispatchingInterval);
    70         m_hasThrottledProgressEvent = false;
     64        dispatchEventWhenPossible(XMLHttpRequestProgressEvent::create(eventNames().progressEvent, lengthComputable, loaded, total));
     65        m_dispatchThrottledProgressEventTimer.startRepeating(minimumProgressEventDispatchingInterval);
     66        m_hasPendingThrottledProgressEvent = false;
    7167        return;
    7268    }
    7369
    7470    // The timer is already active so minimumProgressEventDispatchingInterval is the least frequent event.
    75     m_hasThrottledProgressEvent = true;
     71    m_hasPendingThrottledProgressEvent = true;
    7672}
    7773
    … …  
    8177        flushProgressEvent();
    8278
    83     dispatchEvent(event);
     79    dispatchEventWhenPossible(event);
    8480}
    8581
    86 void XMLHttpRequestProgressEventThrottle::dispatchEvent(Event& event)
     82void XMLHttpRequestProgressEventThrottle::dispatchEventWhenPossible(Event& event)
    8783{
    88     if (m_deferEvents) {
    89         if (m_deferredEvents.size() > 1 && event.type() == eventNames().readystatechangeEvent && event.type() == m_deferredEvents.last()->type()) {
     84    if (m_shouldDeferEventsDueToSuspension) {
     85        if (m_eventsDeferredDueToSuspension.size() > 1 && event.type() == eventNames().readystatechangeEvent && event.type() == m_eventsDeferredDueToSuspension.last()->type()) {
    9086            // Readystatechange events are state-less so avoid repeating two identical events in a row on resume.
    9187            return;
    9288        }
    93         m_deferredEvents.append(event);
     89        m_eventsDeferredDueToSuspension.append(event);
    9490    } else
    95         m_target->dispatchEvent(event);
     91        m_target.dispatchEvent(event);
    9692}
    9793
    … …  
    106102    }
    107103
    108     if (m_target->hasEventListeners(type))
    109         dispatchEvent(XMLHttpRequestProgressEvent::create(type, m_lengthComputable, m_loaded, m_total));
     104    if (m_target.hasEventListeners(type))
     105        dispatchEventWhenPossible(XMLHttpRequestProgressEvent::create(type, m_lengthComputable, m_loaded, m_total));
    110106}
    111107
    112108void XMLHttpRequestProgressEventThrottle::flushProgressEvent()
    113109{
    114     if (m_deferEvents && m_deferredProgressEvent) {
    115         // Move the progress event to the queue, to get it in the right order on resume.
    116         m_deferredEvents.append(m_deferredProgressEvent.releaseNonNull());
     110    if (!m_hasPendingThrottledProgressEvent)
     111        return;
     112
     113    m_hasPendingThrottledProgressEvent = false;
     114    // We stop the timer as this is called when no more events are supposed to occur.
     115    m_dispatchThrottledProgressEventTimer.cancel();
     116
     117    dispatchEventWhenPossible(XMLHttpRequestProgressEvent::create(eventNames().progressEvent, m_lengthComputable, m_loaded, m_total));
     118}
     119
     120void XMLHttpRequestProgressEventThrottle::dispatchDeferredEventsAfterResuming()
     121{
     122    ASSERT(m_shouldDeferEventsDueToSuspension);
     123    m_shouldDeferEventsDueToSuspension = false;
     124
     125    // Take over the deferred events before dispatching them which can potentially add more.
     126    auto eventsDeferredDueToSuspension = WTFMove(m_eventsDeferredDueToSuspension);
     127
     128    flushProgressEvent();
     129
     130    for (auto& deferredEvent : eventsDeferredDueToSuspension)
     131        dispatchEventWhenPossible(deferredEvent);
     132}
     133
     134void XMLHttpRequestProgressEventThrottle::dispatchThrottledProgressEventTimerFired()
     135{
     136    ASSERT(m_dispatchThrottledProgressEventTimer.isActive());
     137    if (!m_hasPendingThrottledProgressEvent) {
     138        // No progress event was queued since the previous dispatch, we can safely stop the timer.
     139        m_dispatchThrottledProgressEventTimer.cancel();
    117140        return;
    118141    }
    119142
    120     if (!hasEventToDispatch())
    121         return;
    122     Ref<Event> event = XMLHttpRequestProgressEvent::create(eventNames().progressEvent, m_lengthComputable, m_loaded, m_total);
    123     m_hasThrottledProgressEvent = false;
    124 
    125     // We stop the timer as this is called when no more events are supposed to occur.
    126     stop();
    127 
    128     dispatchEvent(WTFMove(event));
    129 }
    130 
    131 void XMLHttpRequestProgressEventThrottle::dispatchDeferredEvents()
    132 {
    133     ASSERT(m_deferEvents);
    134     m_deferEvents = false;
    135 
    136     // Take over the deferred events before dispatching them which can potentially add more.
    137     auto deferredEvents = WTFMove(m_deferredEvents);
    138 
    139     RefPtr<Event> deferredProgressEvent = WTFMove(m_deferredProgressEvent);
    140 
    141     for (auto& deferredEvent : deferredEvents)
    142         dispatchEvent(deferredEvent);
    143 
    144     // The progress event will be in the m_deferredEvents vector if the load was finished while suspended.
    145     // If not, just send the most up-to-date progress on resume.
    146     if (deferredProgressEvent)
    147         dispatchEvent(*deferredProgressEvent);
    148 }
    149 
    150 void XMLHttpRequestProgressEventThrottle::fired()
    151 {
    152     ASSERT(isActive());
    153     if (!hasEventToDispatch()) {
    154         // No progress event was queued since the previous dispatch, we can safely stop the timer.
    155         stop();
    156         return;
    157     }
    158 
    159     dispatchEvent(XMLHttpRequestProgressEvent::create(eventNames().progressEvent, m_lengthComputable, m_loaded, m_total));
    160     m_hasThrottledProgressEvent = false;
    161 }
    162 
    163 bool XMLHttpRequestProgressEventThrottle::hasEventToDispatch() const
    164 {
    165     return m_hasThrottledProgressEvent && isActive();
     143    dispatchEventWhenPossible(XMLHttpRequestProgressEvent::create(eventNames().progressEvent, m_lengthComputable, m_loaded, m_total));
     144    m_hasPendingThrottledProgressEvent = false;
    166145}
    167146
    168147void XMLHttpRequestProgressEventThrottle::suspend()
    169148{
    170     // If re-suspended before deferred events have been dispatched, just stop the dispatch
    171     // and continue the last suspend.
    172     if (m_dispatchDeferredEventsTimer.isActive()) {
    173         ASSERT(m_deferEvents);
    174         m_dispatchDeferredEventsTimer.stop();
    175         return;
    176     }
    177     ASSERT(!m_deferredProgressEvent);
    178     ASSERT(m_deferredEvents.isEmpty());
    179     ASSERT(!m_deferEvents);
    180 
    181     m_deferEvents = true;
    182     // If we have a progress event waiting to be dispatched,
    183     // just defer it.
    184     if (hasEventToDispatch()) {
    185         m_deferredProgressEvent = XMLHttpRequestProgressEvent::create(eventNames().progressEvent, m_lengthComputable, m_loaded, m_total);
    186         m_hasThrottledProgressEvent = false;
    187     }
    188     stop();
     149    m_shouldDeferEventsDueToSuspension = true;
    189150}
    190151
    191152void XMLHttpRequestProgressEventThrottle::resume()
    192153{
    193     ASSERT(!m_hasThrottledProgressEvent);
    194 
    195     if (m_deferredEvents.isEmpty() && !m_deferredProgressEvent) {
    196         m_deferEvents = false;
     154    if (m_eventsDeferredDueToSuspension.isEmpty() && !m_hasPendingThrottledProgressEvent) {
     155        m_shouldDeferEventsDueToSuspension = false;
    197156        return;
    198157    }
    199158
    200     // Do not dispatch events inline here, since ScriptExecutionContext is iterating over
    201     // the list of active DOM objects to resume them, and any activated JS event-handler
    202     // could insert new active DOM objects to the list.
    203     // m_deferEvents is kept true until all deferred events have been dispatched.
    204     m_dispatchDeferredEventsTimer.startOneShot(0_s);
     159    m_dispatchDeferredEventsAfterResumingTimer.startOneShot(0_s);
    205160}
    206161
Note: See TracChangeset for help on using the changeset viewer.