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

Changeset 130651 in webkit


Ignore:
Timestamp:
Oct 8, 2012, 10:08:38 AM (14 years ago)
Author:
Nate Chapin
Message:

Source/WebCore: Loader cleanup : Simplify FrameLoader/DocumentLoader setupForReplace()
​https://bugs.webkit.org/show_bug.cgi?id=49072

Reviewed by Eric Seidel.

This patch contains one small known behavior change: multipart/x-mixed-replace main resources with text/html parts
will no longer load the text/html progressively. In practice, loading the html progressively causes the document
to get cleared as soon as the next part's data starts arriving, which leads to a blank page most of the time. This case
seems to be pathological, as IE, FF, Opera and WebKit all do something different currently. This patch will cause
us to behave like Firefox, which is the most sane of the current behaviors.

Test: http/tests/multipart/multipart-html.php

  • loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::commitData): Use isMultipartReplacingLoad() helper.
(WebCore::DocumentLoader::receivedData):
(WebCore::DocumentLoader::setupForReplace): Renamed from setupForReplaceByMIMEType(). Call maybeFinishLoadingMultipartContent()

instead of doing identical work inline. After we call frameLoader()->setReplacing(), we will never load progressively, so remove
the if (doesProgressiveLoad(newMIMEType)) {} block.

(WebCore::DocumentLoader::isMultipartReplacingLoad):
(WebCore::DocumentLoader::maybeFinishLoadingMultipartContent): Inline the old DocumentLoader::setupForeReplace(), check

frameLoader()->isReplacing() instead of the delete doesProgressiveLoad().

  • loader/DocumentLoader.h:
  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::setupForReplace): Move all calls to revertToProvisionalState here.

  • loader/MainResourceLoader.cpp:

(WebCore::MainResourceLoader::didReceiveResponse): Call setupForReplace(), renamed from setupForReplaceByMIMEType().

LayoutTests: Add a test for multipart/x-mixed-replace documents with text/html
parts.
​https://bugs.webkit.org/show_bug.cgi?id=49072

Reviewed by Eric Seidel.

  • http/tests/multipart/multipart-html-expected.txt: Added.
  • http/tests/multipart/multipart-html.php: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r130650 r130651  
     12012-10-08  Nate Chapin  <japhet@chromium.org>
     2
     3        Add a test for multipart/x-mixed-replace documents with text/html
     4        parts.
     5        https://bugs.webkit.org/show_bug.cgi?id=49072
     6
     7        Reviewed by Eric Seidel.
     8
     9        * http/tests/multipart/multipart-html-expected.txt: Added.
     10        * http/tests/multipart/multipart-html.php: Added.
     11
    1122012-10-08  Raphael Kubo da Costa  <raphael.kubo.da.costa@intel.com>
    213

  • trunk/Source/WebCore/ChangeLog

    r130636 r130651  
     12012-10-08  Nate Chapin  <japhet@chromium.org>
     2
     3        Loader cleanup : Simplify FrameLoader/DocumentLoader setupForReplace()
     4        https://bugs.webkit.org/show_bug.cgi?id=49072
     5
     6        Reviewed by Eric Seidel.
     7
     8        This patch contains one small known behavior change: multipart/x-mixed-replace main resources with text/html parts
     9        will no longer load the text/html progressively. In practice, loading the html progressively causes the document
     10        to get cleared as soon as the next part's data starts arriving, which leads to a blank page most of the time. This case
     11        seems to be pathological, as IE, FF, Opera and WebKit all do something different currently. This patch will cause
     12        us to behave like Firefox, which is the most sane of the current behaviors.
     13
     14        Test: http/tests/multipart/multipart-html.php
     15
     16        * loader/DocumentLoader.cpp:
     17        (WebCore::DocumentLoader::commitData): Use isMultipartReplacingLoad() helper.
     18        (WebCore::DocumentLoader::receivedData):
     19        (WebCore::DocumentLoader::setupForReplace): Renamed from setupForReplaceByMIMEType(). Call maybeFinishLoadingMultipartContent()
     20            instead of doing identical work inline. After we call frameLoader()->setReplacing(), we will never load progressively, so remove
     21            the if (doesProgressiveLoad(newMIMEType)) {} block.
     22        (WebCore::DocumentLoader::isMultipartReplacingLoad):
     23        (WebCore::DocumentLoader::maybeFinishLoadingMultipartContent): Inline the old DocumentLoader::setupForeReplace(), check
     24            frameLoader()->isReplacing() instead of the delete doesProgressiveLoad().
     25        * loader/DocumentLoader.h:
     26        * loader/FrameLoader.cpp:
     27        (WebCore::FrameLoader::setupForReplace): Move all calls to revertToProvisionalState here.
     28        * loader/MainResourceLoader.cpp:
     29        (WebCore::MainResourceLoader::didReceiveResponse): Call setupForReplace(), renamed from setupForReplaceByMIMEType().
     30
    1312012-10-08  Zoltan Horvath  <zoltan@webkit.org>
    232

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

    r130612 r130651  
    273273}
    274274
    275 void DocumentLoader::setupForReplace()
    276 {
    277     frameLoader()->setupForReplace();
    278     m_committed = false;
    279 }
    280 
    281275void DocumentLoader::commitIfReady()
    282276{
    … …  
    341335
    342336        // Call receivedFirstData() exactly once per load. We should only reach this point multiple times
    343         // for multipart loads, and isReplacing() will be true after the first time.
    344         if (!m_mainResourceLoader || !m_mainResourceLoader->isLoadingMultipartContent() || !frameLoader()->isReplacing())
     337        // for multipart loads, and FrameLoader::isReplacing() will be true after the first time.
     338        if (!isMultipartReplacingLoad())
    345339            frameLoader()->receivedFirstData();
    346340
    … …  
    386380}
    387381
    388 bool DocumentLoader::doesProgressiveLoad(const String& MIMEType) const
    389 {
    390     return !frameLoader()->isReplacing() || MIMEType == "text/html";
    391 }
    392 
    393382void DocumentLoader::receivedData(const char* data, int length)
    394383{
    395     if (doesProgressiveLoad(m_response.mimeType()))
     384    if (!isMultipartReplacingLoad())
    396385        commitLoad(data, length);
    397386}
    398387
    399 void DocumentLoader::setupForReplaceByMIMEType(const String& newMIMEType)
     388void DocumentLoader::setupForReplace()
    400389{
    401390    if (!mainResourceData())
    402391        return;
    403392   
    404     String oldMIMEType = m_response.mimeType();
    405    
    406     if (!doesProgressiveLoad(oldMIMEType)) {
    407         frameLoader()->client()->revertToProvisionalState(this);
    408         setupForReplace();
    409         RefPtr<SharedBuffer> resourceData = mainResourceData();
    410         commitLoad(resourceData->data(), resourceData->size());
    411     }
    412    
     393    maybeFinishLoadingMultipartContent();
    413394    maybeCreateArchive();
    414395    m_writer.end();
    415    
    416396    frameLoader()->setReplacing();
    417397    m_gotFirstByte = false;
    418    
    419     if (doesProgressiveLoad(newMIMEType)) {
    420         frameLoader()->client()->revertToProvisionalState(this);
    421         setupForReplace();
    422     }
    423398   
    424399    stopLoadingSubresources();
    … …  
    860835}
    861836
     837bool DocumentLoader::isMultipartReplacingLoad() const
     838{
     839    return isLoadingMultipartContent() && frameLoader()->isReplacing();
     840}
     841
    862842void DocumentLoader::startLoadingMainResource()
    863843{
    … …  
    889869void DocumentLoader::maybeFinishLoadingMultipartContent()
    890870{
    891     if (!doesProgressiveLoad(m_response.mimeType())) {
    892         frameLoader()->client()->revertToProvisionalState(this);
    893         setupForReplace();
    894         RefPtr<SharedBuffer> resourceData = mainResourceData();
    895         commitLoad(resourceData->data(), resourceData->size());
    896     }
     871    if (!frameLoader()->isReplacing())
     872        return;
     873
     874    frameLoader()->setupForReplace();
     875    m_committed = false;
     876    RefPtr<SharedBuffer> resourceData = mainResourceData();
     877    commitLoad(resourceData->data(), resourceData->size());
    897878}
    898879

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

    r128418 r130651  
    113113        bool isLoading() const { return isLoadingMainResource() || !m_subresourceLoaders.isEmpty() || !m_plugInStreamLoaders.isEmpty(); }
    114114        void receivedData(const char*, int);
    115         void setupForReplaceByMIMEType(const String& newMIMEType);
     115        void setupForReplace();
    116116        void finishedLoading();
    117117        const ResourceResponse& response() const { return m_response; }
    … …  
    251251
    252252    private:
    253         void setupForReplace();
    254253        void commitIfReady();
    255254        void setMainDocumentError(const ResourceError&);
    256255        void commitLoad(const char*, int);
    257         bool doesProgressiveLoad(const String& MIMEType) const;
    258256        void checkLoadComplete();
    259257        void clearMainResourceLoader();
    … …  
    263261        void clearArchiveResources();
    264262#endif
     263
     264        bool isMultipartReplacingLoad() const;
    265265
    266266        void deliverSubstituteResourcesAfterDelay();

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

    r130313 r130651  
    11461146void FrameLoader::setupForReplace()
    11471147{
     1148    m_client->revertToProvisionalState(m_documentLoader.get());
    11481149    setState(FrameStateProvisional);
    11491150    m_provisionalDocumentLoader = m_documentLoader;

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

    r130612 r130651  
    387387
    388388    if (m_loadingMultipartContent) {
    389         frameLoader()->activeDocumentLoader()->setupForReplaceByMIMEType(r.mimeType());
     389        m_documentLoader->setupForReplace();
    390390        clearResourceData();
    391391    }
Note: See TracChangeset for help on using the changeset viewer.