REGRESSION: Assertion failure in FrameLoader::continueLoadAfterWillSubmitForm() when
authorjberlin@webkit.org <jberlin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Jan 2011 20:22:17 +0000 (20:22 +0000)
committerjberlin@webkit.org <jberlin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Jan 2011 20:22:17 +0000 (20:22 +0000)
navigating back to an unreachable URL
https://bugs.webkit.org/show_bug.cgi?id=52388

Reviewed by Anders Carlsson.

Source/WebCore:

Test: http/tests/navigation/go-back-to-error-page.html

* history/PageCache.cpp:
(WebCore::PageCache::canCachePageContainingThisFrame):
Do not cache any error pages (which we can recognize as having substitute data and/or an
unreachableURL).

Tools:

Add a third parameter (unreachableURL) to queueLoadHTMLString.

* DumpRenderTree/LayoutTestController.cpp:
(queueLoadHTMLStringCallback):
(LayoutTestController::queueLoadHTMLString):
(LayoutTestController::queueLoadAlternateHTMLString):
* DumpRenderTree/LayoutTestController.h:

* DumpRenderTree/WorkQueueItem.h:
(LoadHTMLStringItem::LoadHTMLStringItem):

* DumpRenderTree/chromium/LayoutTestController.cpp:
(WorkItemLoadHTMLString::WorkItemLoadHTMLString):
(WorkItemLoadHTMLString::run):
(LayoutTestController::queueLoadHTMLString):

* DumpRenderTree/gtk/WorkQueueItemGtk.cpp:
(LoadHTMLStringItem::invoke):
* DumpRenderTree/mac/WorkQueueItemMac.mm:
(LoadHTMLStringItem::invoke):
* DumpRenderTree/win/WorkQueueItemWin.cpp:
(LoadHTMLStringItem::invoke):

LayoutTests:

Add a test that loads an alternate HTML String for an "unreachable" URL, loads another
page, and then goes back to the "unreachable" URL, which is now reachable.

This tests two things:
1) No assertion failure when going back to an unreachable URL.
2) There is no page cached for the unreachable URL.

* http/tests/navigation/go-back-to-error-page-expected.txt: Added.
* http/tests/navigation/go-back-to-error-page.html: Added.
* http/tests/navigation/resources/page-to-go-back-from.html: Added.
* http/tests/navigation/resources/page-treated-as-unreachable.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@75966 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/navigation/go-back-to-error-page-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/navigation/go-back-to-error-page.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/resources/page-to-go-back-from.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/resources/page-treated-as-unreachable.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/history/PageCache.cpp
Tools/ChangeLog
Tools/DumpRenderTree/LayoutTestController.cpp
Tools/DumpRenderTree/LayoutTestController.h
Tools/DumpRenderTree/WorkQueueItem.h
Tools/DumpRenderTree/chromium/LayoutTestController.cpp
Tools/DumpRenderTree/gtk/WorkQueueItemGtk.cpp
Tools/DumpRenderTree/mac/WorkQueueItemMac.mm
Tools/DumpRenderTree/win/WorkQueueItemWin.cpp

index 7d56cf4..f2c001b 100644 (file)
@@ -1,3 +1,23 @@
+2011-01-17  Jessie Berlin  <jberlin@apple.com>
+
+        Reviewed by Anders Carlsson.
+
+        REGRESSION: Assertion failure in FrameLoader::continueLoadAfterWillSubmitForm() when
+        navigating back to an unreachable URL
+        https://bugs.webkit.org/show_bug.cgi?id=52388
+
+        Add a test that loads an alternate HTML String for an "unreachable" URL, loads another
+        page, and then goes back to the "unreachable" URL, which is now reachable.
+
+        This tests two things:
+        1) No assertion failure when going back to an unreachable URL.
+        2) There is no page cached for the unreachable URL.
+
+        * http/tests/navigation/go-back-to-error-page-expected.txt: Added.
+        * http/tests/navigation/go-back-to-error-page.html: Added.
+        * http/tests/navigation/resources/page-to-go-back-from.html: Added.
+        * http/tests/navigation/resources/page-treated-as-unreachable.html: Added.
+
 2011-01-17  Pavel Feldman  <pfeldman@chromium.org>
 
         Not reviewed: skip flaky inspector test (will look at it tomorrow).
diff --git a/LayoutTests/http/tests/navigation/go-back-to-error-page-expected.txt b/LayoutTests/http/tests/navigation/go-back-to-error-page-expected.txt
new file mode 100644 (file)
index 0000000..158bb8f
--- /dev/null
@@ -0,0 +1 @@
+PASS: Going back to a page that was previously treated as unreachable loads the actual page.
diff --git a/LayoutTests/http/tests/navigation/go-back-to-error-page.html b/LayoutTests/http/tests/navigation/go-back-to-error-page.html
new file mode 100644 (file)
index 0000000..09300ad
--- /dev/null
@@ -0,0 +1,14 @@
+<script>
+if (window.layoutTestController) {
+    layoutTestController.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+    layoutTestController.dumpAsText();
+    layoutTestController.queueLoadHTMLString("<script>document.write('<div id=result>FAIL: The alternate HTML string was cached, but should not have been cached for an error page</div>');<\/script>", "resources", "http://127.0.0.1:8000/navigation/resources/page-treated-as-unreachable.html");
+    layoutTestController.queueLoad("resources/page-to-go-back-from.html");
+    layoutTestController.queueBackNavigation(1);
+} else
+    document.write('This test must be run by DumpRenderTree!')
+</script>
+
+<p>This tests that going back to an error page (an unreachable URL for which there is substitute
+data) does not cause an assertion failure.</p>
+
diff --git a/LayoutTests/http/tests/navigation/resources/page-to-go-back-from.html b/LayoutTests/http/tests/navigation/resources/page-to-go-back-from.html
new file mode 100644 (file)
index 0000000..de248f9
--- /dev/null
@@ -0,0 +1,8 @@
+<html>
+  <head>
+    <title>A page that can be navigated back from</title>
+  </head>
+  <body>
+    <p>This is a page that can be navigated back from.</p>
+  </body>
+</html>
diff --git a/LayoutTests/http/tests/navigation/resources/page-treated-as-unreachable.html b/LayoutTests/http/tests/navigation/resources/page-treated-as-unreachable.html
new file mode 100644 (file)
index 0000000..fb8a0ae
--- /dev/null
@@ -0,0 +1,8 @@
+<html>
+  <head>
+    <title>This page will be initially treated as unreachable</title>
+  </head>
+  <body>
+    <p>PASS: Going back to a page that was previously treated as unreachable loads the actual page.</p>
+  </body>
+</html>
index f0a3483..c1176ca 100644 (file)
@@ -1,3 +1,18 @@
+2011-01-17  Jessie Berlin  <jberlin@apple.com>
+
+        Reviewed by Anders Carlsson.
+
+        REGRESSION: Assertion failure in FrameLoader::continueLoadAfterWillSubmitForm() when
+        navigating back to an unreachable URL
+        https://bugs.webkit.org/show_bug.cgi?id=52388
+
+        Test: http/tests/navigation/go-back-to-error-page.html
+
+        * history/PageCache.cpp:
+        (WebCore::PageCache::canCachePageContainingThisFrame):
+        Do not cache any error pages (which we can recognize as having substitute data and/or an
+        unreachableURL).
+
 2011-01-17  Pavel Feldman  <pfeldman@chromium.org>
 
         Not reviewed: fixing typo in r75952.
index b14b606..a5dfb1e 100644 (file)
@@ -252,6 +252,8 @@ bool PageCache::canCachePageContainingThisFrame(Frame* frame)
     
     return frame->loader()->documentLoader()
         && frame->loader()->documentLoader()->mainDocumentError().isNull()
+        // Do not cache error pages (these can be recognized as pages with substitute data or unreachable URLs).
+        && !(frame->loader()->documentLoader()->substituteData().isValid() && !frame->loader()->documentLoader()->substituteData().failingURL().isEmpty())
         // FIXME: If we ever change this so that frames with plug-ins will be cached,
         // we need to make sure that we don't cache frames that have outstanding NPObjects
         // (objects created by the plug-in). Since there is no way to pause/resume a Netscape plug-in,
index 708ec09..6551dbe 100644 (file)
@@ -1,3 +1,34 @@
+2011-01-17  Jessie Berlin  <jberlin@apple.com>
+
+        Reviewed by Anders Carlsson.
+
+        REGRESSION: Assertion failure in FrameLoader::continueLoadAfterWillSubmitForm() when
+        navigating back to an unreachable URL
+        https://bugs.webkit.org/show_bug.cgi?id=52388
+
+        Add a third parameter (unreachableURL) to queueLoadHTMLString.
+
+        * DumpRenderTree/LayoutTestController.cpp:
+        (queueLoadHTMLStringCallback):
+        (LayoutTestController::queueLoadHTMLString):
+        (LayoutTestController::queueLoadAlternateHTMLString):
+        * DumpRenderTree/LayoutTestController.h:
+
+        * DumpRenderTree/WorkQueueItem.h:
+        (LoadHTMLStringItem::LoadHTMLStringItem):
+
+        * DumpRenderTree/chromium/LayoutTestController.cpp:
+        (WorkItemLoadHTMLString::WorkItemLoadHTMLString):
+        (WorkItemLoadHTMLString::run):
+        (LayoutTestController::queueLoadHTMLString):
+
+        * DumpRenderTree/gtk/WorkQueueItemGtk.cpp:
+        (LoadHTMLStringItem::invoke):
+        * DumpRenderTree/mac/WorkQueueItemMac.mm:
+        (LoadHTMLStringItem::invoke):
+        * DumpRenderTree/win/WorkQueueItemWin.cpp:
+        (LoadHTMLStringItem::invoke):
+
 2011-01-17  Dan Bernstein  <mitz@apple.com>
 
         Rubber-stamped by Mark Rowe.
index a3d9727..1133a88 100644 (file)
@@ -858,8 +858,16 @@ static JSValueRef queueLoadHTMLStringCallback(JSContextRef context, JSObjectRef
         baseURL.adopt(JSStringCreateWithUTF8CString(""));
 
     LayoutTestController* controller = static_cast<LayoutTestController*>(JSObjectGetPrivate(thisObject));
-    controller->queueLoadHTMLString(content.get(), baseURL.get());
 
+    if (argumentCount >= 3) {
+        JSRetainPtr<JSStringRef> unreachableURL;
+        unreachableURL.adopt(JSValueToStringCopy(context, arguments[2], exception));
+        ASSERT(!*exception);
+        controller->queueLoadAlternateHTMLString(content.get(), baseURL.get(), unreachableURL.get());
+        return JSValueMakeUndefined(context);
+    }
+
+    controller->queueLoadHTMLString(content.get(), baseURL.get());
     return JSValueMakeUndefined(context);
 }
 
@@ -2103,6 +2111,11 @@ void LayoutTestController::queueLoadHTMLString(JSStringRef content, JSStringRef
     WorkQueue::shared()->queue(new LoadHTMLStringItem(content, baseURL));
 }
 
+void LayoutTestController::queueLoadAlternateHTMLString(JSStringRef content, JSStringRef baseURL, JSStringRef unreachableURL)
+{
+    WorkQueue::shared()->queue(new LoadHTMLStringItem(content, baseURL, unreachableURL));
+}
+
 void LayoutTestController::queueBackNavigation(int howFarBack)
 {
     WorkQueue::shared()->queue(new BackItem(howFarBack));
index 9704128..b80d805 100644 (file)
@@ -77,6 +77,7 @@ public:
     void queueForwardNavigation(int howFarForward);
     void queueLoad(JSStringRef url, JSStringRef target);
     void queueLoadHTMLString(JSStringRef content, JSStringRef baseURL);
+    void queueLoadAlternateHTMLString(JSStringRef content, JSStringRef baseURL, JSStringRef unreachableURL);
     void queueLoadingScript(JSStringRef script);
     void queueNonLoadingScript(JSStringRef script);
     void queueReload();
index 34276c8..a5823c1 100644 (file)
@@ -61,11 +61,19 @@ public:
     {
     }
 
+    LoadHTMLStringItem(const JSStringRef content, const JSStringRef baseURL, const JSStringRef unreachableURL)
+        : m_content(content)
+        , m_baseURL(baseURL)
+        , m_unreachableURL(unreachableURL)
+    {
+    }
+
 private:
     virtual bool invoke() const;
 
     JSRetainPtr<JSStringRef> m_content;
     JSRetainPtr<JSStringRef> m_baseURL;
+    JSRetainPtr<JSStringRef> m_unreachableURL;
 };
 
 class ReloadItem : public WorkQueueItem {
index 4671438..529019b 100644 (file)
@@ -484,15 +484,20 @@ public:
     WorkItemLoadHTMLString(const std::string& html, const WebURL& baseURL)
         : m_html(html)
         , m_baseURL(baseURL) {}
+    WorkItemLoadHTMLString(const std::string& html, const WebURL& baseURL, const WebURL& unreachableURL)
+        : m_html(html)
+        , m_baseURL(baseURL)
+        , m_unreachableURL(unreachableURL) {}
     bool run(TestShell* shell)
     {
         shell->webView()->mainFrame()->loadHTMLString(
-            WebKit::WebData(m_html.data(), m_html.length()), m_baseURL);
+            WebKit::WebData(m_html.data(), m_html.length()), m_baseURL, m_unreachableURL);
         return true;
     }
 private:
     std::string m_html;
     WebURL m_baseURL;
+    WebURL m_unreachableURL;
 };
 
 void LayoutTestController::queueLoadHTMLString(const CppArgumentList& arguments, CppVariant* result)
@@ -502,7 +507,10 @@ void LayoutTestController::queueLoadHTMLString(const CppArgumentList& arguments,
         WebURL baseURL;
         if (arguments.size() > 1 && arguments[1].isString())
             baseURL = WebURL(GURL(arguments[1].toString()));
-        m_workQueue.addWork(new WorkItemLoadHTMLString(html, baseURL));
+        if (arguments.size() > 2 && arguments[2].isString())
+            m_workQueue.addWork(new WorkItemLoadHTMLString(html, baseURL, WebURL(GURL(arguments[2].toString()))));
+        else
+            m_workQueue.addWork(new WorkItemLoadHTMLString(html, baseURL));
     }
     result->setNull();
 }
index 0f44f54..d10b193 100644 (file)
@@ -61,6 +61,12 @@ bool LoadHTMLStringItem::invoke() const
 {
     GOwnPtr<gchar> content(JSStringCopyUTF8CString(m_content.get()));
     GOwnPtr<gchar> baseURL(JSStringCopyUTF8CString(m_baseURL.get()));
+
+    if (m_unreachableURL) {
+        GOwnPtr<gchar> unreachableURL(JSStringCopyUTF8CString(m_unreachableURL.get()));
+        webkit_web_frame_load_alternate_string(mainFrame, content.get(), baseURL.get(), unreachableURL.get());
+        return true;
+    }
     webkit_web_frame_load_string(mainFrame, content.get(), 0, 0, baseURL.get());
     return true;
 }
index 797afb7..c28e991 100644 (file)
@@ -59,6 +59,12 @@ bool LoadHTMLStringItem::invoke() const
     RetainPtr<CFStringRef> contentCF(AdoptCF, JSStringCopyCFString(kCFAllocatorDefault, m_content.get()));
     RetainPtr<CFStringRef> baseURLCF(AdoptCF, JSStringCopyCFString(kCFAllocatorDefault, m_baseURL.get()));
 
+    if (m_unreachableURL) {
+        RetainPtr<CFStringRef> unreachableURLCF(AdoptCF, JSStringCopyCFString(kCFAllocatorDefault, m_unreachableURL.get()));
+        [mainFrame loadAlternateHTMLString:(NSString *)contentCF.get() baseURL:[NSURL URLWithString:(NSString *)baseURLCF.get()] forUnreachableURL:[NSURL URLWithString:(NSString *)unreachableURLCF.get()]];
+        return true;
+    }
+
     [mainFrame loadHTMLString:(NSString *)contentCF.get() baseURL:[NSURL URLWithString:(NSString *)baseURLCF.get()]];
     return true;
 }
index a24ca37..49f0667 100644 (file)
@@ -89,6 +89,16 @@ bool LoadHTMLStringItem::invoke() const
     BSTR contentBSTR = SysAllocString(content.c_str());
     BSTR baseURLBSTR = SysAllocString(baseURL.c_str());
 
+    if (m_unreachableURL) {
+        wstring unreachableURL = jsStringRefToWString(m_unreachableURL.get());
+        BSTR unreachableURLBSTR = SysAllocString(unreachableURL.c_str());
+        frame->loadAlternateHTMLString(contentBSTR, baseURLBSTR, unreachableURLBSTR);
+        SysFreeString(contentBSTR);
+        SysFreeString(baseURLBSTR);
+        SysFreeString(unreachableURLBSTR);
+        return true;
+    }
+
     frame->loadHTMLString(contentBSTR, baseURLBSTR);
 
     SysFreeString(contentBSTR);