Commit 2e1f1cbb authored by yoichio@chromium.org's avatar yoichio@chromium.org

FrameSelection::updateApperance for caret should not cause layout.

We do not need update caret rectangle synchronously because we just need 
updated caret in painting.
Thus this CL delays computing new caret rectangle.

Source/core/editing/FrameSelection.cpp:
- Add ResetCaretBlinkOption to updateAppearance(). The option is set 
to ResetCaretBlink when it is called from setSelection.
If ResetCaretBlink, we reset caret blinking.
If we need to repaint caret, set |m_caretRectDirty| flag.
If |m_caretRectDirty| is set, we call PageAnimator::scheduleVisualUpdate to 
trigger repaint.
For range, create VisibleSelection without validation like 
HTMLTextFormControlElement::setSelectionRange.
- Add the setCaretRectNeedsUpdate function to just set |m_caretRectDirty| flag
 and call new scheduleVisualUpdate function, which calls
PageAnimator::scheduleVisualUpdate.
- FrameSelection::invalidateCaretRect does
 1. Checks the dirty flag.
 2. Gets new caret rectangle and node which has the caret renderer.
 3. if caret is changed, invalidate the new caret rect and the old caret rect.
 4. Caches the new caret rect and node. Sets dirty flag off.
This function is similar to old recomputeCaretRect(deleted).
- Delete unused updateRenderTreeIfneeded() from notifyRenderOfSelectionChange().
In old days, notifyRenderOfSelectionChange() used renderer.
- Refactor FrameSelection::absoluteCaretBounds() to call updateCaretRect
 directly. Since we update layout, ASSERT that document's lifecycle is not in
InPaintInvalidation.


Source/core/editing/Caret.cpp:
- Remove document->updateRenderTreeIfNeeded() from CaretBase::updateCaretRect
 and call it from caller.
- In FrameSelection::paintCaret, call without updateRenderTreeIfNeeded because
the tree must be updated on the painting phase.
Source/core/frame/FrameView.cpp:
- FrameView::invalidateTreeIfNeeded calls FrameSelection::InvalidateCaretRect
to invalidate caret rect for each frame.

Source/core/html/HTMLTextFormControlElementTest.cpp:
- Add new test to confirm setSelectionRange with another start/end not cause 
layout.
- Delete FrameSelectionLocalCaretRectDoesNotCauseLayout because localCaretRect 
is removed.

Layouttests:
- This cl reduces redundant RenderObject::invalidatePaintRectangle calls so we 
need to delete lines from expected.txt but result images are still same.
 
BUG=382809, 397303, 403317
TEST=Layouttests/fast/forms/textarea-scrollbar.html, textarea-scrolled-type.html

Review URL: https://codereview.chromium.org/431983005

git-svn-id: svn://svn.chromium.org/blink/trunk@180269 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 03437977
...@@ -1303,4 +1303,8 @@ crbug.com/401902 [ Win7 Debug ] virtual/gpu/fast/canvas/canvas-lose-restore-goo ...@@ -1303,4 +1303,8 @@ crbug.com/401902 [ Win7 Debug ] virtual/gpu/fast/canvas/canvas-lose-restore-goo
crbug.com/402379 [ Win7 Debug ] storage/indexeddb/cursor-continue-validity.html [ Pass Slow ] crbug.com/402379 [ Win7 Debug ] storage/indexeddb/cursor-continue-validity.html [ Pass Slow ]
crbug.com/402379 [ Win7 Debug ] storage/indexeddb/mozilla/indexes.html [ Pass Slow ] crbug.com/402379 [ Win7 Debug ] storage/indexeddb/mozilla/indexes.html [ Pass Slow ]
crbug.com/382809 editing/selection/repaint-rect-for-vertical-writing-mode-with-positioned-root.html [ NeedsRebaseline ]
crbug.com/382809 fast/repaint/caret-outside-block.html [ NeedsRebaseline ]
crbug.com/382809 fast/repaint/inline-outline-repaint.html [ NeedsRebaseline ]
crbug.com/402801 [ XP Release ] http/tests/misc/location-replace-crossdomain.html [ Pass Slow ] crbug.com/402801 [ XP Release ] http/tests/misc/location-replace-crossdomain.html [ Pass Slow ]
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
"contentsOpaque": true, "contentsOpaque": true,
"drawsContent": true, "drawsContent": true,
"repaintRects": [ "repaintRects": [
[357, 199, 3, 22],
[357, 199, 3, 22] [357, 199, 3, 22]
] ]
} }
......
...@@ -70,10 +70,12 @@ void DragCaretController::setCaretPosition(const VisiblePosition& position) ...@@ -70,10 +70,12 @@ void DragCaretController::setCaretPosition(const VisiblePosition& position)
invalidateCaretRect(node); invalidateCaretRect(node);
document = &node->document(); document = &node->document();
} }
if (m_position.isNull() || m_position.isOrphan()) if (m_position.isNull() || m_position.isOrphan()) {
clearCaretRect(); clearCaretRect();
else } else {
document->updateRenderTreeIfNeeded();
updateCaretRect(document, m_position); updateCaretRect(document, m_position);
}
} }
static bool removingNodeRemovesPosition(Node& node, const Position& position) static bool removingNodeRemovesPosition(Node& node, const Position& position)
...@@ -134,7 +136,6 @@ RenderBlock* CaretBase::caretRenderer(Node* node) ...@@ -134,7 +136,6 @@ RenderBlock* CaretBase::caretRenderer(Node* node)
bool CaretBase::updateCaretRect(Document* document, const PositionWithAffinity& caretPosition) bool CaretBase::updateCaretRect(Document* document, const PositionWithAffinity& caretPosition)
{ {
document->updateRenderTreeIfNeeded();
m_caretLocalRect = LayoutRect(); m_caretLocalRect = LayoutRect();
m_caretRectNeedsUpdate = false; m_caretRectNeedsUpdate = false;
......
...@@ -89,6 +89,10 @@ public: ...@@ -89,6 +89,10 @@ public:
NonDirectional, NonDirectional,
Directional Directional
}; };
enum ResetCaretBlinkOption {
None,
ResetCaretBlink
};
Element* rootEditableElement() const { return m_selection.rootEditableElement(); } Element* rootEditableElement() const { return m_selection.rootEditableElement(); }
Element* rootEditableElementOrDocumentElement() const; Element* rootEditableElementOrDocumentElement() const;
...@@ -139,13 +143,8 @@ public: ...@@ -139,13 +143,8 @@ public:
// Return the renderer that is responsible for painting the caret (in the selection start node) // Return the renderer that is responsible for painting the caret (in the selection start node)
RenderBlock* caretRenderer() const; RenderBlock* caretRenderer() const;
// Caret rect local to the caret's renderer
LayoutRect localCaretRect();
LayoutRect localCaretRectWithoutUpdateForTesting() const { return CaretBase::localCaretRectWithoutUpdate(); }
// Bounds of (possibly transformed) caret in absolute coords // Bounds of (possibly transformed) caret in absolute coords
IntRect absoluteCaretBounds(); IntRect absoluteCaretBounds();
void setCaretRectNeedsUpdate() { CaretBase::setCaretRectNeedsUpdate(); }
void didChangeFocus(); void didChangeFocus();
void willBeModified(EAlteration, SelectionDirection); void willBeModified(EAlteration, SelectionDirection);
...@@ -168,8 +167,11 @@ public: ...@@ -168,8 +167,11 @@ public:
void didMergeTextNodes(const Text& oldNode, unsigned offset); void didMergeTextNodes(const Text& oldNode, unsigned offset);
void didSplitTextNode(const Text& oldNode); void didSplitTextNode(const Text& oldNode);
void updateAppearance(ResetCaretBlinkOption = None);
void setCaretVisible(bool caretIsVisible) { setCaretVisibility(caretIsVisible ? Visible : Hidden); } void setCaretVisible(bool caretIsVisible) { setCaretVisibility(caretIsVisible ? Visible : Hidden); }
bool recomputeCaretRect(); bool isCaretBoundsDirty() const { return m_caretRectDirty; }
void setCaretRectNeedsUpdate();
void scheduleVisualUpdate() const;
void invalidateCaretRect(); void invalidateCaretRect();
void paintCaret(GraphicsContext*, const LayoutPoint&, const LayoutRect& clipRect); void paintCaret(GraphicsContext*, const LayoutPoint&, const LayoutRect& clipRect);
...@@ -183,9 +185,6 @@ public: ...@@ -183,9 +185,6 @@ public:
bool isFocusedAndActive() const; bool isFocusedAndActive() const;
void pageActivationChanged(); void pageActivationChanged();
// Painting.
void updateAppearance();
void updateSecureKeyboardEntryIfActive(); void updateSecureKeyboardEntryIfActive();
#ifndef NDEBUG #ifndef NDEBUG
...@@ -280,13 +279,13 @@ private: ...@@ -280,13 +279,13 @@ private:
RefPtrWillBeMember<Range> m_logicalRange; RefPtrWillBeMember<Range> m_logicalRange;
RefPtrWillBeMember<Node> m_previousCaretNode; // The last node which painted the caret. Retained for clearing the old caret when it moves. RefPtrWillBeMember<Node> m_previousCaretNode; // The last node which painted the caret. Retained for clearing the old caret when it moves.
LayoutRect m_previousCaretRect;
RefPtrWillBeMember<EditingStyle> m_typingStyle; RefPtrWillBeMember<EditingStyle> m_typingStyle;
Timer<FrameSelection> m_caretBlinkTimer; Timer<FrameSelection> m_caretBlinkTimer;
// The painted bounds of the caret in absolute coordinates
IntRect m_absCaretBounds; bool m_caretRectDirty : 1;
bool m_absCaretBoundsDirty : 1;
bool m_caretPaint : 1; bool m_caretPaint : 1;
bool m_isCaretBlinkingSuspended : 1; bool m_isCaretBlinkingSuspended : 1;
bool m_focused : 1; bool m_focused : 1;
......
...@@ -995,6 +995,9 @@ void FrameView::invalidateTreeIfNeeded() ...@@ -995,6 +995,9 @@ void FrameView::invalidateTreeIfNeeded()
#ifndef NDEBUG #ifndef NDEBUG
renderView()->assertSubtreeClearedPaintInvalidationState(); renderView()->assertSubtreeClearedPaintInvalidationState();
#endif #endif
if (m_frame->selection().isCaretBoundsDirty())
m_frame->selection().invalidateCaretRect();
} }
DocumentLifecycle& FrameView::lifecycle() const DocumentLifecycle& FrameView::lifecycle() const
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "core/page/SpellCheckerClient.h" #include "core/page/SpellCheckerClient.h"
#include "core/rendering/RenderTreeAsText.h" #include "core/rendering/RenderTreeAsText.h"
#include "core/testing/DummyPageHolder.h" #include "core/testing/DummyPageHolder.h"
#include "core/testing/UnitTestHelpers.h"
#include "wtf/OwnPtr.h" #include "wtf/OwnPtr.h"
#include <gtest/gtest.h> #include <gtest/gtest.h>
...@@ -107,35 +108,29 @@ TEST_F(HTMLTextFormControlElementTest, SetSelectionRange) ...@@ -107,35 +108,29 @@ TEST_F(HTMLTextFormControlElementTest, SetSelectionRange)
EXPECT_EQ(3, textControl().selectionEnd()); EXPECT_EQ(3, textControl().selectionEnd());
} }
TEST_F(HTMLTextFormControlElementTest, FrameSelectionLocalCaretRectDoesNotCauseLayout) TEST_F(HTMLTextFormControlElementTest, SetSelectionRangeDoesNotCauseLayout)
{
input().focus();
input().setValue("Hello, input form.");
FrameSelection& frameSelection = document().frame()->selection();
frameSelection.setCaretRectNeedsUpdate();
forceLayoutFlag();
int startLayoutCount = layoutCount();
frameSelection.localCaretRect();
EXPECT_EQ(startLayoutCount, layoutCount());
}
TEST_F(HTMLTextFormControlElementTest, SetSameSelectionRangeDoesNotCauseLayout)
{ {
input().focus(); input().focus();
input().setValue("Hello, input form."); input().setValue("Hello, input form.");
input().setSelectionRange(1, 1); input().setSelectionRange(1, 1);
FrameSelection& frameSelection = document().frame()->selection(); FrameSelection& frameSelection = document().frame()->selection();
LayoutRect oldCaretRect = frameSelection.localCaretRectWithoutUpdateForTesting();
EXPECT_FALSE(oldCaretRect.isEmpty());
forceLayoutFlag(); forceLayoutFlag();
LayoutRect oldCaretRect = frameSelection.absoluteCaretBounds();
EXPECT_FALSE(oldCaretRect.isEmpty());
int startLayoutCount = layoutCount(); int startLayoutCount = layoutCount();
input().setSelectionRange(1, 1); input().setSelectionRange(1, 1);
EXPECT_EQ(startLayoutCount, layoutCount()); EXPECT_EQ(startLayoutCount, layoutCount());
LayoutRect newCaretRect = frameSelection.absoluteCaretBounds();
LayoutRect newCaretRect = frameSelection.localCaretRectWithoutUpdateForTesting();
EXPECT_EQ(oldCaretRect, newCaretRect); EXPECT_EQ(oldCaretRect, newCaretRect);
forceLayoutFlag();
oldCaretRect = frameSelection.absoluteCaretBounds();
EXPECT_FALSE(oldCaretRect.isEmpty());
startLayoutCount = layoutCount();
input().setSelectionRange(2, 2);
EXPECT_EQ(startLayoutCount, layoutCount());
newCaretRect = frameSelection.absoluteCaretBounds();
EXPECT_NE(oldCaretRect, newCaretRect);
} }
typedef Position (*PositionFunction)(const Position&); typedef Position (*PositionFunction)(const Position&);
...@@ -164,7 +159,7 @@ void testBoundary(HTMLDocument& document, HTMLTextFormControlElement& textContro ...@@ -164,7 +159,7 @@ void testBoundary(HTMLDocument& document, HTMLTextFormControlElement& textContro
for (unsigned i = 0; i < textControl.innerEditorValue().length(); i++) { for (unsigned i = 0; i < textControl.innerEditorValue().length(); i++) {
textControl.setSelectionRange(i, i); textControl.setSelectionRange(i, i);
Position position = document.frame()->selection().start(); Position position = document.frame()->selection().start();
SCOPED_TRACE(testing::Message() << "offset " << position.deprecatedEditingOffset() << " of " << nodePositionAsStringForTesting(position.deprecatedNode()).ascii().data()); SCOPED_TRACE(::testing::Message() << "offset " << position.deprecatedEditingOffset() << " of " << nodePositionAsStringForTesting(position.deprecatedNode()).ascii().data());
{ {
SCOPED_TRACE("HTMLTextFormControlElement::startOfWord"); SCOPED_TRACE("HTMLTextFormControlElement::startOfWord");
testFunctionEquivalence(position, HTMLTextFormControlElement::startOfWord, startOfWord); testFunctionEquivalence(position, HTMLTextFormControlElement::startOfWord, startOfWord);
......
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