- Revision
- 134191
- Author
- [email protected]
- Date
- 2012-11-11 23:09:44 -0800 (Sun, 11 Nov 2012)
Log Message
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.
Source/WebCore:
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.
LayoutTests:
Remove Chromium test expectations as these tests now pass.
* platform/chromium/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (134190 => 134191)
--- trunk/LayoutTests/ChangeLog 2012-11-12 06:37:55 UTC (rev 134190)
+++ trunk/LayoutTests/ChangeLog 2012-11-12 07:09:44 UTC (rev 134191)
@@ -1,3 +1,14 @@
+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.
+
+ Remove Chromium test expectations as these tests now pass.
+
+ * platform/chromium/TestExpectations:
+
2012-11-11 Dongwoo Joshua Im <[email protected]>
[CSS3] Parsing the property, text-align-last.
Modified: trunk/LayoutTests/platform/chromium/TestExpectations (134190 => 134191)
--- trunk/LayoutTests/platform/chromium/TestExpectations 2012-11-12 06:37:55 UTC (rev 134190)
+++ trunk/LayoutTests/platform/chromium/TestExpectations 2012-11-12 07:09:44 UTC (rev 134191)
@@ -2884,9 +2884,9 @@
webkit.org/b/65009 [ Mac ] scrollbars/scrollbar-drag-thumb-with-large-content.html [ Failure ]
-#webkit.org/b/65124 [ Android Linux Win ] fast/repaint/japanese-rl-selection-clear.html [ Failure ]
-#webkit.org/b/65124 [ Win ] fast/repaint/japanese-rl-selection-repaint.html [ Failure ]
-#webkit.org/b/65124 [ Android Linux Win ] fast/repaint/japanese-rl-selection-repaint-in-regions.html [ Failure ]
+webkit.org/b/65124 [ Android Linux Win ] fast/repaint/japanese-rl-selection-clear.html [ Failure ]
+webkit.org/b/65124 [ Win ] fast/repaint/japanese-rl-selection-repaint.html [ Failure ]
+webkit.org/b/65124 [ Android Linux Win ] fast/repaint/japanese-rl-selection-repaint-in-regions.html [ Failure ]
webkit.org/b/65124 [ Android Linux Win ] fast/repaint/line-flow-with-floats-in-regions.html [ Failure ]
webkit.org/b/65199 [ Android Linux SnowLeopard Win ] fast/writing-mode/broken-ideograph-small-caps.html [ Failure ]
@@ -4189,30 +4189,3 @@
webkit.org/b/101539 [ Linux Win ] editing/execCommand/switch-list-type-with-orphaned-li.html [ Failure ]
webkit.org/b/101618 [ Win7 Debug ] http/tests/inspector/indexeddb/database-data.html [ Crash ]
-
-# Failures following r133840:
-webkit.org/b/101547 fast/repaint/4774354.html [ ImageOnlyFailure ]
-webkit.org/b/101547 fast/repaint/caret-outside-block.html [ ImageOnlyFailure ]
-webkit.org/b/101547 fast/repaint/delete-into-nested-block.html [ ImageOnlyFailure ]
-webkit.org/b/101547 fast/repaint/inline-outline-repaint.html [ ImageOnlyFailure ]
-# The following test has prior expectations which were disabled. Re-enable when removing this:
-webkit.org/b/101547 fast/repaint/japanese-rl-selection-clear.html [ ImageOnlyFailure Failure ]
-# The following test has prior expectations which were disabled. Re-enable when removing this:
-webkit.org/b/101547 fast/repaint/japanese-rl-selection-repaint-in-regions.html [ ImageOnlyFailure Failure ]
-# The following test has prior expectations which were disabled. Re-enable when removing this:
-webkit.org/b/101547 fast/repaint/japanese-rl-selection-repaint.html [ ImageOnlyFailure Failure ]
-webkit.org/b/101547 fast/repaint/no-caret-repaint-in-non-content-editable-element.html [ ImageOnlyFailure ]
-webkit.org/b/101547 fast/repaint/repaint-across-writing-mode-boundary.html [ ImageOnlyFailure ]
-webkit.org/b/101547 fast/repaint/selection-after-delete.html [ ImageOnlyFailure ]
-webkit.org/b/101547 fast/repaint/selection-rl.html [ ImageOnlyFailure ]
-# The following test has prior expectations which were disabled. Re-enable when removing this:
-webkit.org/b/101547 svg/custom/foreignObject-crash-on-hover.xml [ ImageOnlyFailure ]
-# The following test has prior expectations which were disabled. Re-enable when removing this:
-webkit.org/b/101547 svg/custom/hit-test-unclosed-subpaths.svg [ ImageOnlyFailure ]
-# The following test has prior expectations which were disabled. Re-enable when removing this:
-webkit.org/b/101547 svg/custom/hit-test-with-br.xhtml [ ImageOnlyFailure ]
-
-webkit.org/b/101794 fast/canvas/canvas-as-image-hidpi.html [ ImageOnlyFailure ]
-webkit.org/b/101794 platform/chromium/virtual/gpu/fast/canvas/canvas-as-image-hidpi.html [ ImageOnlyFailure ]
-webkit.org/b/101794 fast/canvas/canvas-resize-reset-pixelRatio.html [ Failure ]
-webkit.org/b/101794 platform/chromium/virtual/gpu/fast/canvas/canvas-resize-reset-pixelRatio.html [ Failure ]
Modified: trunk/Source/WebCore/ChangeLog (134190 => 134191)
--- trunk/Source/WebCore/ChangeLog 2012-11-12 06:37:55 UTC (rev 134190)
+++ trunk/Source/WebCore/ChangeLog 2012-11-12 07:09:44 UTC (rev 134191)
@@ -1,3 +1,28 @@
+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-11 Dongwoo Joshua Im <[email protected]>
[CSS3] Parsing the property, text-align-last.
Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (134190 => 134191)
--- trunk/Source/WebCore/editing/FrameSelection.cpp 2012-11-12 06:37:55 UTC (rev 134190)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp 2012-11-12 07:09:44 UTC (rev 134191)
@@ -294,9 +294,10 @@
if (!s.isNone() && !(options & DoNotSetFocus))
setFocusedNodeIfNeeded();
-
- updateAppearance();
+ if (!(options & DoNotUpdateAppearance))
+ 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();
@@ -1151,6 +1152,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())
@@ -1727,12 +1743,8 @@
void FrameSelection::updateAppearance()
{
#if ENABLE(TEXT_CARET)
- bool caretRectChangedOrCleared = false;
- if (isNonOrphanedCaret(m_selection)) {
- m_frame->document()->updateLayout();
- caretRectChangedOrCleared = recomputeCaretRect();
- } else
- caretRectChangedOrCleared = true;
+ m_frame->document()->updateLayout();
+ bool caretRectChangedOrCleared = recomputeCaretRect();
bool caretBrowsing = m_frame->settings() && m_frame->settings()->caretBrowsingEnabled();
bool shouldBlink = caretIsVisible() && isCaret() && (isContentEditable() || caretBrowsing);
@@ -1753,10 +1765,10 @@
invalidateCaretRect();
}
}
-#endif
-
+#else
// We need to update style in case the node containing the selection is made display:none.
m_frame->document()->updateStyleIfNeeded();
+#endif
RenderView* view = m_frame->contentRenderer();
if (!view)
Modified: trunk/Source/WebCore/editing/FrameSelection.h (134190 => 134191)
--- trunk/Source/WebCore/editing/FrameSelection.h 2012-11-12 06:37:55 UTC (rev 134190)
+++ trunk/Source/WebCore/editing/FrameSelection.h 2012-11-12 07:09:44 UTC (rev 134191)
@@ -123,7 +123,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)
@@ -153,7 +154,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: trunk/Source/WebCore/loader/FrameLoader.cpp (134190 => 134191)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2012-11-12 06:37:55 UTC (rev 134190)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2012-11-12 07:09:44 UTC (rev 134191)
@@ -570,7 +570,7 @@
m_frame->script()->clearWindowShell(newDocument->domWindow(), 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();
