- Revision
- 130651
- Author
- jap...@chromium.org
- Date
- 2012-10-08 10:08:38 -0700 (Mon, 08 Oct 2012)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (130650 => 130651)
--- trunk/LayoutTests/ChangeLog 2012-10-08 17:06:38 UTC (rev 130650)
+++ trunk/LayoutTests/ChangeLog 2012-10-08 17:08:38 UTC (rev 130651)
@@ -1,3 +1,14 @@
+2012-10-08 Nate Chapin <jap...@chromium.org>
+
+ 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.
+
2012-10-08 Raphael Kubo da Costa <raphael.kubo.da.co...@intel.com>
[EFL] Skip svg/text/caret-in-svg-text.xhtml in WK1.
Added: trunk/LayoutTests/http/tests/multipart/multipart-html-expected.txt (0 => 130651)
--- trunk/LayoutTests/http/tests/multipart/multipart-html-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/multipart/multipart-html-expected.txt 2012-10-08 17:08:38 UTC (rev 130651)
@@ -0,0 +1 @@
+This text should only appear once 10
Added: trunk/LayoutTests/http/tests/multipart/multipart-html.php (0 => 130651)
--- trunk/LayoutTests/http/tests/multipart/multipart-html.php (rev 0)
+++ trunk/LayoutTests/http/tests/multipart/multipart-html.php 2012-10-08 17:08:38 UTC (rev 130651)
@@ -0,0 +1,26 @@
+<?php
+header('Content-type: multipart/x-mixed-replace; boundary=boundary');
+header('Connection: keep-alive');
+echo "--boundary\r\n";
+echo "Content-Type: text/html\r\n\r\n";
+echo str_pad('', 5000);
+?>
+
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+
+<?php
+for ($i = 0; $i <= 10; $i++) {
+ echo "--boundary\r\n";
+ echo "Content-Type: text/html\r\n\r\n";
+ echo "This text should only appear once ";
+ echo $i;
+ echo str_pad('', 5000);
+ echo "\r\n\r\n";
+ flush();
+ usleep(100000);
+ $i++;
+}
+?>
Modified: trunk/Source/WebCore/ChangeLog (130650 => 130651)
--- trunk/Source/WebCore/ChangeLog 2012-10-08 17:06:38 UTC (rev 130650)
+++ trunk/Source/WebCore/ChangeLog 2012-10-08 17:08:38 UTC (rev 130651)
@@ -1,3 +1,33 @@
+2012-10-08 Nate Chapin <jap...@chromium.org>
+
+ 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().
+
2012-10-08 Zoltan Horvath <zol...@webkit.org>
[Qt] r122720 causes performance regression with DirectFB on ARMv7
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (130650 => 130651)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2012-10-08 17:06:38 UTC (rev 130650)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2012-10-08 17:08:38 UTC (rev 130651)
@@ -272,12 +272,6 @@
m_isStopping = false;
}
-void DocumentLoader::setupForReplace()
-{
- frameLoader()->setupForReplace();
- m_committed = false;
-}
-
void DocumentLoader::commitIfReady()
{
if (!m_committed) {
@@ -340,8 +334,8 @@
#endif
// Call receivedFirstData() exactly once per load. We should only reach this point multiple times
- // for multipart loads, and isReplacing() will be true after the first time.
- if (!m_mainResourceLoader || !m_mainResourceLoader->isLoadingMultipartContent() || !frameLoader()->isReplacing())
+ // for multipart loads, and FrameLoader::isReplacing() will be true after the first time.
+ if (!isMultipartReplacingLoad())
frameLoader()->receivedFirstData();
bool userChosen = true;
@@ -385,42 +379,23 @@
info.addMember(m_mainResourceData);
}
-bool DocumentLoader::doesProgressiveLoad(const String& MIMEType) const
-{
- return !frameLoader()->isReplacing() || MIMEType == "text/html";
-}
-
void DocumentLoader::receivedData(const char* data, int length)
{
- if (doesProgressiveLoad(m_response.mimeType()))
+ if (!isMultipartReplacingLoad())
commitLoad(data, length);
}
-void DocumentLoader::setupForReplaceByMIMEType(const String& newMIMEType)
+void DocumentLoader::setupForReplace()
{
if (!mainResourceData())
return;
- String oldMIMEType = m_response.mimeType();
-
- if (!doesProgressiveLoad(oldMIMEType)) {
- frameLoader()->client()->revertToProvisionalState(this);
- setupForReplace();
- RefPtr<SharedBuffer> resourceData = mainResourceData();
- commitLoad(resourceData->data(), resourceData->size());
- }
-
+ maybeFinishLoadingMultipartContent();
maybeCreateArchive();
m_writer.end();
-
frameLoader()->setReplacing();
m_gotFirstByte = false;
- if (doesProgressiveLoad(newMIMEType)) {
- frameLoader()->client()->revertToProvisionalState(this);
- setupForReplace();
- }
-
stopLoadingSubresources();
stopLoadingPlugIns();
#if ENABLE(WEB_ARCHIVE) || ENABLE(MHTML)
@@ -859,6 +834,11 @@
return m_mainResourceLoader && m_mainResourceLoader->isLoadingMultipartContent();
}
+bool DocumentLoader::isMultipartReplacingLoad() const
+{
+ return isLoadingMultipartContent() && frameLoader()->isReplacing();
+}
+
void DocumentLoader::startLoadingMainResource()
{
m_mainDocumentError = ResourceError();
@@ -888,12 +868,13 @@
void DocumentLoader::maybeFinishLoadingMultipartContent()
{
- if (!doesProgressiveLoad(m_response.mimeType())) {
- frameLoader()->client()->revertToProvisionalState(this);
- setupForReplace();
- RefPtr<SharedBuffer> resourceData = mainResourceData();
- commitLoad(resourceData->data(), resourceData->size());
- }
+ if (!frameLoader()->isReplacing())
+ return;
+
+ frameLoader()->setupForReplace();
+ m_committed = false;
+ RefPtr<SharedBuffer> resourceData = mainResourceData();
+ commitLoad(resourceData->data(), resourceData->size());
}
void DocumentLoader::iconLoadDecisionAvailable()
Modified: trunk/Source/WebCore/loader/DocumentLoader.h (130650 => 130651)
--- trunk/Source/WebCore/loader/DocumentLoader.h 2012-10-08 17:06:38 UTC (rev 130650)
+++ trunk/Source/WebCore/loader/DocumentLoader.h 2012-10-08 17:08:38 UTC (rev 130651)
@@ -112,7 +112,7 @@
bool isCommitted() const { return m_committed; }
bool isLoading() const { return isLoadingMainResource() || !m_subresourceLoaders.isEmpty() || !m_plugInStreamLoaders.isEmpty(); }
void receivedData(const char*, int);
- void setupForReplaceByMIMEType(const String& newMIMEType);
+ void setupForReplace();
void finishedLoading();
const ResourceResponse& response() const { return m_response; }
const ResourceError& mainDocumentError() const { return m_mainDocumentError; }
@@ -250,11 +250,9 @@
bool m_deferMainResourceDataLoad;
private:
- void setupForReplace();
void commitIfReady();
void setMainDocumentError(const ResourceError&);
void commitLoad(const char*, int);
- bool doesProgressiveLoad(const String& MIMEType) const;
void checkLoadComplete();
void clearMainResourceLoader();
@@ -263,6 +261,8 @@
void clearArchiveResources();
#endif
+ bool isMultipartReplacingLoad() const;
+
void deliverSubstituteResourcesAfterDelay();
void substituteResourceDeliveryTimerFired(Timer<DocumentLoader>*);
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (130650 => 130651)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2012-10-08 17:06:38 UTC (rev 130650)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2012-10-08 17:08:38 UTC (rev 130651)
@@ -1145,6 +1145,7 @@
void FrameLoader::setupForReplace()
{
+ m_client->revertToProvisionalState(m_documentLoader.get());
setState(FrameStateProvisional);
m_provisionalDocumentLoader = m_documentLoader;
m_documentLoader = 0;
Modified: trunk/Source/WebCore/loader/MainResourceLoader.cpp (130650 => 130651)
--- trunk/Source/WebCore/loader/MainResourceLoader.cpp 2012-10-08 17:06:38 UTC (rev 130650)
+++ trunk/Source/WebCore/loader/MainResourceLoader.cpp 2012-10-08 17:08:38 UTC (rev 130651)
@@ -386,7 +386,7 @@
#endif
if (m_loadingMultipartContent) {
- frameLoader()->activeDocumentLoader()->setupForReplaceByMIMEType(r.mimeType());
+ m_documentLoader->setupForReplace();
clearResourceData();
}
