Diff
Modified: branches/safari-536.30-branch/Source/WebCore/ChangeLog (148459 => 148460)
--- branches/safari-536.30-branch/Source/WebCore/ChangeLog 2013-04-15 20:05:18 UTC (rev 148459)
+++ branches/safari-536.30-branch/Source/WebCore/ChangeLog 2013-04-15 20:09:32 UTC (rev 148460)
@@ -70,6 +70,67 @@
2013-04-15 Roger Fong <[email protected]>
+ Merge 133840, 134191, 134197.
+
+ 2012-11-12 Ryosuke Niwa <[email protected]>
+
+ Build fix after r134191. Turns out that FrameView::performPostLayoutTasks calls FrameSelection::updateAppearance
+ in the middle of a layout. So we can't have assertions in recomputeCaretRect and updateAppearance.
+
+ Furthermore, we can't update layout in updateAppearance. So do that in its call sites.
+
+ * editing/FrameSelection.cpp:
+ (WebCore::FrameSelection::setSelection):
+ (WebCore::FrameSelection::recomputeCaretRect):
+ (WebCore::FrameSelection::updateAppearance):
+ (WebCore::FrameSelection::setCaretVisibility):
+
+ 2012-11-09 Ryosuke Niwa <[email protected]>
+
+ Multiple Layout Tests (e.g. fast/repaint/japanese-rl-selection-clear.html) is failing after r133840.
+ https://bugs.webkit.org/show_bug.cgi?id=101547
+
+ Reviewed by Simon Fraser.
+
+ I overlooked the fact when the selection is null, we still have to invalidate the caret rect that
+ previously existed. Revert the optimization added in r133840 to skip caret invalidation when new
+ selection is null, and add a special method to be called by FrameLoader prior to destruction instead.
+ This will let us avoid doing an extra layout upon destruction and not regress repaint tests.
+
+ Covered by existing tests.
+
+ * editing/FrameSelection.cpp:
+ (WebCore::FrameSelection::setSelection): Added DoNotUpdateAppearance option.
+ (WebCore::FrameSelection::prepareForDestruction): Added.
+ (WebCore::FrameSelection::updateAppearance): Reverted the flawed optimization added in r133840.
+ Also, don't update style before updating selection unless text caret is disabled since we always
+ update the layout (including style) when text caret is enabled.
+ * editing/FrameSelection.h:
+ (FrameSelection):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::clear): Call prepareForDestruction instead of clear to avoid a layout.
+
+ 2012-11-07 Ryosuke Niwa <[email protected]>
+
+ Crash in WebCore::RenderLayer::normalFlowList
+ https://bugs.webkit.org/show_bug.cgi?id=101528
+
+ Reviewed by Simon Fraser.
+
+ Make sure the layout is up to date before re-computing the caret rect.
+ Avoid doing the layout when the selection is cleared since we don't can
+ since we can always stop the blink timer in that case.
+
+ Unfortunately, we haven't found any reproduction of this crash yet.
+
+ * editing/FrameSelection.cpp:
+ (WebCore::isNonOrphanedCaret):
+ (WebCore):
+ (WebCore::FrameSelection::localCaretRect):
+ (WebCore::FrameSelection::updateAppearance):
+
+2013-04-15 Roger Fong <[email protected]>
+
Merged r138213.
2013-01-09 Abhishek Arya <[email protected]>
Modified: branches/safari-536.30-branch/Source/WebCore/editing/FrameSelection.cpp (148459 => 148460)
--- branches/safari-536.30-branch/Source/WebCore/editing/FrameSelection.cpp 2013-04-15 20:05:18 UTC (rev 148459)
+++ branches/safari-536.30-branch/Source/WebCore/editing/FrameSelection.cpp 2013-04-15 20:09:32 UTC (rev 148460)
@@ -286,9 +286,16 @@
if (!s.isNone() && !(options & DoNotSetFocus))
setFocusedNodeIfNeeded();
-
- updateAppearance();
+ if (!(options & DoNotUpdateAppearance)) {
+#if ENABLE(TEXT_CARET)
+ m_frame->document()->updateLayoutIgnorePendingStylesheets();
+#else
+ m_frame->document()->updateStyleIfNeeded();
+#endif
+ updateAppearance();
+ }
+
// 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();
@@ -1121,6 +1128,21 @@
setSelection(VisibleSelection());
}
+void FrameSelection::prepareForDestruction()
+{
+ m_granularity = CharacterGranularity;
+
+#if ENABLE(TEXT_CARET)
+ m_caretBlinkTimer.stop();
+#endif
+
+ RenderView* view = m_frame->contentRenderer();
+ if (view)
+ view->clearSelection();
+
+ setSelection(VisibleSelection(), CloseTyping | ClearTypingStyle | DoNotUpdateAppearance);
+}
+
void FrameSelection::setStart(const VisiblePosition &pos, EUserTriggered trigger)
{
if (m_selection.isBaseFirst())
@@ -1233,10 +1255,15 @@
return CaretBase::caretRenderer(m_position.deepEquivalent().deprecatedNode());
}
+static bool isNonOrphanedCaret(const VisibleSelection& selection)
+{
+ return selection.isCaret() && !selection.start().isOrphan() && !selection.end().isOrphan();
+}
+
LayoutRect FrameSelection::localCaretRect()
{
if (shouldUpdateCaretRect()) {
- if (!isCaret() || m_selection.start().isOrphan() || m_selection.end().isOrphan())
+ if (!isNonOrphanedCaret(m_selection))
clearCaretRect();
else if (updateCaretRect(m_frame->document(), VisiblePosition(m_selection.start(), m_selection.affinity())))
m_absCaretBoundsDirty = true;
@@ -1688,14 +1715,15 @@
void FrameSelection::updateAppearance()
{
#if ENABLE(TEXT_CARET)
- bool caretRectChanged = recomputeCaretRect();
+ // When selection is clear, assume
+ bool caretRectChangedOrCleared = recomputeCaretRect();
bool caretBrowsing = m_frame->settings() && m_frame->settings()->caretBrowsingEnabled();
bool shouldBlink = caretIsVisible() && isCaret() && (isContentEditable() || caretBrowsing);
// If the caret moved, stop the blink timer so we can restart with a
// black caret in the new location.
- if (caretRectChanged || !shouldBlink || shouldStopBlinkingDueToTypingCommand(m_frame))
+ if (caretRectChangedOrCleared || !shouldBlink || shouldStopBlinkingDueToTypingCommand(m_frame))
m_caretBlinkTimer.stop();
// Start blinking with a black caret. Be sure not to restart if we're
@@ -1711,9 +1739,6 @@
}
#endif
- // We need to update style in case the node containing the selection is made display:none.
- m_frame->document()->updateStyleIfNeeded();
-
RenderView* view = m_frame->contentRenderer();
if (!view)
return;
@@ -1761,6 +1786,8 @@
void FrameSelection::clearCaretRectIfNeeded()
{
#if ENABLE(TEXT_CARET)
+#else
+ m_frame->document()->updateStyleIfNeeded();
if (!m_caretPaint)
return;
m_caretPaint = false;
Modified: branches/safari-536.30-branch/Source/WebCore/editing/FrameSelection.h (148459 => 148460)
--- branches/safari-536.30-branch/Source/WebCore/editing/FrameSelection.h 2013-04-15 20:05:18 UTC (rev 148459)
+++ branches/safari-536.30-branch/Source/WebCore/editing/FrameSelection.h 2013-04-15 20:09:32 UTC (rev 148460)
@@ -118,7 +118,8 @@
ClearTypingStyle = 1 << 2,
SpellCorrectionTriggered = 1 << 3,
DoNotSetFocus = 1 << 4,
- DictationTriggered = 1 << 5
+ DictationTriggered = 1 << 5,
+ DoNotUpdateAppearance = 1 << 6,
};
typedef unsigned SetSelectionOptions; // Union of values in SetSelectionOption and EUserTriggered
static inline EUserTriggered selectionOptionsToUserTriggered(SetSelectionOptions options)
@@ -145,7 +146,8 @@
bool setSelectedRange(Range*, EAffinity, bool closeTyping);
void selectAll();
void clear();
-
+ void prepareForDestruction();
+
// Call this after doing user-triggered selections to make it easy to delete the frame you entirely selected.
void selectFrameElementInParentIfFullySelected();
Modified: branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.cpp (148459 => 148460)
--- branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.cpp 2013-04-15 20:05:18 UTC (rev 148459)
+++ branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.cpp 2013-04-15 20:09:32 UTC (rev 148460)
@@ -526,7 +526,7 @@
m_frame->script()->clearWindowShell(m_frame->document()->inPageCache());
}
- m_frame->selection()->clear();
+ m_frame->selection()->prepareForDestruction();
m_frame->eventHandler()->clear();
if (clearFrameView && m_frame->view())
m_frame->view()->clear();
