Changeset 112144 in webkit


Ignore:
Timestamp:
Mar 26, 2012 1:34:00 PM (12 years ago)
Author:
Nate Chapin
Message:

Simplify setting loading state in DocumentLoader
​https://bugs.webkit.org/show_bug.cgi?id=82099

Reviewed by Adam Barth.

The logic for deciding what to return for DocumentLoader::isLoading()
is crazy. It's indirectly based on the ResourceLoaders that have
registered themselves with the DocumentLoader, but we can make that
relationship more direct.

  • loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::checkLoadComplete): Renamed from updateLoading, since it's not actually

updating anything anymore. It now calls DOMWindow::finishedLoading() if loading is in fact done.

(WebCore::DocumentLoader::startLoadingMainResource): The only reason this had a return value was to call

updateLoading() if loading failed. Just call checkLoadComplete() directly (this allows it to
be private, whereas updateLoading() was public).

(WebCore::DocumentLoader::subresourceLoaderFinishedLoadingOnePart):

  • loader/DocumentLoader.h:

(WebCore::DocumentLoader::isLoading): Rather than holding a separate bool, return based on the presence

of non-multipart ResourceLoaders directly.

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::isLoading): Depend on DocumentLoader::isLoading() for the activeDocumentLoader(),

rather than indirectly the other way around.

(WebCore::FrameLoader::checkLoadCompleteForThisFrame): Remove several assertions that should now be

absolutely identical to the remaining !pdl->isLoading().

(WebCore::FrameLoader::continueLoadAfterWillSubmitForm):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r112142 r112144  
     12012-03-26  Nate Chapin  <japhet@chromium.org>
     2
     3        Simplify setting loading state in DocumentLoader
     4        https://bugs.webkit.org/show_bug.cgi?id=82099
     5
     6        Reviewed by Adam Barth.
     7
     8        The logic for deciding what to return for DocumentLoader::isLoading()
     9        is crazy. It's indirectly based on the ResourceLoaders that have
     10        registered themselves with the DocumentLoader, but we can make that
     11        relationship more direct.
     12
     13        * loader/DocumentLoader.cpp:
     14        (WebCore::DocumentLoader::checkLoadComplete): Renamed from updateLoading, since it's not actually
     15            updating anything anymore. It now calls DOMWindow::finishedLoading() if loading is in fact done.
     16        (WebCore::DocumentLoader::startLoadingMainResource): The only reason this had a return value was to call
     17            updateLoading() if loading failed. Just call checkLoadComplete() directly (this allows it to
     18            be private, whereas updateLoading() was public).
     19        (WebCore::DocumentLoader::subresourceLoaderFinishedLoadingOnePart):
     20        * loader/DocumentLoader.h:
     21        (WebCore::DocumentLoader::isLoading): Rather than holding a separate bool, return based on the presence
     22            of non-multipart ResourceLoaders directly.
     23        * loader/FrameLoader.cpp:
     24        (WebCore::FrameLoader::isLoading): Depend on DocumentLoader::isLoading() for the activeDocumentLoader(),
     25            rather than indirectly the other way around.
     26        (WebCore::FrameLoader::checkLoadCompleteForThisFrame): Remove several assertions that should now be
     27            absolutely identical to the remaining !pdl->isLoading().
     28        (WebCore::FrameLoader::continueLoadAfterWillSubmitForm):
     29
    1302012-03-26  James Robinson  <jamesr@chromium.org>
    231

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

    r111361 r112144  
    9191    , m_committed(false)
    9292    , m_isStopping(false)
    93     , m_loading(false)
    9493    , m_gotFirstByte(false)
    9594    , m_primaryLoadComplete(false)
    … …  
    214213void DocumentLoader::stopLoading()
    215214{
    216     // In some rare cases, calling FrameLoader::stopLoading could set m_loading to false.
     215    // In some rare cases, calling FrameLoader::stopLoading could cause isLoading() to return false.
    217216    // (This can happen when there's a single XMLHttpRequest currently loading and stopLoading causes it
    218217    // to stop loading. Because of this, we need to save it so we don't return early.
    219     bool loading = m_loading;
     218    bool loading = isLoading();
    220219   
    221220    if (m_committed) {
    … …  
    237236        // If something above restarted loading we might run into mysterious crashes like
    238237        // https://bugs.webkit.org/show_bug.cgi?id=62764 and <rdar://problem/9328684>
    239         ASSERT(!m_loading);
     238        ASSERT(!isLoading());
    240239        return;
    241240    }
    … …  
    372371}
    373372
    374 void DocumentLoader::updateLoading()
    375 {
    376     if (!m_frame) {
    377         setLoading(false);
    378         return;
    379     }
     373void DocumentLoader::checkLoadComplete()
     374{
     375    if (!m_frame || isLoading())
     376        return;
    380377    ASSERT(this == frameLoader()->activeDocumentLoader());
    381     bool wasLoading = m_loading;
    382     setLoading(frameLoader()->isLoading());
    383 
    384     if (wasLoading && !m_loading) {
    385         if (DOMWindow* window = m_frame->existingDOMWindow())
    386             window->finishedLoading();
    387     }
     378
     379    if (DOMWindow* window = m_frame->existingDOMWindow())
     380        window->finishedLoading();
    388381}
    389382
    … …  
    424417    ASSERT(frameLoader());
    425418    clearErrors();
    426    
    427     setLoading(true);
    428    
    429419    frameLoader()->prepareForLoadStart();
    430420}
    … …  
    440430
    441431        if (this == frameLoader()->activeDocumentLoader())
    442             updateLoading();
     432            checkLoadComplete();
    443433    }
    444434}
    … …  
    771761{
    772762    m_subresourceLoaders.add(loader);
    773     setLoading(true);
    774763}
    775764
    … …  
    777766{
    778767    m_subresourceLoaders.remove(loader);
    779     updateLoading();
     768    checkLoadComplete();
    780769    if (Frame* frame = m_frame)
    781770        frame->loader()->checkLoadComplete();
    … …  
    785774{
    786775    m_plugInStreamLoaders.add(loader);
    787     setLoading(true);
    788776}
    789777
    … …  
    791779{
    792780    m_plugInStreamLoaders.remove(loader);
    793     updateLoading();
     781    checkLoadComplete();
    794782}
    795783
    … …  
    799787}
    800788
    801 bool DocumentLoader::isLoadingSubresources() const
    802 {
    803     return !m_subresourceLoaders.isEmpty();
    804 }
    805 
    806 bool DocumentLoader::isLoadingPlugIns() const
    807 {
    808     return !m_plugInStreamLoaders.isEmpty();
    809 }
    810 
    811789bool DocumentLoader::isLoadingMultipartContent() const
    812790{
    … …  
    814792}
    815793
    816 bool DocumentLoader::startLoadingMainResource(unsigned long identifier)
     794void DocumentLoader::startLoadingMainResource(unsigned long identifier)
    817795{
    818796    ASSERT(!m_mainResourceLoader);
    … …  
    831809        LOG_ERROR("could not create WebResourceHandle for URL %s -- should be caught by policy handler level", m_request.url().string().ascii().data());
    832810        m_mainResourceLoader = 0;
    833         return false;
    834     }
    835 
    836     return true;
     811        ASSERT(!isLoading());
     812        checkLoadComplete();
     813    }
    837814}
    838815
    … …  
    846823    m_multipartSubresourceLoaders.add(loader);
    847824    m_subresourceLoaders.remove(loader);
    848     updateLoading();
     825    checkLoadComplete();
    849826    if (Frame* frame = m_frame)
    850827        frame->loader()->checkLoadComplete();   

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

    r111361 r112144  
    111111        void setCommitted(bool committed) { m_committed = committed; }
    112112        bool isCommitted() const { return m_committed; }
    113         bool isLoading() const { return m_loading; }
    114         void setLoading(bool loading) { m_loading = loading; }
    115         void updateLoading();
     113        bool isLoading() const { return isLoadingMainResource() || !m_subresourceLoaders.isEmpty() || !m_plugInStreamLoaders.isEmpty(); }
    116114        void receivedData(const char*, int);
    117115        void setupForReplaceByMIMEType(const String& newMIMEType);
    … …  
    196194        void setDefersLoading(bool);
    197195
    198         bool startLoadingMainResource(unsigned long identifier);
     196        void startLoadingMainResource(unsigned long identifier);
    199197        void cancelMainResourceLoad(const ResourceError&);
    200198       
    … …  
    208206       
    209207        bool isLoadingMainResource() const;
    210         bool isLoadingSubresources() const;
    211         bool isLoadingPlugIns() const;
    212208        bool isLoadingMultipartContent() const;
    213209
    … …  
    263259        void commitLoad(const char*, int);
    264260        bool doesProgressiveLoad(const String& MIMEType) const;
     261        void checkLoadComplete();
    265262
    266263        void deliverSubstituteResourcesAfterDelay();
    … …  
    301298        bool m_committed;
    302299        bool m_isStopping;
    303         bool m_loading;
    304300        bool m_gotFirstByte;
    305301        bool m_primaryLoadComplete;

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

    r111901 r112144  
    16391639    if (!docLoader)
    16401640        return false;
    1641     return docLoader->isLoadingMainResource() || docLoader->isLoadingSubresources() || docLoader->isLoadingPlugIns();
     1641    return docLoader->isLoading();
    16421642}
    16431643
    … …  
    22132213
    22142214                ASSERT(!pdl->isLoading());
    2215                 ASSERT(!pdl->isLoadingMainResource());
    2216                 ASSERT(!pdl->isLoadingSubresources());
    2217                 ASSERT(!pdl->isLoadingPlugIns());
    22182215
    22192216                // If we're in the middle of loading multipart data, we need to restore the document loader.
    … …  
    23172314
    23182315    m_provisionalDocumentLoader->timing()->markNavigationStart(frame());
    2319 
    2320     if (!m_provisionalDocumentLoader->startLoadingMainResource(identifier))
    2321         m_provisionalDocumentLoader->updateLoading();
     2316    m_provisionalDocumentLoader->startLoadingMainResource(identifier);
    23222317}
    23232318
Note: See TracChangeset for help on using the changeset viewer.