Changeset 48687 in webkit


Ignore:
Timestamp:
Sep 23, 2009 4:27:01 PM (15 years ago)
Author:
Darin Adler
Message:

Crash when website does a history.back() followed by an alert()
​https://bugs.webkit.org/show_bug.cgi?id=29686
rdar://problem/6984996

Patch by Darin Adler <Darin Adler> on 2009-09-23
Reviewed by Sam Weinig.

When loading is deferred, we need to defer timer-based loads
too, not just networking-driven loads. Otherwise we can get
syncronouse navigation while running a script, which leads to
crashes and other badness.

This patch includes a manual test; an automated test may be
possible some time in the future.

  • dom/Document.cpp:

(WebCore::Document::processHttpEquiv): Use scheduleLocationChange
instead of scheduleHTTPRedirection to implement the navigation
needed for x-frame-options.

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::FrameLoader): Updated for data members with
new names and new data members.
(WebCore::FrameLoader::setDefersLoading): When turning deferral
off, call startRedirectionTimer and startCheckCompleteTimer, since
either of them might have been fired and ignored while defersLoading
was true.
(WebCore::FrameLoader::clear): Updated for replacement of the
m_checkCompletedTimer and m_checkLoadCompleteTimer timers.
(WebCore::FrameLoader::allAncestorsAreComplete): Added.
(WebCore::FrameLoader::checkCompleted): Added code to set
m_shouldCallCheckCompleted to false. Changed code that calls
startRedirectionTimer to call it unconditionally, since that
function now knows when to do work and doesn't expect callers
to handle that any more.
(WebCore::FrameLoader::checkTimerFired): Added. Replaces the old
timer fired callbacks. Calls checkCompleted and checkLoadComplete
as appropriate, but not when defersLoading is true.
(WebCore::FrameLoader::startCheckCompleteTimer): Added. Replaces
the two different calls to start timers before. Only starts the
timers if they are needed.
(WebCore::FrameLoader::scheduleCheckCompleted): Changed to call
startCheckCompleteTimer after setting boolean.
(WebCore::FrameLoader::scheduleCheckLoadComplete): Ditto.
(WebCore::FrameLoader::scheduleHistoryNavigation): Removed
canGoBackOrForward check. The logic works more naturally when
we don't do anything until the timer fires.
(WebCore::FrameLoader::redirectionTimerFired): Do nothing if
defersLoading is true. Also moved canGoBackOrForward check here.
(WebCore::FrameLoader::scheduleRedirection): Changed code that
calls startRedirectionTimer to do so unconditionally. That
function now handles the rules about when to start the timer
rather than expecting the caller to do so.
(WebCore::FrameLoader::startRedirectionTimer): Added code to
handle the case where there is no redirection scheduled,
where the timer is already active, or where this is a classic
redirection and there is an ancestor that has not yet completed
loading.
(WebCore::FrameLoader::completed): Call startRedirectionTimer
here directly instead of calling a cover named parentCompleted.
Hooray! One less function in the giant FrameLoader class!
(WebCore::FrameLoader::checkLoadComplete): Added code to set
m_shouldCallCheckLoadComplete to false.

  • loader/FrameLoader.h: Replaced the two functions

checkCompletedTimerFired and checkLoadCompleteTimerFired with
one function, checkTimerFired. Removed the parentCompleted
function. Added the startCheckCompleteTimer and
allAncestorsAreComplete functions. Replaced the
m_checkCompletedTimer and m_checkLoadCompleteTimer data
members with m_checkTimer, m_shouldCallCheckCompleted, and
m_shouldCallCheckLoadComplete.

  • manual-tests/go-back-after-alert.html: Added.
  • manual-tests/resources/alert-and-go-back.html: Added.
Location:
trunk/WebCore
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r48686 r48687  
     12009-09-23  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Sam Weinig.
     4
     5        Crash when website does a history.back() followed by an alert()
     6        https://bugs.webkit.org/show_bug.cgi?id=29686
     7        rdar://problem/6984996
     8
     9        When loading is deferred, we need to defer timer-based loads
     10        too, not just networking-driven loads. Otherwise we can get
     11        syncronouse navigation while running a script, which leads to
     12        crashes and other badness.
     13
     14        This patch includes a manual test; an automated test may be
     15        possible some time in the future.
     16
     17        * dom/Document.cpp:
     18        (WebCore::Document::processHttpEquiv): Use scheduleLocationChange
     19        instead of scheduleHTTPRedirection to implement the navigation
     20        needed for x-frame-options.
     21
     22        * loader/FrameLoader.cpp:
     23        (WebCore::FrameLoader::FrameLoader): Updated for data members with
     24        new names and new data members.
     25        (WebCore::FrameLoader::setDefersLoading): When turning deferral
     26        off, call startRedirectionTimer and startCheckCompleteTimer, since
     27        either of them might have been fired and ignored while defersLoading
     28        was true.
     29        (WebCore::FrameLoader::clear): Updated for replacement of the
     30        m_checkCompletedTimer and m_checkLoadCompleteTimer timers.
     31        (WebCore::FrameLoader::allAncestorsAreComplete): Added.
     32        (WebCore::FrameLoader::checkCompleted): Added code to set
     33        m_shouldCallCheckCompleted to false. Changed code that calls
     34        startRedirectionTimer to call it unconditionally, since that
     35        function now knows when to do work and doesn't expect callers
     36        to handle that any more.
     37        (WebCore::FrameLoader::checkTimerFired): Added. Replaces the old
     38        timer fired callbacks. Calls checkCompleted and checkLoadComplete
     39        as appropriate, but not when defersLoading is true.
     40        (WebCore::FrameLoader::startCheckCompleteTimer): Added. Replaces
     41        the two different calls to start timers before. Only starts the
     42        timers if they are needed.
     43        (WebCore::FrameLoader::scheduleCheckCompleted): Changed to call
     44        startCheckCompleteTimer after setting boolean.
     45        (WebCore::FrameLoader::scheduleCheckLoadComplete): Ditto.
     46        (WebCore::FrameLoader::scheduleHistoryNavigation): Removed
     47        canGoBackOrForward check. The logic works more naturally when
     48        we don't do anything until the timer fires.
     49        (WebCore::FrameLoader::redirectionTimerFired): Do nothing if
     50        defersLoading is true. Also moved canGoBackOrForward check here.
     51        (WebCore::FrameLoader::scheduleRedirection): Changed code that
     52        calls startRedirectionTimer to do so unconditionally. That
     53        function now handles the rules about when to start the timer
     54        rather than expecting the caller to do so.
     55        (WebCore::FrameLoader::startRedirectionTimer): Added code to
     56        handle the case where there is no redirection scheduled,
     57        where the timer is already active, or where this is a classic
     58        redirection and there is an ancestor that has not yet completed
     59        loading.
     60        (WebCore::FrameLoader::completed): Call startRedirectionTimer
     61        here directly instead of calling a cover named parentCompleted.
     62        Hooray! One less function in the giant FrameLoader class!
     63        (WebCore::FrameLoader::checkLoadComplete): Added code to set
     64        m_shouldCallCheckLoadComplete to false.
     65
     66        * loader/FrameLoader.h: Replaced the two functions
     67        checkCompletedTimerFired and checkLoadCompleteTimerFired with
     68        one function, checkTimerFired. Removed the parentCompleted
     69        function. Added the startCheckCompleteTimer and
     70        allAncestorsAreComplete functions. Replaced the
     71        m_checkCompletedTimer and m_checkLoadCompleteTimer data
     72        members with m_checkTimer, m_shouldCallCheckCompleted, and
     73        m_shouldCallCheckLoadComplete.
     74
     75        * manual-tests/go-back-after-alert.html: Added.
     76        * manual-tests/resources/alert-and-go-back.html: Added.
     77
    1782009-09-23  David Kilzer  <ddkilzer@apple.com>
    279

  • trunk/WebCore/dom/Document.cpp

    r48559 r48687  
    21742174        if (frameLoader->shouldInterruptLoadForXFrameOptions(content, url())) {
    21752175            frameLoader->stopAllLoaders();
    2176             frameLoader->scheduleHTTPRedirection(0, blankURL());
     2176            frameLoader->scheduleLocationChange(blankURL(), String());
    21772177        }
    21782178    }

  • trunk/WebCore/loader/FrameLoader.cpp

    r48680 r48687  
    273273    , m_containsPlugIns(false)
    274274    , m_redirectionTimer(this, &FrameLoader::redirectionTimerFired)
    275     , m_checkCompletedTimer(this, &FrameLoader::checkCompletedTimerFired)
    276     , m_checkLoadCompleteTimer(this, &FrameLoader::checkLoadCompleteTimerFired)
     275    , m_checkTimer(this, &FrameLoader::checkTimerFired)
     276    , m_shouldCallCheckCompleted(false)
     277    , m_shouldCallCheckLoadComplete(false)
    277278    , m_opener(0)
    278279    , m_openedByDOM(false)
    … …  
    324325    if (m_policyDocumentLoader)
    325326        m_policyDocumentLoader->setDefersLoading(defers);
     327
     328    if (!defers) {
     329        startRedirectionTimer();
     330        startCheckCompleteTimer();
     331    }
    326332}
    327333
    … …  
    850856    m_scheduledRedirection.clear();
    851857
    852     m_checkCompletedTimer.stop();
    853     m_checkLoadCompleteTimer.stop();
     858    m_checkTimer.stop();
     859    m_shouldCallCheckCompleted = false;
     860    m_shouldCallCheckLoadComplete = false;
    854861
    855862    m_receivedData = false;
    … …  
    12531260}
    12541261
     1262bool FrameLoader::allAncestorsAreComplete() const
     1263{
     1264    for (Frame* ancestor = m_frame; ancestor; ancestor = ancestor->tree()->parent()) {
     1265        if (!ancestor->loader()->m_isComplete)
     1266            return false;
     1267    }
     1268    return true;
     1269}
     1270
    12551271void FrameLoader::checkCompleted()
    12561272{
     1273    m_shouldCallCheckCompleted = false;
     1274
    12571275    if (m_frame->view())
    12581276        m_frame->view()->checkStopDelayingDeferredRepaints();
    … …  
    12801298    checkCallImplicitClose(); // if we didn't do it before
    12811299
    1282     // Do not start a redirection timer for subframes here.
    1283     // That is deferred until the parent is completed.
    1284     if (m_scheduledRedirection && !m_frame->tree()->parent())
    1285         startRedirectionTimer();
     1300    startRedirectionTimer();
    12861301
    12871302    completed();
    … …  
    12901305}
    12911306
    1292 void FrameLoader::checkCompletedTimerFired(Timer<FrameLoader>*)
    1293 {
    1294     checkCompleted();
     1307void FrameLoader::checkTimerFired(Timer<FrameLoader>*)
     1308{
     1309    if (Page* page = m_frame->page()) {
     1310        if (page->defersLoading())
     1311            return;
     1312    }
     1313    if (m_shouldCallCheckCompleted)
     1314        checkCompleted();
     1315    if (m_shouldCallCheckLoadComplete)
     1316        checkLoadComplete();
     1317}
     1318
     1319void FrameLoader::startCheckCompleteTimer()
     1320{
     1321    if (!(m_shouldCallCheckCompleted || m_shouldCallCheckLoadComplete))
     1322        return;
     1323    if (m_checkTimer.isActive())
     1324        return;
     1325    m_checkTimer.startOneShot(0);
    12951326}
    12961327
    12971328void FrameLoader::scheduleCheckCompleted()
    12981329{
    1299     if (!m_checkCompletedTimer.isActive())
    1300         m_checkCompletedTimer.startOneShot(0);
    1301 }
    1302 
    1303 void FrameLoader::checkLoadCompleteTimerFired(Timer<FrameLoader>*)
    1304 {
    1305     if (!m_frame->page())
    1306         return;
    1307     checkLoadComplete();
     1330    m_shouldCallCheckCompleted = true;
     1331    startCheckCompleteTimer();
    13081332}
    13091333
    13101334void FrameLoader::scheduleCheckLoadComplete()
    13111335{
    1312     if (!m_checkLoadCompleteTimer.isActive())
    1313         m_checkLoadCompleteTimer.startOneShot(0);
     1336    m_shouldCallCheckLoadComplete = true;
     1337    startCheckCompleteTimer();
    13141338}
    13151339
    … …  
    14391463    if (!m_frame->page())
    14401464        return;
    1441 
    1442     // navigation will always be allowed in the 0 steps case, which is OK because that's supposed to force a reload.
    1443     if (!canGoBackOrForward(steps)) {
    1444         cancelRedirection();
    1445         return;
    1446     }
    14471465
    14481466    scheduleRedirection(new ScheduledRedirection(steps));
    … …  
    14821500{
    14831501    ASSERT(m_frame->page());
     1502
     1503    if (m_frame->page()->defersLoading())
     1504        return;
    14841505
    14851506    OwnPtr<ScheduledRedirection> redirection(m_scheduledRedirection.release());
    … …  
    14991520            // go(i!=0) from a frame navigates into the history of the frame only,
    15001521            // in both IE and NS (but not in Mozilla). We can't easily do that.
    1501             goBackOrForward(redirection->historySteps);
     1522            if (canGoBackOrForward(redirection->historySteps))
     1523                goBackOrForward(redirection->historySteps);
    15021524            return;
    15031525        case ScheduledRedirection::formSubmission:
    … …  
    17211743
    17221744    return widget != 0;
    1723 }
    1724 
    1725 void FrameLoader::parentCompleted()
    1726 {
    1727     if (m_scheduledRedirection && !m_redirectionTimer.isActive())
    1728         startRedirectionTimer();
    17291745}
    17301746
    … …  
    21402156    if (!m_isComplete && m_scheduledRedirection->type != ScheduledRedirection::redirection)
    21412157        completed();
    2142     if (m_isComplete || m_scheduledRedirection->type != ScheduledRedirection::redirection)
    2143         startRedirectionTimer();
     2158    startRedirectionTimer();
    21442159}
    21452160
    21462161void FrameLoader::startRedirectionTimer()
    21472162{
     2163    if (!m_scheduledRedirection)
     2164        return;
     2165
    21482166    ASSERT(m_frame->page());
    2149     ASSERT(m_scheduledRedirection);
    2150 
    2151     m_redirectionTimer.stop();
     2167
     2168    if (m_redirectionTimer.isActive())
     2169        return;
     2170
     2171    if (m_scheduledRedirection->type == ScheduledRedirection::redirection && !allAncestorsAreComplete())
     2172        return;
     2173
    21522174    m_redirectionTimer.startOneShot(m_scheduledRedirection->delay);
    21532175
    … …  
    21882210    RefPtr<Frame> protect(m_frame);
    21892211    for (Frame* child = m_frame->tree()->firstChild(); child; child = child->tree()->nextSibling())
    2190         child->loader()->parentCompleted();
     2212        child->loader()->startRedirectionTimer();
    21912213    if (Frame* parent = m_frame->tree()->parent())
    21922214        parent->loader()->checkCompleted();
    … …  
    35603582    ASSERT(m_client->hasWebView());
    35613583   
     3584    m_shouldCallCheckLoadComplete = false;
     3585
    35623586    // FIXME: Always traversing the entire frame tree is a bit inefficient, but
    35633587    // is currently needed in order to null out the previous history item for all frames.

  • trunk/WebCore/loader/FrameLoader.h

    r48661 r48687  
    413413   
    414414        void redirectionTimerFired(Timer<FrameLoader>*);
    415         void checkCompletedTimerFired(Timer<FrameLoader>*);
    416         void checkLoadCompleteTimerFired(Timer<FrameLoader>*);
     415        void checkTimerFired(Timer<FrameLoader>*);
    417416       
    418417        void cancelRedirection(bool newLoadInProgress = false);
    … …  
    421420
    422421        void completed();
    423         void parentCompleted();
    424422
    425423        bool shouldUsePlugin(const KURL&, const String& mimeType, bool hasFallback, bool& useFallback);
    … …  
    540538        void scheduleCheckCompleted();
    541539        void scheduleCheckLoadComplete();
     540        void startCheckCompleteTimer();
    542541
    543542        KURL originalRequestURL() const;
    … …  
    547546        void saveScrollPositionAndViewStateToItem(HistoryItem*);
    548547
     548        bool allAncestorsAreComplete() const; // including this
    549549        bool allChildrenAreComplete() const; // immediate children, not all descendants
    550550
    … …  
    613613   
    614614        Timer<FrameLoader> m_redirectionTimer;
    615         Timer<FrameLoader> m_checkCompletedTimer;
    616         Timer<FrameLoader> m_checkLoadCompleteTimer;
     615        Timer<FrameLoader> m_checkTimer;
     616        bool m_shouldCallCheckCompleted;
     617        bool m_shouldCallCheckLoadComplete;
    617618
    618619        Frame* m_opener;
Note: See TracChangeset for help on using the changeset viewer.