Audit the use of updateStyleAndLayoutIgnorePendingStylesheets in...

Audit the use of updateStyleAndLayoutIgnorePendingStylesheets in InputMethodController::setSelectionOffsets

BUG=590369

Review-Url: https://codereview.chromium.org/2316053002
Cr-Commit-Position: refs/heads/master@{#417250}

third_party/WebKit/Source/core/editing/InputMethodController.cpp

@@ -118,19 +118,6 @@ void insertTextDuringCompositionWithEvents(LocalFrame& frame, const String& text
 
 } // anonymous namespace
 
-InputMethodController::SelectionOffsetsScope::SelectionOffsetsScope(InputMethodController* inputMethodController)
-    : m_inputMethodController(inputMethodController)
-    , m_offsets(inputMethodController->getSelectionOffsets())
-{
-}
-
-InputMethodController::SelectionOffsetsScope::~SelectionOffsetsScope()
-{
-    m_inputMethodController->setSelectionOffsets(m_offsets);
-}
-
-// ----------------------------
-
 InputMethodController* InputMethodController::create(LocalFrame& frame)
 {
     return new InputMethodController(frame);
@@ -254,8 +241,15 @@ bool InputMethodController::confirmCompositionOrInsertText(const String& text, C
     if (confirmBehavior == DoNotKeepSelection)
         return confirmComposition(composingText(), DoNotKeepSelection);
 
-    SelectionOffsetsScope selectionOffsetsScope(this);
-    return confirmComposition();
+    PlainTextRange oldOffsets = getSelectionOffsets();
+    bool result = confirmComposition();
+
+    // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
+    // needs to be audited. see http://crbug.com/590369 for more details.
+    frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
+
+    setSelectionOffsets(oldOffsets);
+    return result;
 }
 
 void InputMethodController::cancelComposition()
@@ -354,6 +348,10 @@ void InputMethodController::setComposition(const String& text, const Vector<Comp
             TypingCommand::deleteSelection(*frame().document(), TypingCommand::PreventSpellChecking);
         }
 
+        // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
+        // needs to be audited. see http://crbug.com/590369 for more details.
+        frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
+
         setEditableSelectionOffsets(selectedRange);
         return;
     }
@@ -405,6 +403,10 @@ void InputMethodController::setComposition(const String& text, const Vector<Comp
     if (baseNode->layoutObject())
         baseNode->layoutObject()->setShouldDoFullPaintInvalidation();
 
+    // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
+    // needs to be audited. see http://crbug.com/590369 for more details.
+    frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
+
     // We shouldn't close typing in the middle of setComposition.
     setEditableSelectionOffsets(selectedRange, NotUserTriggered);
 
@@ -496,9 +498,7 @@ bool InputMethodController::setSelectionOffsets(const PlainTextRange& selectionO
     if (!rootEditableElement)
         return false;
 
-    // TODO(dglazkov): The use of updateStyleAndLayoutIgnorePendingStylesheets needs to be audited.
-    // see http://crbug.com/590369 for more details.
-    rootEditableElement->document().updateStyleAndLayoutIgnorePendingStylesheets();
+    DCHECK(!rootEditableElement->document().needsLayoutTreeUpdate());
 
     const EphemeralRange range = selectionOffsets.createRange(*rootEditableElement);
     if (range.isNull())

third_party/WebKit/Source/core/editing/InputMethodController.h

@@ -82,18 +82,6 @@ public:
     PlainTextRange createRangeForSelection(int start, int end, size_t textLength) const;
 
 private:
-    class SelectionOffsetsScope {
-        WTF_MAKE_NONCOPYABLE(SelectionOffsetsScope);
-        STACK_ALLOCATED();
-    public:
-        explicit SelectionOffsetsScope(InputMethodController*);
-        ~SelectionOffsetsScope();
-    private:
-        Member<InputMethodController> m_inputMethodController;
-        const PlainTextRange m_offsets;
-    };
-    friend class SelectionOffsetsScope;
-
     Member<LocalFrame> m_frame;
     Member<Range> m_compositionRange;
     bool m_isDirty;

third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp

@@ -57,36 +57,42 @@ TEST_F(InputMethodControllerTest, BackspaceFromEndOfInput)
         insertHTMLElement("<input id='sample'>", "sample"));
 
     input->setValue("fooX");
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(4, 4));
     EXPECT_STREQ("fooX", input->value().utf8().data());
     controller().extendSelectionAndDelete(0, 0);
     EXPECT_STREQ("fooX", input->value().utf8().data());
 
     input->setValue("fooX");
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(4, 4));
     EXPECT_STREQ("fooX", input->value().utf8().data());
     controller().extendSelectionAndDelete(1, 0);
     EXPECT_STREQ("foo", input->value().utf8().data());
 
     input->setValue(String::fromUTF8("foo\xE2\x98\x85")); // U+2605 == "black star"
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(4, 4));
     EXPECT_STREQ("foo\xE2\x98\x85", input->value().utf8().data());
     controller().extendSelectionAndDelete(1, 0);
     EXPECT_STREQ("foo", input->value().utf8().data());
 
     input->setValue(String::fromUTF8("foo\xF0\x9F\x8F\x86")); // U+1F3C6 == "trophy"
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(4, 4));
     EXPECT_STREQ("foo\xF0\x9F\x8F\x86", input->value().utf8().data());
     controller().extendSelectionAndDelete(1, 0);
     EXPECT_STREQ("foo", input->value().utf8().data());
 
     input->setValue(String::fromUTF8("foo\xE0\xB8\x81\xE0\xB9\x89")); // composed U+0E01 "ka kai" + U+0E49 "mai tho"
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(4, 4));
     EXPECT_STREQ("foo\xE0\xB8\x81\xE0\xB9\x89", input->value().utf8().data());
     controller().extendSelectionAndDelete(1, 0);
     EXPECT_STREQ("foo", input->value().utf8().data());
 
     input->setValue("fooX");
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(4, 4));
     EXPECT_STREQ("fooX", input->value().utf8().data());
     controller().extendSelectionAndDelete(0, 1);
@@ -131,12 +137,14 @@ TEST_F(InputMethodControllerTest, DeleteBySettingEmptyComposition)
         insertHTMLElement("<input id='sample'>", "sample"));
 
     input->setValue("foo ");
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(4, 4));
     EXPECT_STREQ("foo ", input->value().utf8().data());
     controller().extendSelectionAndDelete(0, 0);
     EXPECT_STREQ("foo ", input->value().utf8().data());
 
     input->setValue("foo ");
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(4, 4));
     EXPECT_STREQ("foo ", input->value().utf8().data());
     controller().extendSelectionAndDelete(1, 0);
@@ -201,6 +209,7 @@ TEST_F(InputMethodControllerTest, SetCompositionForInputWithDifferentNewCursorPo
         insertHTMLElement("<input id='sample'>", "sample"));
 
     input->setValue("hello");
+    document().updateStyleAndLayout();
     controller().setEditableSelectionOffsets(PlainTextRange(2, 2));
     EXPECT_STREQ("hello", input->value().utf8().data());
     EXPECT_EQ(2u, controller().getSelectionOffsets().start());

third_party/WebKit/Source/web/WebLocalFrameImpl.cpp

@@ -1224,6 +1224,11 @@ void WebLocalFrameImpl::moveCaretSelection(const WebPoint& pointInViewport)
 bool WebLocalFrameImpl::setEditableSelectionOffsets(int start, int end)
 {
     TRACE_EVENT0("blink", "WebLocalFrameImpl::setEditableSelectionOffsets");
+
+    // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
+    // needs to be audited.  See http://crbug.com/590369 for more details.
+    frame()->document()->updateStyleAndLayoutIgnorePendingStylesheets();
+
     return frame()->inputMethodController().setEditableSelectionOffsets(PlainTextRange(start, end));
 }