Changeset 127690 in webkit

Timestamp:

Sep 5, 2012 9:48:20 PM (12 years ago)

Author:

Nate Chapin

Message:

Source/WebCore: [chromium] Some SubstituteData loads broken after r121912
​https://bugs.webkit.org/show_bug.cgi?id=91685

Reviewed by Adam Barth.

Test: WebFrameTest.ReplaceNavigationAfterHistoryNavigation in chromium's webkit_unit_tests.

• loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::commitData): receivedFirstData() should be called exactly once per load,

on the first commit. I had mistakely assumed in r121912 that isReplacing() was true only for multipart
loads, and only after the first commit (chromium uses it for some SubstituteData loads to ensure the error page
replaces the failed load). We need to check whether we are loading multipart content before assuming isReplacing()
will tell us what we need to know.

Source/WebKit/chromium: Test for ​https://bugs.webkit.org/show_bug.cgi?id=91685

Reviewed by Adam Barth.

• tests/WebFrameTest.cpp:

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/DocumentLoader.cpp (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/tests/WebFrameTest.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-09-05  Nate Chapin  <japhet@chromium.org>
+
+        [chromium] Some SubstituteData loads broken after r121912
+        https://bugs.webkit.org/show_bug.cgi?id=91685
+
+        Reviewed by Adam Barth.
+
+        Test: WebFrameTest.ReplaceNavigationAfterHistoryNavigation in chromium's webkit_unit_tests.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::commitData): receivedFirstData() should be called exactly once per load,
+            on the first commit. I had mistakely assumed in r121912 that isReplacing() was true only for multipart
+            loads, and only after the first commit (chromium uses it for some SubstituteData loads to ensure the error page
+            replaces the failed load). We need to check whether we are loading multipart content before assuming isReplacing()
+            will tell us what we need to know.
+
 2012-09-05  James Robinson  <jamesr@chromium.org>
 

trunk/Source/WebCore/loader/DocumentLoader.cpp

@@ -337 +337 @@
 #endif
 
-        if (!frameLoader()->isReplacing())
+        // 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())
             frameLoader()->receivedFirstData();
 

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2012-09-05  Nate Chapin  <japhet@chromium.org>
+
+        Test for https://bugs.webkit.org/show_bug.cgi?id=91685
+
+        Reviewed by Adam Barth.
+
+        * tests/WebFrameTest.cpp:
+
 2012-09-05  Rick Byers  <rbyers@chromium.org>
 

trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp

@@ -46 +46 @@
 #include "WebFrameClient.h"
 #include "WebFrameImpl.h"
+#include "WebHistoryItem.h"
 #include "WebRange.h"
 #include "WebScriptSource.h"
@@ -55 +56 @@
 #include "WebViewImpl.h"
 #include "platform/WebFloatRect.h"
+#include "platform/WebURLResponse.h"
 #include "v8.h"
 #include <gtest/gtest.h>
@@ -1147 +1149 @@
 }
 
+class TestSubstituteDataWebFrameClient : public WebFrameClient {
+public:
+    TestSubstituteDataWebFrameClient()
+        : m_commitCalled(false)
+    {
+    }
+
+    virtual void didFailProvisionalLoad(WebFrame* frame, const WebURLError& error)
+    {
+        frame->loadHTMLString("This should appear", toKURL("data:text/html,chromewebdata"), error.unreachableURL, true);
+        webkit_support::RunAllPendingMessages();
+    }
+
+    virtual void didCommitProvisionalLoad(WebFrame* frame, bool)
+    {
+        if (frame->dataSource()->response().url() != WebURL(URLTestHelpers::toKURL("about:blank")))
+            m_commitCalled = true;
+    }
+
+    bool commitCalled() const { return m_commitCalled; }
+
+private:
+    bool m_commitCalled;
+};
+
+TEST_F(WebFrameTest, ReplaceNavigationAfterHistoryNavigation)
+{
+    TestSubstituteDataWebFrameClient webFrameClient;
+
+    WebView* webView = FrameTestHelpers::createWebViewAndLoad("about:blank", true, &webFrameClient);
+    webkit_support::RunAllPendingMessages();
+    WebFrame* frame = webView->mainFrame();
+
+    // Load a url as a history navigation that will return an error. TestSubstituteDataWebFrameClient
+    // will start a SubstituteData load in response to the load failure, which should get fully committed.
+    // Due to https://bugs.webkit.org/show_bug.cgi?id=91685, FrameLoader::didReceiveData() wasn't getting
+    // called in this case, which resulted in the SubstituteData document not getting displayed.
+    WebURLError error;
+    error.reason = 1337;
+    error.domain = "WebFrameTest";
+    std::string errorURL = "http://0.0.0.0";
+    WebURLResponse response;
+    response.initialize();
+    response.setURL(URLTestHelpers::toKURL(errorURL));
+    response.setMIMEType("text/html");
+    response.setHTTPStatusCode(500);
+    WebHistoryItem errorHistoryItem;
+    errorHistoryItem.initialize();
+    errorHistoryItem.setURLString(WebString::fromUTF8(errorURL.c_str(), errorURL.length()));
+    errorHistoryItem.setOriginalURLString(WebString::fromUTF8(errorURL.c_str(), errorURL.length()));
+    webkit_support::RegisterMockedErrorURL(URLTestHelpers::toKURL(errorURL), response, error);
+    frame->loadHistoryItem(errorHistoryItem);
+    webkit_support::ServeAsynchronousMockedRequests();
+
+    WebString text = frame->contentAsText(std::numeric_limits<size_t>::max());
+    EXPECT_EQ("This should appear", std::string(text.utf8().data()));
+    EXPECT_TRUE(webFrameClient.commitCalled());
+}
+
 } // namespace