[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.21-584-g1e41756
kov at webkit.org
kov at webkit.org
Fri Feb 26 22:17:12 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit b5dbeb30d7d74755ebf4b1270ab3e56d0adf1a91
Author: kov at webkit.org <kov at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Feb 10 21:26:31 2010 +0000
WebCore
2010-02-10 Gustavo Noronha Silva <gns at gnome.org>
Reviewed by Xan Lopez.
[GTK] Hits assertion on history back, with page cache enabled, in specific conditions
https://bugs.webkit.org/show_bug.cgi?id=34773
When unsetting the adjustments from a ScrollView, also disconnect
them from the Scrollbars.
Test: fast/frames/frame-crash-with-page-cache.html
* platform/gtk/ScrollViewGtk.cpp:
(WebCore::ScrollView::setGtkAdjustments):
* platform/gtk/ScrollbarGtk.cpp:
(ScrollbarGtk::~ScrollbarGtk):
(ScrollbarGtk::detachAdjustment):
* platform/gtk/ScrollbarGtk.h:
LayoutTests
2010-02-10 Gustavo Noronha Silva <gustavo.noronha at collabora.co.uk>
Reviewed by Xan Lopez.
[GTK] Hits assertion on history back, with page cache enabled, in specific conditions
https://bugs.webkit.org/show_bug.cgi?id=34773
* fast/frames/frame-crash-with-page-cache.html: Added.
* fast/frames/resources/cached-page-1.html: Added.
* fast/frames/resources/cached-page-2.html: Added.
* fast/frames/resources/cached-page-3.html: Added.
* fast/frames/resources/cached-page-iframe.html: Added.
WebKit/gtk
2010-02-09 Gustavo Noronha Silva <gns at gnome.org>
Reviewed by Xan Lopez.
[GTK] Hits assertion on history back, with page cache enabled, in specific conditions
https://bugs.webkit.org/show_bug.cgi?id=34773
Make sure cached frames have their scrollbars disconnected from
the WebView's adjustments.
* WebCoreSupport/FrameLoaderClientGtk.cpp:
(WebKit::FrameLoaderClient::savePlatformDataToCachedFrame):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@54620 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 1e1f0f8..3d5e9cd 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2010-02-10 Gustavo Noronha Silva <gustavo.noronha at collabora.co.uk>
+
+ Reviewed by Xan Lopez.
+
+ [GTK] Hits assertion on history back, with page cache enabled, in specific conditions
+ https://bugs.webkit.org/show_bug.cgi?id=34773
+
+ * fast/frames/frame-crash-with-page-cache.html: Added.
+ * fast/frames/resources/cached-page-1.html: Added.
+ * fast/frames/resources/cached-page-2.html: Added.
+ * fast/frames/resources/cached-page-3.html: Added.
+ * fast/frames/resources/cached-page-iframe.html: Added.
+
2010-02-09 Alexey Proskuryakov <ap at apple.com>
Reviewed by Geoffrey Garen.
diff --git a/LayoutTests/fast/frames/frame-crash-with-page-cache-expected.txt b/LayoutTests/fast/frames/frame-crash-with-page-cache-expected.txt
new file mode 100644
index 0000000..6dcdd22
--- /dev/null
+++ b/LayoutTests/fast/frames/frame-crash-with-page-cache-expected.txt
@@ -0,0 +1,11 @@
+main frame - has 1 onunload handler(s)
+main frame - has 1 onunload handler(s)
+If WebKit does not assert or crash, you passed.
+open page-1
+page-1, about to navigate to page-2.
+page-1 running unload handler
+page-2, about to navigate to page-3.
+On page-3, going back, now.
+Back on page-2, timer is still firing.
+PASS.
+
diff --git a/LayoutTests/fast/frames/frame-crash-with-page-cache.html b/LayoutTests/fast/frames/frame-crash-with-page-cache.html
new file mode 100644
index 0000000..3621f24
--- /dev/null
+++ b/LayoutTests/fast/frames/frame-crash-with-page-cache.html
@@ -0,0 +1,34 @@
+<html>
+<script>
+window.finish = function()
+{
+ if (layoutTestController)
+ layoutTestController.notifyDone();
+}
+
+window.log = function(message) {
+ document.getElementById("result").innerHTML += message + "<br>";
+}
+
+window.failure = function(message) {
+ log("FAIL: " + message);
+ finish();
+}
+
+function test() {
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ layoutTestController.setCanOpenWindows();
+ layoutTestController.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+ }
+ log("open page-1");
+ window.open("resources/cached-page-1.html");
+}
+</script>
+
+<body onload="test()">
+If WebKit does not assert or crash, you passed.
+<div id="result"></div>
+</body>
+</html>
diff --git a/LayoutTests/fast/frames/resources/cached-page-1.html b/LayoutTests/fast/frames/resources/cached-page-1.html
new file mode 100644
index 0000000..610c8d4
--- /dev/null
+++ b/LayoutTests/fast/frames/resources/cached-page-1.html
@@ -0,0 +1,32 @@
+<html>
+<head>
+<script>
+var intervalId;
+
+function endTest() {
+ window.opener.log("PASS.");
+ window.opener.finish();
+}
+
+function loadNext() {
+ if (window.opener.canExit2) {
+ endTest();
+ return;
+ }
+
+ intervalId = setInterval(endTest, 100);
+
+ window.opener.log("page-1, about to navigate to page-2.")
+ location.href = "cached-page-2.html";
+}
+
+// This unload handler exists just to make sure this page is not added
+// to the page cache!
+function myUnload() {
+ window.opener.log("page-1 running unload handler");
+}
+</script>
+</head>
+<body onload="loadNext()" onunload="myUnload()">
+</body>
+</html>
diff --git a/LayoutTests/fast/frames/resources/cached-page-2.html b/LayoutTests/fast/frames/resources/cached-page-2.html
new file mode 100644
index 0000000..fd4f272
--- /dev/null
+++ b/LayoutTests/fast/frames/resources/cached-page-2.html
@@ -0,0 +1,27 @@
+<script>
+var intervalId;
+
+function goBack() {
+ if (!window.opener.canExit3)
+ return;
+
+ clearInterval(intervalId);
+ window.opener.log("Back on page-2, timer is still firing.");
+ history.back();
+}
+
+function loadNext() {
+ window.opener.canExit2 = true;
+
+ // The crash only happens when we scroll here!
+ window.scrollBy(0, 100);
+
+ intervalId = setInterval(goBack, 20);
+
+ window.opener.log("page-2, about to navigate to page-3.")
+ location.href = "cached-page-3.html";
+}
+</script>
+<body onload="loadNext()">
+<div style="height: 1200px; background-color: green"></div>
+</body>
diff --git a/LayoutTests/fast/frames/resources/cached-page-3.html b/LayoutTests/fast/frames/resources/cached-page-3.html
new file mode 100644
index 0000000..54c6388
--- /dev/null
+++ b/LayoutTests/fast/frames/resources/cached-page-3.html
@@ -0,0 +1,10 @@
+<script>
+function goBack() {
+ window.opener.canExit3 = true;
+ window.opener.log("On page-3, going back, now.");
+ history.back();
+}
+</script>
+<body>
+<iframe src="cached-page-iframe.html" onload="goBack()"></iframe>
+</body>
diff --git a/LayoutTests/fast/frames/resources/cached-page-iframe.html b/LayoutTests/fast/frames/resources/cached-page-iframe.html
new file mode 100644
index 0000000..4e11948
--- /dev/null
+++ b/LayoutTests/fast/frames/resources/cached-page-iframe.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+<div style="height: 1200px; background-color: green"></div>
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 9ea9bef..c34861c 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2010-02-10 Gustavo Noronha Silva <gns at gnome.org>
+
+ Reviewed by Xan Lopez.
+
+ [GTK] Hits assertion on history back, with page cache enabled, in specific conditions
+ https://bugs.webkit.org/show_bug.cgi?id=34773
+
+ When unsetting the adjustments from a ScrollView, also disconnect
+ them from the Scrollbars.
+
+ Test: fast/frames/frame-crash-with-page-cache.html
+
+ * platform/gtk/ScrollViewGtk.cpp:
+ (WebCore::ScrollView::setGtkAdjustments):
+ * platform/gtk/ScrollbarGtk.cpp:
+ (ScrollbarGtk::~ScrollbarGtk):
+ (ScrollbarGtk::detachAdjustment):
+ * platform/gtk/ScrollbarGtk.h:
+
2010-02-09 Alexey Proskuryakov <ap at apple.com>
Reviewed by Geoffrey Garen.
diff --git a/WebCore/platform/ScrollView.h b/WebCore/platform/ScrollView.h
index a7173a7..111d6b6 100644
--- a/WebCore/platform/ScrollView.h
+++ b/WebCore/platform/ScrollView.h
@@ -314,7 +314,7 @@ private:
#if PLATFORM(GTK)
public:
- void setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj);
+ void setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj, bool resetValues = true);
GtkAdjustment* m_horizontalAdjustment;
GtkAdjustment* m_verticalAdjustment;
void setScrollOffset(const IntSize& offset) { m_scrollOffset = offset; }
diff --git a/WebCore/platform/gtk/ScrollViewGtk.cpp b/WebCore/platform/gtk/ScrollViewGtk.cpp
index a7e7e15..e868250 100644
--- a/WebCore/platform/gtk/ScrollViewGtk.cpp
+++ b/WebCore/platform/gtk/ScrollViewGtk.cpp
@@ -2,7 +2,7 @@
* Copyright (C) 2006, 2007, 2008 Apple Computer, Inc. All rights reserved.
* Copyright (C) 2006 Michael Emmel mike.emmel at gmail.com
* Copyright (C) 2007, 2009 Holger Hans Peter Freyther
- * Copyright (C) 2008 Collabora Ltd.
+ * Copyright (C) 2008, 2010 Collabora Ltd.
*
* All rights reserved.
*
@@ -76,7 +76,7 @@ PassRefPtr<Scrollbar> ScrollView::createScrollbar(ScrollbarOrientation orientati
* The following is assumed:
* (hadj && vadj) || (!hadj && !vadj)
*/
-void ScrollView::setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj)
+void ScrollView::setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj, bool resetValues)
{
ASSERT(!hadj == !vadj);
@@ -85,17 +85,40 @@ void ScrollView::setGtkAdjustments(GtkAdjustment* hadj, GtkAdjustment* vadj)
// Reset the adjustments to a sane default
if (m_horizontalAdjustment) {
+ ScrollbarGtk* hScrollbar = reinterpret_cast<ScrollbarGtk*>(horizontalScrollbar());
+ if (hScrollbar)
+ hScrollbar->attachAdjustment(m_horizontalAdjustment);
+
+ ScrollbarGtk* vScrollbar = reinterpret_cast<ScrollbarGtk*>(verticalScrollbar());
+ if (vScrollbar)
+ vScrollbar->attachAdjustment(m_verticalAdjustment);
+
+ // We used to reset everything to 0 here, but when page cache
+ // is enabled we reuse FrameViews that are cached. Since their
+ // size is not going to change when being restored, (which is
+ // what would cause the upper limit in the adjusments to be
+ // set in the normal case), we make sure they are up-to-date
+ // here. This is needed for the parent scrolling widget to be
+ // able to report correct values.
m_horizontalAdjustment->lower = 0;
- m_horizontalAdjustment->upper = 0;
- m_horizontalAdjustment->value = 0;
+ m_horizontalAdjustment->upper = resetValues ? 0 : frameRect().width();
+ m_horizontalAdjustment->value = resetValues ? 0 : scrollOffset().width();
gtk_adjustment_changed(m_horizontalAdjustment);
gtk_adjustment_value_changed(m_horizontalAdjustment);
m_verticalAdjustment->lower = 0;
- m_verticalAdjustment->upper = 0;
- m_verticalAdjustment->value = 0;
+ m_verticalAdjustment->upper = resetValues ? 0 : frameRect().height();
+ m_verticalAdjustment->value = resetValues ? 0 : scrollOffset().height();
gtk_adjustment_changed(m_verticalAdjustment);
gtk_adjustment_value_changed(m_verticalAdjustment);
+ } else {
+ ScrollbarGtk* hScrollbar = reinterpret_cast<ScrollbarGtk*>(horizontalScrollbar());
+ if (hScrollbar)
+ hScrollbar->detachAdjustment();
+
+ ScrollbarGtk* vScrollbar = reinterpret_cast<ScrollbarGtk*>(verticalScrollbar());
+ if (vScrollbar)
+ vScrollbar->detachAdjustment();
}
/* reconsider having a scrollbar */
diff --git a/WebCore/platform/gtk/ScrollbarGtk.cpp b/WebCore/platform/gtk/ScrollbarGtk.cpp
index 00c6ea0..0c3037a 100644
--- a/WebCore/platform/gtk/ScrollbarGtk.cpp
+++ b/WebCore/platform/gtk/ScrollbarGtk.cpp
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2007, 2009 Holger Hans Peter Freyther zecke at selfish.org
+ * Copyright (C) 2010 Gustavo Noronha Silva <gns at gnome.org>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -87,6 +88,32 @@ ScrollbarGtk::ScrollbarGtk(ScrollbarClient* client, ScrollbarOrientation orienta
ScrollbarGtk::~ScrollbarGtk()
{
+ if (m_adjustment)
+ detachAdjustment();
+}
+
+void ScrollbarGtk::attachAdjustment(GtkAdjustment* adjustment)
+{
+ if (platformWidget())
+ return;
+
+ if (m_adjustment)
+ detachAdjustment();
+
+ m_adjustment = adjustment;
+
+ g_object_ref(m_adjustment);
+ g_signal_connect(m_adjustment, "value-changed", G_CALLBACK(ScrollbarGtk::gtkValueChanged), this);
+
+ updateThumbProportion();
+ updateThumbPosition();
+}
+
+void ScrollbarGtk::detachAdjustment()
+{
+ if (!m_adjustment)
+ return;
+
g_signal_handlers_disconnect_by_func(G_OBJECT(m_adjustment), (gpointer)ScrollbarGtk::gtkValueChanged, this);
// For the case where we only operate on the GtkAdjustment it is best to
@@ -98,6 +125,7 @@ ScrollbarGtk::~ScrollbarGtk()
gtk_adjustment_changed(m_adjustment);
gtk_adjustment_value_changed(m_adjustment);
g_object_unref(m_adjustment);
+ m_adjustment = 0;
}
IntPoint ScrollbarGtk::getLocationInParentWindow(const IntRect& rect)
diff --git a/WebCore/platform/gtk/ScrollbarGtk.h b/WebCore/platform/gtk/ScrollbarGtk.h
index b4b5989..e02bb50 100644
--- a/WebCore/platform/gtk/ScrollbarGtk.h
+++ b/WebCore/platform/gtk/ScrollbarGtk.h
@@ -59,7 +59,9 @@ protected:
virtual void updateThumbPosition();
virtual void updateThumbProportion();
-
+
+ void detachAdjustment();
+ void attachAdjustment(GtkAdjustment*);
private:
static void gtkValueChanged(GtkAdjustment*, ScrollbarGtk*);
IntPoint getLocationInParentWindow(const IntRect&);
diff --git a/WebKit/gtk/ChangeLog b/WebKit/gtk/ChangeLog
index b0c768a..7a44884 100644
--- a/WebKit/gtk/ChangeLog
+++ b/WebKit/gtk/ChangeLog
@@ -1,3 +1,16 @@
+2010-02-09 Gustavo Noronha Silva <gns at gnome.org>
+
+ Reviewed by Xan Lopez.
+
+ [GTK] Hits assertion on history back, with page cache enabled, in specific conditions
+ https://bugs.webkit.org/show_bug.cgi?id=34773
+
+ Make sure cached frames have their scrollbars disconnected from
+ the WebView's adjustments.
+
+ * WebCoreSupport/FrameLoaderClientGtk.cpp:
+ (WebKit::FrameLoaderClient::savePlatformDataToCachedFrame):
+
2010-02-09 Gustavo Noronha Silva <gustavo.noronha at collabora.co.uk>
Reviewed by Xan Lopez.
diff --git a/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp b/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
index 143a634..1ccc8a1 100644
--- a/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
+++ b/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
@@ -3,7 +3,7 @@
* Copyright (C) 2007, 2008, 2009 Holger Hans Peter Freyther
* Copyright (C) 2007 Christian Dywan <christian at twotoasts.de>
* Copyright (C) 2008, 2009 Collabora Ltd. All rights reserved.
- * Copyright (C) 2009 Gustavo Noronha Silva <gns at gnome.org>
+ * Copyright (C) 2009, 2010 Gustavo Noronha Silva <gns at gnome.org>
* Copyright (C) Research In Motion Limited 2009. All rights reserved.
*
* This library is free software; you can redistribute it and/or
@@ -646,9 +646,7 @@ void FrameLoaderClient::setCopiesOnScroll()
void FrameLoaderClient::detachedFromParent2()
{
- FrameView *view = core(m_frame)->view();
- if (view)
- view->setGtkAdjustments(0, 0);
+ notImplemented();
}
void FrameLoaderClient::detachedFromParent3()
@@ -1116,15 +1114,21 @@ void FrameLoaderClient::updateGlobalHistoryRedirectLinks()
notImplemented();
}
-void FrameLoaderClient::savePlatformDataToCachedFrame(CachedFrame*)
+void FrameLoaderClient::savePlatformDataToCachedFrame(CachedFrame* cachedFrame)
{
+ // We need to do this here in order to disconnect the scrollbars
+ // that are being used by the frame that is being cached from the
+ // adjustments, otherwise they will react to changes in the
+ // adjustments, and bad things will happen.
+ if (cachedFrame->view())
+ cachedFrame->view()->setGtkAdjustments(0, 0);
}
-static void postCommitFrameViewSetup(WebKitWebFrame *frame, FrameView *view)
+static void postCommitFrameViewSetup(WebKitWebFrame *frame, FrameView *view, bool resetValues)
{
WebKitWebView* containingWindow = getViewFromFrame(frame);
WebKitWebViewPrivate* priv = WEBKIT_WEB_VIEW_GET_PRIVATE(containingWindow);
- view->setGtkAdjustments(priv->horizontalAdjustment, priv->verticalAdjustment);
+ view->setGtkAdjustments(priv->horizontalAdjustment, priv->verticalAdjustment, resetValues);
if (priv->currentMenu) {
GtkMenu* menu = priv->currentMenu;
@@ -1143,7 +1147,7 @@ void FrameLoaderClient::transitionToCommittedFromCachedFrame(CachedFrame* cached
if (frame != frame->page()->mainFrame())
return;
- postCommitFrameViewSetup(m_frame, cachedFrame->view());
+ postCommitFrameViewSetup(m_frame, cachedFrame->view(), false);
}
void FrameLoaderClient::transitionToCommittedForNewPage()
@@ -1162,7 +1166,7 @@ void FrameLoaderClient::transitionToCommittedForNewPage()
if (frame != frame->page()->mainFrame())
return;
- postCommitFrameViewSetup(m_frame, frame->view());
+ postCommitFrameViewSetup(m_frame, frame->view(), true);
}
}
diff --git a/WebKit/gtk/tests/testwebview.c b/WebKit/gtk/tests/testwebview.c
index e37e2dd..c028a36 100644
--- a/WebKit/gtk/tests/testwebview.c
+++ b/WebKit/gtk/tests/testwebview.c
@@ -201,18 +201,15 @@ static void do_test_webkit_web_view_adjustments(gboolean with_page_cache)
if (!with_page_cache)
g_main_loop_run(loop);
+ /* Make sure GTK+ has time to process the changes in size, for the adjusments */
+ while (gtk_events_pending())
+ gtk_main_iteration();
+
/* Make sure upper and lower bounds have been restored correctly */
g_assert_cmpfloat(lower, ==, gtk_adjustment_get_lower(adjustment));
g_assert_cmpfloat(upper, ==, gtk_adjustment_get_upper(adjustment));
- /* Scrollbar should return to the same position it was in.
- * FIXME: with page cache enabled, the result is different, we
- * need to figure out why!
- */
- if (!with_page_cache)
- g_assert_cmpfloat(gtk_adjustment_get_value(adjustment), ==, 100.0);
- else
- g_assert_cmpfloat(gtk_adjustment_get_value(adjustment), ==, 20.0);
+ g_assert_cmpfloat(gtk_adjustment_get_value(adjustment), ==, 100.0);
g_free(effective_uri);
g_free(second_uri);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list