2011-01-21 Charlie Reis <creis@chromium.org>

        Reviewed by Darin Fisher.

        FrameLoader::checkLoadCompleteForThisFrame uses wrong history item
        https://bugs.webkit.org/show_bug.cgi?id=48812

        Test that we avoid updating back/forward list on a canceled navigation
        if a new navigation is already in process.  Also update forward-and-cancel
        to go forward, ensuring back/forward state is reset if user clicks stop.

        * http/tests/navigation/back-twice-without-commit-expected.txt: Added.
        * http/tests/navigation/back-twice-without-commit.html: Added.
        * http/tests/navigation/forward-and-cancel-expected.txt:
        * http/tests/navigation/forward-and-cancel.html: Go forward after stop, not back.
        * http/tests/navigation/resources/back-twice-page-2.html: Added.
        * http/tests/navigation/resources/back-twice-page-3.html: Added.
        * http/tests/navigation/resources/forward-and-cancel-frames.html: Reduced delay.
2011-01-21  Charlie Reis  <creis@chromium.org>

        Reviewed by Darin Fisher.

        FrameLoader::checkLoadCompleteForThisFrame uses wrong history item
        https://bugs.webkit.org/show_bug.cgi?id=48812

        Most calls to stopAllLoaders now clear the history's provisional item(s).
        We can now avoid resetting the back/forward state if a new navigation
        is in progress.

        Test: http/tests/navigation/back-twice-without-commit.html
        Test: http/tests/navigation/forward-and-cancel.html

        * loader/FrameLoader.cpp:
        * loader/FrameLoader.h:
        * loader/FrameLoaderTypes.h:
        * WebCore.exp.in: Update stopAllLoaders signature.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76357 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent f6cc8ed6
2011-01-21 Charlie Reis <creis@chromium.org>
Reviewed by Darin Fisher.
FrameLoader::checkLoadCompleteForThisFrame uses wrong history item
https://bugs.webkit.org/show_bug.cgi?id=48812
Test that we avoid updating back/forward list on a canceled navigation
if a new navigation is already in process. Also update forward-and-cancel
to go forward, ensuring back/forward state is reset if user clicks stop.
* http/tests/navigation/back-twice-without-commit-expected.txt: Added.
* http/tests/navigation/back-twice-without-commit.html: Added.
* http/tests/navigation/forward-and-cancel-expected.txt:
* http/tests/navigation/forward-and-cancel.html: Go forward after stop, not back.
* http/tests/navigation/resources/back-twice-page-2.html: Added.
* http/tests/navigation/resources/back-twice-page-3.html: Added.
* http/tests/navigation/resources/forward-and-cancel-frames.html: Reduced delay.
2011-01-20 Mihai Parparita <mihaip@chromium.org>
Reviewed by Eric Seidel.
......
This test checks that going back twice without committing doesn't corrupt the back/forward list.
If testing manually, click here.
============== Back Forward List ==============
curr-> http://127.0.0.1:8000/navigation/back-twice-without-commit.html **nav target**
http://127.0.0.1:8000/navigation/resources/back-twice-page-2.html **nav target**
http://127.0.0.1:8000/navigation/resources/back-twice-page-3.html **nav target**
===============================================
<script>
if (window.layoutTestController) {
if (!window.localStorage.page1Started) {
// On the first visit, clear the back forward list to start fresh,
// then set up the test.
window.localStorage.page1Started = true;
layoutTestController.clearBackForwardList();
layoutTestController.dumpBackForwardList();
layoutTestController.dumpAsText();
// Visit two pages, then go back to page 2, which has a slow frame the
// second time, and then back to page 1 before page 2 commits.
layoutTestController.queueLoad("resources/back-twice-page-2.html");
layoutTestController.queueLoad("resources/back-twice-page-3.html");
layoutTestController.queueBackNavigation(1);
layoutTestController.queueBackNavigation(1);
} else {
delete window.localStorage.page1Started;
}
}
</script>
<p>This test checks that going back twice without committing doesn't corrupt the back/forward list.
<p>If testing manually, <a href="resources/back-twice-page-2.html">click here</a>.
This test checks that the backForward list is not corrupted when a frame load is canceled.
If testing manually, click here.
============== Back Forward List ==============
curr-> http://127.0.0.1:8000/navigation/forward-and-cancel.html **nav target**
http://127.0.0.1:8000/navigation/forward-and-cancel.html **nav target**
http://127.0.0.1:8000/navigation/resources/forward-and-cancel-frames-container.html **nav target**
http://127.0.0.1:8000/navigation/resources/forward-and-cancel-frames.html (in frame "<!--framePath //<!--frame0-->-->")
about:blank (in frame "frame1")
http://127.0.0.1:8000/navigation/resources/otherpage.html (in frame "<!--framePath //<!--frame1-->-->")
http://127.0.0.1:8000/navigation/resources/forward-and-cancel-frames-container.html
curr-> http://127.0.0.1:8000/navigation/resources/forward-and-cancel-frames-container.html
http://127.0.0.1:8000/navigation/resources/forward-and-cancel-frames.html (in frame "<!--framePath //<!--frame0-->-->")
http://127.0.0.1:8000/navigation/resources/slow-resource-1-sec.pl (in frame "frame1") **nav target**
http://127.0.0.1:8000/navigation/resources/slow-resource.pl?delay=250 (in frame "frame1") **nav target**
http://127.0.0.1:8000/navigation/resources/otherpage.html (in frame "<!--framePath //<!--frame1-->-->")
===============================================
......@@ -7,9 +7,10 @@
// Important to use about:blank, which can commit immediately while walking the tree.
// 5. Go forward to slow URL, but stop before the navigation commits.
// Important to cancel the load and ensure the history is not corrupted.
// 6. Go back to start page with no frames.
// Important for testing that subframes can be removed.
// 6. Go forward and let slow URL load.
// Important for testing that navigation state is reset after stopping.
if (window.layoutTestController) {
layoutTestController.clearBackForwardList();
layoutTestController.dumpBackForwardList();
layoutTestController.dumpAsText();
layoutTestController.queueLoad("resources/forward-and-cancel-frames-container.html");
......@@ -21,7 +22,8 @@ if (window.layoutTestController) {
layoutTestController.queueNonLoadingScript("setTimeout('history.forward();',0); setTimeout('window.stop();',10);");
// Now go back to make sure the backForwardList is not corrupted.
layoutTestController.queueNonLoadingScript("setTimeout('history.back();',50);");
layoutTestController.queueNonLoadingScript("setTimeout('history.forward();',50);");
layoutTestController.queueNonLoadingScript("setTimeout('layoutTestController.notifyDone();',100);");
// Wait until we get back to this page.
layoutTestController.queueLoadingScript("layoutTestController.waitUntilDone();");
......@@ -29,15 +31,3 @@ if (window.layoutTestController) {
</script>
<p>This test checks that the backForward list is not corrupted when a frame load is canceled.
<p>If testing manually, <a href="resources/forward-and-cancel-frames-container.html">click here</a>.
<script>
if (window.layoutTestController) {
// Only notify done when we return to this page a second time.
if (!window.localStorage.started) {
window.localStorage.started = true;
} else {
delete window.localStorage.started;
layoutTestController.notifyDone();
}
}
</script>
<p>Page 2.
<p>This test checks that going back twice without committing doesn't corrupt the back/forward list.
<p>If testing manually, <a href="back-twice-page-3.html">click here</a>.
<script>
if (!window.localStorage.page2Started) {
window.localStorage.page2Started = true;
} else {
delete window.localStorage.page2Started;
// The second time we visit the page (i.e., while going back), insert an
// iframe that doesn't commit during the test.
var f = document.createElement("iframe");
f.src = "../../history/resources/back-during-onload-hung-page.php";
document.body.appendChild(f);
// Now go back. This will compete with the second queueBackNavigation
// from back-twice-without-commit.html.
history.back();
}
</script>
<p>Page 3.
<p>This test checks that going back twice without committing doesn't corrupt the back/forward list.
<p>If testing manually, hold down the back keyboard shortcut or click back twice quickly.
......@@ -12,4 +12,4 @@ function clickLink() {
<iframe id="frame1" src="about:blank"></iframe>
<br>
<p>This test checks that the backForward list is not corrupted when a frame load is canceled.
<p>If testing manually, <a id="link" href="slow-resource-1-sec.pl" target="frame1">click here</a> and then Back. Then click Forward and quickly click Stop. Ensure that Back and Forward still work.
<p>If testing manually, <a id="link" href="slow-resource.pl?delay=250" target="frame1">click here</a> and then Back. Then click Forward and quickly click Stop. Ensure that Back and Forward still work.
2011-01-21 Charlie Reis <creis@chromium.org>
Reviewed by Darin Fisher.
FrameLoader::checkLoadCompleteForThisFrame uses wrong history item
https://bugs.webkit.org/show_bug.cgi?id=48812
Most calls to stopAllLoaders now clear the history's provisional item(s).
We can now avoid resetting the back/forward state if a new navigation
is in progress.
Test: http/tests/navigation/back-twice-without-commit.html
Test: http/tests/navigation/forward-and-cancel.html
* loader/FrameLoader.cpp:
* loader/FrameLoader.h:
* loader/FrameLoaderTypes.h:
* WebCore.exp.in: Update stopAllLoaders signature.
2011-01-21 Carlos Garcia Campos <cgarcia@igalia.com>
Reviewed by Martin Robinson.
......@@ -163,7 +163,7 @@ __ZN7WebCore11FrameLoader11shouldCloseEv
__ZN7WebCore11FrameLoader11urlSelectedERKNS_4KURLERKN3WTF6StringENS4_10PassRefPtrINS_5EventEEEbbNS_14ReferrerPolicyE
__ZN7WebCore11FrameLoader12shouldReloadERKNS_4KURLES3_
__ZN7WebCore11FrameLoader14detachChildrenEv
__ZN7WebCore11FrameLoader14stopAllLoadersENS_14DatabasePolicyE
__ZN7WebCore11FrameLoader14stopAllLoadersENS_14DatabasePolicyENS_26ClearProvisionalItemPolicyE
__ZN7WebCore11FrameLoader16detachFromParentEv
__ZN7WebCore11FrameLoader16loadFrameRequestERKNS_16FrameLoadRequestEbbN3WTF10PassRefPtrINS_5EventEEENS5_INS_9FormStateEEENS_14ReferrerPolicyE
__ZN7WebCore11FrameLoader17stopForUserCancelEb
......
......@@ -1682,13 +1682,13 @@ bool FrameLoader::shouldAllowNavigation(Frame* targetFrame) const
return false;
}
void FrameLoader::stopLoadingSubframes()
void FrameLoader::stopLoadingSubframes(DatabasePolicy databasePolicy, ClearProvisionalItemPolicy clearProvisionalItemPolicy)
{
for (RefPtr<Frame> child = m_frame->tree()->firstChild(); child; child = child->tree()->nextSibling())
child->loader()->stopAllLoaders();
child->loader()->stopAllLoaders(databasePolicy, clearProvisionalItemPolicy);
}
void FrameLoader::stopAllLoaders(DatabasePolicy databasePolicy)
void FrameLoader::stopAllLoaders(DatabasePolicy databasePolicy, ClearProvisionalItemPolicy clearProvisionalItemPolicy)
{
ASSERT(!m_frame->document() || !m_frame->document()->inPageCache());
if (m_pageDismissalEventBeingDispatched)
......@@ -1702,7 +1702,12 @@ void FrameLoader::stopAllLoaders(DatabasePolicy databasePolicy)
policyChecker()->stopCheck();
stopLoadingSubframes();
// If no new load is in progress, we should clear the provisional item from history
// before we call stopLoading.
if (clearProvisionalItemPolicy == ShouldClearProvisionalItem)
history()->setProvisionalItem(0);
stopLoadingSubframes(databasePolicy, clearProvisionalItemPolicy);
if (m_provisionalDocumentLoader)
m_provisionalDocumentLoader->stopLoading(databasePolicy);
if (m_documentLoader)
......@@ -2353,7 +2358,8 @@ void FrameLoader::checkLoadCompleteForThisFrame()
// Reset the back forward list to the last committed history item at the top level.
item = page->mainFrame()->loader()->history()->currentItem();
bool shouldReset = true;
// Only reset if we aren't already going to a new provisional item.
bool shouldReset = !history()->provisionalItem();
if (!(pdl->isLoadingInAPISense() && !pdl->isStopping())) {
m_delegateIsHandlingProvisionalLoadError = true;
m_client->dispatchDidFailProvisionalLoad(error);
......@@ -2362,7 +2368,7 @@ void FrameLoader::checkLoadCompleteForThisFrame()
// FIXME: can stopping loading here possibly have any effect, if isLoading is false,
// which it must be to be in this branch of the if? And is it OK to just do a full-on
// stopAllLoaders instead of stopLoadingSubframes?
stopLoadingSubframes();
stopLoadingSubframes(DatabasePolicyStop, ShouldNotClearProvisionalItem);
pdl->stopLoading();
// If we're in the middle of loading multipart data, we need to restore the document loader.
......@@ -2964,7 +2970,8 @@ void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest&, Pass
}
FrameLoadType type = policyChecker()->loadType();
stopAllLoaders();
// A new navigation is in progress, so don't clear the history's provisional item.
stopAllLoaders(DatabasePolicyStop, ShouldNotClearProvisionalItem);
// <rdar://problem/6250856> - In certain circumstances on pages with multiple frames, stopAllLoaders()
// might detach the current FrameLoader, in which case we should bail on this newly defunct load.
......
......@@ -128,7 +128,7 @@ public:
bool canHandleRequest(const ResourceRequest&);
// Also not cool.
void stopAllLoaders(DatabasePolicy = DatabasePolicyStop);
void stopAllLoaders(DatabasePolicy = DatabasePolicyStop, ClearProvisionalItemPolicy = ShouldClearProvisionalItem);
void stopForUserCancel(bool deferCheckLoadComplete = false);
bool isLoadingMainResource() const { return m_isLoadingMainResource; }
......@@ -355,7 +355,7 @@ private:
void addExtraFieldsToRequest(ResourceRequest&, FrameLoadType loadType, bool isMainResource, bool cookiePolicyURLFromRequest);
// Also not cool.
void stopLoadingSubframes();
void stopLoadingSubframes(DatabasePolicy, ClearProvisionalItemPolicy);
void clearProvisionalLoad();
void markLoadComplete();
......
......@@ -73,6 +73,11 @@ namespace WebCore {
DatabasePolicyStop, // The database thread should be stopped and database connections closed.
DatabasePolicyContinue
};
enum ClearProvisionalItemPolicy {
ShouldClearProvisionalItem,
ShouldNotClearProvisionalItem
};
enum ObjectContentType {
ObjectContentNone,
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment