[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
darin at apple.com
darin at apple.com
Thu Apr 8 00:53:47 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 3010dc88df3a89d86a2b140ceca87ed505385f62
Author: darin at apple.com <darin at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Jan 4 21:49:01 2010 +0000
WebCore: Reentrancy problem with selection in some edge cases.
https://bugs.webkit.org/show_bug.cgi?id=32842
rdar://problem/7449974
Reviewed by Dan Bernstein.
Test: fast/forms/selection-layout-reentry-strange-case.html
Move the selection display update process done in the
selectionLayoutChanged function into the layout timer
instead of doing it immediately when selection changes occur.
* dom/Document.cpp:
(WebCore::Document::updateLayout): Changed this to use the
definition of needsLayout from FrameView rather than rolling
its own.
* editing/SelectionController.cpp:
(WebCore::SelectionController::SelectionController):
Initialize m_needsDisplayUpdate to false.
(WebCore::SelectionController::setSelection): Call
the new setNeedsDisplayUpdate function instead of the old
badly named Frame::selectionLayoutChanged function.
(WebCore::SelectionController::setNeedsDisplayUpdate):
Set m_needsDisplayUpdate. If it is just becoming true, then
call FrameView::scheduleRelayout.
* editing/SelectionController.h: Added setNeedsDisplayUpdate,
needsDisplayUpdate, and m_needsDisplayUpdate.
* page/Frame.cpp:
(WebCore::Frame::setCaretVisible): Call setNeedsDisplayUpdate.
(WebCore::Frame::selectionLayoutChanged): Call
setNeedsDisplayUpdate to set it to false, since this is the
function that performs "selection display update". Later I want
to rename this function. Also added a global reentrancy check
since that's the easiest way I could think of to prevent infinite
recursion in the case where recomputeCaretRect ends up triggering
a layout. As a side effect, fixed theoretical problems in
TEXT_CARET builds by improving the ifdef.
(WebCore::Frame::caretBlinkTimerFired): Loosen assertions a
bit. Later we might want to decouple caret rect from caret state
a bit more and add these assertions back.
(WebCore::Frame::selectionBounds): Call Document::updateLayout.
This function is for external clients and they do not currently
do anything to make sure layout is up to date.
* page/FrameView.cpp:
(WebCore::FrameView::needsLayout): Add a new clause, since
we need a call to layout if needsDisplayUpdate is true.
LayoutTests: Reentrancy problem with selection in some edge cases.
https://bugs.webkit.org/show_bug.cgi?id=32842
rdar://problem/7449974
Reviewed by Dan Bernstein.
* fast/forms/selection-layout-reentry-strange-case-expected.txt: Added.
* fast/forms/selection-layout-reentry-strange-case.html: Added.
* platform/mac/accessibility/frame-with-title-expected.txt: Updated since
the number of layouts is now different.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52756 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index eedb526..d2303f2 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2010-01-04 Darin Adler <darin at apple.com>
+
+ Reviewed by Dan Bernstein.
+
+ Reentrancy problem with selection in some edge cases.
+ https://bugs.webkit.org/show_bug.cgi?id=32842
+ rdar://problem/7449974
+
+ * fast/forms/selection-layout-reentry-strange-case-expected.txt: Added.
+ * fast/forms/selection-layout-reentry-strange-case.html: Added.
+ * platform/mac/accessibility/frame-with-title-expected.txt: Updated since
+ the number of layouts is now different.
+
2010-01-04 Dmitry Titov <dimich at chromium.org>
Not reviewed, revert r52745 and r52746 on behalf of Nikolas Zimmermann, as discussed on IRC.
diff --git a/LayoutTests/fast/forms/selection-layout-reentry-strange-case-expected.txt b/LayoutTests/fast/forms/selection-layout-reentry-strange-case-expected.txt
new file mode 100644
index 0000000..811b710
--- /dev/null
+++ b/LayoutTests/fast/forms/selection-layout-reentry-strange-case-expected.txt
@@ -0,0 +1,2 @@
+t down
+Yes, this is a strange test case. If you can see this and it did not crash, then all is well.
diff --git a/LayoutTests/fast/forms/selection-layout-reentry-strange-case.html b/LayoutTests/fast/forms/selection-layout-reentry-strange-case.html
new file mode 100644
index 0000000..72b96c6
--- /dev/null
+++ b/LayoutTests/fast/forms/selection-layout-reentry-strange-case.html
@@ -0,0 +1,19 @@
+<textarea id="container">si
+<DIV abcdefgh_POSIT></DIV>
+</textarea>t down
+
+<script>
+ document.execCommand("FindString", true, "sit");
+ document.execCommand("FindString", true, "down");
+</script>
+
+<style></style>
+
+<script type="text/javascript">
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+ document.getElementById('container').innerHTML="";
+</script>
+
+<p>Yes, this is a strange test case. If you can see this and it
+did not crash, then all is well.</p>
diff --git a/LayoutTests/platform/mac/accessibility/frame-with-title-expected.txt b/LayoutTests/platform/mac/accessibility/frame-with-title-expected.txt
index 0cad128..538cfd8 100644
--- a/LayoutTests/platform/mac/accessibility/frame-with-title-expected.txt
+++ b/LayoutTests/platform/mac/accessibility/frame-with-title-expected.txt
@@ -21,7 +21,7 @@ AXBlockQuoteLevel: 0
AXTopLevelUIElement: <AXWebArea>
AXLinkUIElements: <array of size 0>
AXLoaded: 1
-AXLayoutCount: 3
+AXLayoutCount: 4
AXURL: about:blank
@@ -48,7 +48,7 @@ AXBlockQuoteLevel: 0
AXTopLevelUIElement: <AXWebArea>
AXLinkUIElements: <array of size 0>
AXLoaded: 1
-AXLayoutCount: 4
+AXLayoutCount: 5
AXURL: about:blank
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 2bad0f6..b652ca9 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,56 @@
+2010-01-04 Darin Adler <darin at apple.com>
+
+ Reviewed by Dan Bernstein.
+
+ Reentrancy problem with selection in some edge cases.
+ https://bugs.webkit.org/show_bug.cgi?id=32842
+ rdar://problem/7449974
+
+ Test: fast/forms/selection-layout-reentry-strange-case.html
+
+ Move the selection display update process done in the
+ selectionLayoutChanged function into the layout timer
+ instead of doing it immediately when selection changes occur.
+
+ * dom/Document.cpp:
+ (WebCore::Document::updateLayout): Changed this to use the
+ definition of needsLayout from FrameView rather than rolling
+ its own.
+
+ * editing/SelectionController.cpp:
+ (WebCore::SelectionController::SelectionController):
+ Initialize m_needsDisplayUpdate to false.
+ (WebCore::SelectionController::setSelection): Call
+ the new setNeedsDisplayUpdate function instead of the old
+ badly named Frame::selectionLayoutChanged function.
+ (WebCore::SelectionController::setNeedsDisplayUpdate):
+ Set m_needsDisplayUpdate. If it is just becoming true, then
+ call FrameView::scheduleRelayout.
+
+ * editing/SelectionController.h: Added setNeedsDisplayUpdate,
+ needsDisplayUpdate, and m_needsDisplayUpdate.
+
+ * page/Frame.cpp:
+ (WebCore::Frame::setCaretVisible): Call setNeedsDisplayUpdate.
+ (WebCore::Frame::selectionLayoutChanged): Call
+ setNeedsDisplayUpdate to set it to false, since this is the
+ function that performs "selection display update". Later I want
+ to rename this function. Also added a global reentrancy check
+ since that's the easiest way I could think of to prevent infinite
+ recursion in the case where recomputeCaretRect ends up triggering
+ a layout. As a side effect, fixed theoretical problems in
+ TEXT_CARET builds by improving the ifdef.
+ (WebCore::Frame::caretBlinkTimerFired): Loosen assertions a
+ bit. Later we might want to decouple caret rect from caret state
+ a bit more and add these assertions back.
+ (WebCore::Frame::selectionBounds): Call Document::updateLayout.
+ This function is for external clients and they do not currently
+ do anything to make sure layout is up to date.
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::needsLayout): Add a new clause, since
+ we need a call to layout if needsDisplayUpdate is true.
+
2010-01-04 Brent Fulgham <bfulgham at webkit.org>
Reviewed by Adam Roben.
diff --git a/WebCore/dom/Document.cpp b/WebCore/dom/Document.cpp
index 2f00d1d..fd5f118 100644
--- a/WebCore/dom/Document.cpp
+++ b/WebCore/dom/Document.cpp
@@ -3,7 +3,7 @@
* (C) 1999 Antti Koivisto (koivisto at kde.org)
* (C) 2001 Dirk Mueller (mueller at kde.org)
* (C) 2006 Alexey Proskuryakov (ap at webkit.org)
- * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
* Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
* Copyright (C) 2008, 2009 Google Inc. All rights reserved.
*
@@ -1348,9 +1348,8 @@ void Document::updateLayout()
updateStyleIfNeeded();
- // Only do a layout if changes have occurred that make it necessary.
FrameView* v = view();
- if (v && renderer() && (v->layoutPending() || renderer()->needsLayout()))
+ if (v && v->needsLayout())
v->layout();
}
diff --git a/WebCore/editing/SelectionController.cpp b/WebCore/editing/SelectionController.cpp
index 518df45..13699d7 100644
--- a/WebCore/editing/SelectionController.cpp
+++ b/WebCore/editing/SelectionController.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2004, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2008, 2009, 2010 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -70,6 +70,7 @@ SelectionController::SelectionController(Frame* frame, bool isDragCaretControlle
, m_isDragCaretController(isDragCaretController)
, m_isCaretBlinkingSuspended(false)
, m_focused(frame && frame->page() && frame->page()->focusController()->focusedFrame() == frame)
+ , m_needsDisplayUpdate(false)
{
}
@@ -145,7 +146,8 @@ void SelectionController::setSelection(const VisibleSelection& s, bool closeTypi
if (!s.isNone())
m_frame->setFocusedNodeIfNeeded();
- m_frame->selectionLayoutChanged();
+ setNeedsDisplayUpdate();
+
// Always clear the x position used for vertical arrow navigation.
// It will be restored by the vertical arrow navigation code if necessary.
m_xPosForVerticalArrowNavigation = NoXPosForVerticalArrowNavigation;
@@ -1301,6 +1303,20 @@ bool SelectionController::isFocusedAndActive() const
return m_focused && m_frame->page() && m_frame->page()->focusController()->isActive();
}
+void SelectionController::setNeedsDisplayUpdate(bool needsUpdate)
+{
+ if (m_needsDisplayUpdate == needsUpdate)
+ return;
+ m_needsDisplayUpdate = needsUpdate;
+
+ if (!m_needsDisplayUpdate)
+ return;
+ FrameView* view = m_frame->view();
+ if (!view)
+ return;
+ view->scheduleRelayout();
+}
+
#ifndef NDEBUG
void SelectionController::formatForDebugger(char* buffer, unsigned length) const
diff --git a/WebCore/editing/SelectionController.h b/WebCore/editing/SelectionController.h
index d03df52..372c28d 100644
--- a/WebCore/editing/SelectionController.h
+++ b/WebCore/editing/SelectionController.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2004 Apple Computer, Inc. All rights reserved.
+ * Copyright (C) 2004, 2009, 2010 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -124,6 +124,10 @@ public:
bool isFocusedAndActive() const;
void pageActivationChanged();
+ // Selection display machinery
+ void setNeedsDisplayUpdate(bool = true);
+ bool needsDisplayUpdate() const { return m_needsDisplayUpdate; }
+
#ifndef NDEBUG
void formatForDebugger(char* buffer, unsigned length) const;
void showTreeForThis() const;
@@ -174,7 +178,7 @@ private:
bool m_isDragCaretController : 1;
bool m_isCaretBlinkingSuspended : 1;
bool m_focused : 1;
-
+ bool m_needsDisplayUpdate : 1;
};
inline bool operator==(const SelectionController& a, const SelectionController& b)
diff --git a/WebCore/page/Frame.cpp b/WebCore/page/Frame.cpp
index bc1b1ca..5902e8e 100644
--- a/WebCore/page/Frame.cpp
+++ b/WebCore/page/Frame.cpp
@@ -5,7 +5,7 @@
* 2000 Simon Hausmann <hausmann at kde.org>
* 2000 Stefan Schimanski <1Stein at gmx.de>
* 2001 George Staikos <staikos at kde.org>
- * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
* Copyright (C) 2005 Alexey Proskuryakov <ap at nypop.com>
* Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
* Copyright (C) 2008 Eric Seidel <eric at webkit.org>
@@ -563,7 +563,7 @@ void Frame::setCaretVisible(bool flag)
return;
clearCaretRectIfNeeded();
m_caretVisible = flag;
- selectionLayoutChanged();
+ selection()->setNeedsDisplayUpdate();
}
void Frame::clearCaretRectIfNeeded()
@@ -630,9 +630,17 @@ void Frame::setFocusedNodeIfNeeded()
void Frame::selectionLayoutChanged()
{
- bool caretRectChanged = selection()->recomputeCaretRect();
+ selection()->setNeedsDisplayUpdate(false);
#if ENABLE(TEXT_CARET)
+ static bool alreadyInSelectionLayoutChanged;
+ if (alreadyInSelectionLayoutChanged)
+ return;
+
+ alreadyInSelectionLayoutChanged = true;
+
+ bool caretRectChanged = selection()->recomputeCaretRect();
+
bool caretBrowsing = settings() && settings()->caretBrowsingEnabled();
bool shouldBlink = m_caretVisible
&& selection()->isCaret() && (selection()->isContentEditable() || caretBrowsing);
@@ -653,9 +661,8 @@ void Frame::selectionLayoutChanged()
selection()->invalidateCaretRect();
}
}
-#else
- if (!caretRectChanged)
- return;
+
+ alreadyInSelectionLayoutChanged = false;
#endif
RenderView* view = contentRenderer();
@@ -691,8 +698,10 @@ void Frame::selectionLayoutChanged()
void Frame::caretBlinkTimerFired(Timer<Frame>*)
{
#if ENABLE(TEXT_CARET)
- ASSERT(m_caretVisible);
- ASSERT(selection()->isCaret());
+ if (!m_caretVisible || !selection()->isCaret()) {
+ ASSERT(selection()->needsDisplayUpdate());
+ return;
+ }
bool caretPaint = m_caretPaint;
if (selection()->isCaretBlinkingSuspended() && caretPaint)
return;
@@ -1252,6 +1261,8 @@ void Frame::setExcludeFromTextSearch(bool exclude)
// returns FloatRect because going through IntRect would truncate any floats
FloatRect Frame::selectionBounds(bool clipToVisibleContent) const
{
+ m_doc->updateLayout();
+
RenderView* root = contentRenderer();
FrameView* view = m_view.get();
if (!root || !view)
diff --git a/WebCore/page/FrameView.cpp b/WebCore/page/FrameView.cpp
index 483a220..59c5a12 100644
--- a/WebCore/page/FrameView.cpp
+++ b/WebCore/page/FrameView.cpp
@@ -3,7 +3,7 @@
* 1999 Lars Knoll <knoll at kde.org>
* 1999 Antti Koivisto <koivisto at kde.org>
* 2000 Dirk Mueller <mueller at kde.org>
- * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
* (C) 2006 Graham Dennis (graham.dennis at gmail.com)
* (C) 2006 Alexey Proskuryakov (ap at nypop.com)
* Copyright (C) 2009 Google Inc. All rights reserved.
@@ -1198,7 +1198,8 @@ bool FrameView::needsLayout() const
|| m_layoutRoot
|| (document && document->childNeedsStyleRecalc()) // can occur when using WebKit ObjC interface
|| m_frame->needsReapplyStyles()
- || (m_deferSetNeedsLayouts && m_setNeedsLayoutWasDeferred);
+ || (m_deferSetNeedsLayouts && m_setNeedsLayoutWasDeferred)
+ || m_frame->selection()->needsDisplayUpdate();
}
void FrameView::setNeedsLayout()
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list