Changeset 166421 in webkit

Timestamp:

Mar 28, 2014 12:11:18 PM (10 years ago)

Author:

ap@apple.com

Message:

Eliminate a sync cancelComposition call in WebPageProxy::editorStateChanged
​https://bugs.webkit.org/show_bug.cgi?id=130727

Reviewed by Enrica Casucci.

Added a separate CompositionWasCanceled IPC call, with which WebProcess can tell
UIProcess that it should notify an input method of canceled composition. There are
a lot of incorrect edge cases where we don't call it correctly, but this was true
in the old implementation too (for different edge cases).

This change only affects iOS and async NSTextInputClient code path on Mac. I don't
want to change sync NSTextInputClient code path unless absolutely necessary.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::WebPageProxy):
(WebKit::WebPageProxy::editorStateChanged):
(WebKit::WebPageProxy::compositionWasCanceled):
(WebKit::WebPageProxy::resetStateAfterProcessExited):

• UIProcess/WebPageProxy.h:
• UIProcess/WebPageProxy.messages.in:
• UIProcess/mac/WebPageProxyMac.mm:

(WebKit::WebPageProxy::insertText):
(WebKit::WebPageProxy::executeKeypressCommands):

• WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::didChangeSelection):

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebPageProxy.cpp (modified) (5 diffs)
• UIProcess/WebPageProxy.h (modified) (2 diffs)
• UIProcess/WebPageProxy.messages.in (modified) (1 diff)
• UIProcess/mac/WebPageProxyMac.mm (modified) (2 diffs)
• WebProcess/WebPage/WebPage.cpp (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2014-03-28  Alexey Proskuryakov  <ap@apple.com>
+
+        Eliminate a sync cancelComposition call in WebPageProxy::editorStateChanged
+        https://bugs.webkit.org/show_bug.cgi?id=130727
+
+        Reviewed by Enrica Casucci.
+
+        Added a separate CompositionWasCanceled IPC call, with which WebProcess can tell
+        UIProcess that it should notify an input method of canceled composition. There are
+        a lot of incorrect edge cases where we don't call it correctly, but this was true
+        in the old implementation too (for different edge cases).
+
+        This change only affects iOS and async NSTextInputClient code path on Mac. I don't
+        want to change sync NSTextInputClient code path unless absolutely necessary.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::WebPageProxy):
+        (WebKit::WebPageProxy::editorStateChanged):
+        (WebKit::WebPageProxy::compositionWasCanceled):
+        (WebKit::WebPageProxy::resetStateAfterProcessExited):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+        * UIProcess/mac/WebPageProxyMac.mm:
+        (WebKit::WebPageProxy::insertText):
+        (WebKit::WebPageProxy::executeKeypressCommands):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::didChangeSelection):
+
 2014-03-28  Mario Sanchez Prada  <mario.prada@samsung.com>
 

trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

@@ -269 +269 @@
     , m_backForwardList(WebBackForwardList::create(*this))
     , m_loadStateAtProcessExit(FrameLoadState::State::Finished)
+#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
     , m_temporarilyClosedComposition(false)
+#endif
     , m_textZoomFactor(1)
     , m_pageZoomFactor(1)
@@ -3121 +3123 @@
 #if PLATFORM(COCOA)
     bool couldChangeSecureInputState = m_editorState.isInPasswordField != editorState.isInPasswordField || m_editorState.selectionIsNone;
+#endif
+#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
     bool closedComposition = !editorState.shouldIgnoreCompositionSelectionChange && !editorState.hasComposition && (m_editorState.hasComposition || m_temporarilyClosedComposition);
     m_temporarilyClosedComposition = editorState.shouldIgnoreCompositionSelectionChange && (m_temporarilyClosedComposition || m_editorState.hasComposition) && !editorState.hasComposition;
@@ -3131 +3135 @@
     if (couldChangeSecureInputState && !editorState.selectionIsNone)
         m_pageClient.updateSecureInputState();
+#endif
 
     if (editorState.shouldIgnoreCompositionSelectionChange)
         return;
 
+#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
     if (closedComposition)
         m_pageClient.notifyInputContextAboutDiscardedComposition();
@@ -3143 +3149 @@
         m_pageClient.notifyInputContextAboutDiscardedComposition();
     }
-#if PLATFORM(IOS)
-    else {
+#elif PLATFORM(IOS)
+    if (!editorState.hasComposition) {
         // We need to notify the client on iOS to make sure the selection is redrawn.
         notifyRevealedSelection();
     }
-#endif
 #elif PLATFORM(EFL) || PLATFORM(GTK)
     m_pageClient.updateTextInputState();
 #endif
+}
+
+void WebPageProxy::compositionWasCanceled(const EditorState& editorState)
+{
+#if PLATFORM(COCOA)
+    m_pageClient.notifyInputContextAboutDiscardedComposition();
+#endif
+    editorStateChanged(editorState);
 }
 
@@ -4074 +4087 @@
     // FIXME: Reset m_editorState.
     // FIXME: Notify input methods about abandoned composition.
+#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
     m_temporarilyClosedComposition = false;
+#endif
 
 #if PLATFORM(MAC)

trunk/Source/WebKit2/UIProcess/WebPageProxy.h

@@ -1187 +1187 @@
 
     void editorStateChanged(const EditorState&);
+    void compositionWasCanceled(const EditorState&);
 
     // Back/Forward list management
@@ -1471 +1472 @@
 
     EditorState m_editorState;
+#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
     bool m_temporarilyClosedComposition; // Editor state changed from hasComposition to !hasComposition, but that was only with shouldIgnoreCompositionSelectionChange yet.
+#endif
 
     double m_textZoomFactor;

trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in

@@ -204 +204 @@
     # Editor notifications
     EditorStateChanged(WebKit::EditorState editorState)
+    CompositionWasCanceled(WebKit::EditorState editorState)
 
     # Find messages

trunk/Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm

@@ -181 +181 @@
     bool handled = true;
     process().sendSync(Messages::WebPage::InsertText(text, replacementRange), Messages::WebPage::InsertText::Reply(handled, m_editorState), m_pageID);
+#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
     m_temporarilyClosedComposition = false;
+#endif
 
     return handled;
@@ -271 +273 @@
     bool result = false;
     process().sendSync(Messages::WebPage::ExecuteKeypressCommands(commands), Messages::WebPage::ExecuteKeypressCommands::Reply(result, m_editorState), m_pageID);
+#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
     m_temporarilyClosedComposition = false;
+#endif
     return result;
 }

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

@@ -4110 +4110 @@
 void WebPage::didChangeSelection()
 {
-    send(Messages::WebPageProxy::EditorStateChanged(editorState()));
+#if (PLATFORM(MAC) && USE(ASYNC_NSTEXTINPUTCLIENT)) || PLATFORM(IOS)
+    Frame& frame = m_page->focusController().focusedOrMainFrame();
+    // Abandon the current inline input session if selection changed for any other reason but an input method direct action.
+    // FIXME: Many changes that affect composition node do not go through didChangeSelection(). We need to do something when DOM manipulation affects the composition, because otherwise input method's idea about it will be different from Editor's.
+    // FIXME: We can't cancel composition when selection changes to NoSelection, but we probably should.
+    if (frame.editor().hasComposition() && !frame.editor().ignoreCompositionSelectionChange() && !frame.selection().isNone()) {
+        frame.editor().cancelComposition();
+        send(Messages::WebPageProxy::CompositionWasCanceled(editorState()));
+    } else
+#endif
+        send(Messages::WebPageProxy::EditorStateChanged(editorState()));
+
 #if PLATFORM(IOS)
     m_drawingArea->scheduleCompositingLayerFlush();