[Webkit-unassigned] [Bug 185248] [GTK][WPE] Special combination characters doesn't respect the keystroke order when high CPU load

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=185248

Zan Dobersek <zan at falconsigh.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zan at falconsigh.net

--- Comment #7 from Zan Dobersek <zan at falconsigh.net> ---
Comment on attachment 386193
  --> https://bugs.webkit.org/attachment.cgi?id=386193
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386193&action=review

> Source/WebCore/platform/PlatformKeyboardEvent.h:103
> +        Optional<Vector<WebCore::CompositionUnderline>> preeditUnderlines() const { return m_preeditUnderlines; }
> +        Optional<uint64_t> preeditSelectionRangeStart() const { return m_preeditSelectionRangeStart; }
> +        Optional<uint64_t> preeditSelectionRangeLength() const { return m_preeditSelectionRangeLength; }

These can be const references.

> Source/WebKit/Shared/NativeWebKeyboardEvent.h:73
> +    NativeWebKeyboardEvent(GdkEvent*, const String&, HandledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>>, Optional<EditingRange>, Vector<String>&& commands);

The Optional parameters should be rvalue references.

> Source/WebKit/Shared/NativeWebKeyboardEvent.h:79
> +    NativeWebKeyboardEvent(struct wpe_input_keyboard_event*, const String&, HandledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>>, Optional<EditingRange>);

The Optional parameters should be rvalue references.

> Source/WebKit/Shared/WebEvent.h:262
> +    WebKeyboardEvent(Type, const String& text, const String& key, const String& code, const String& keyIdentifier, int windowsVirtualKeyCode, int nativeVirtualKeyCode, bool handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>>, Optional<EditingRange>, Vector<String>&& commands, bool isKeypad, OptionSet<Modifier>, WallTime timestamp);

The Optional parameters should be rvalue references.

> Source/WebKit/Shared/WebEvent.h:266
> +    WebKeyboardEvent(Type, const String& text, const String& key, const String& code, const String& keyIdentifier, int windowsVirtualKeyCode, int nativeVirtualKeyCode, bool handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>>, Optional<EditingRange>, bool isKeypad, OptionSet<Modifier>, WallTime timestamp);

The Optional parameters should be rvalue references.

> Source/WebKit/Shared/WebEvent.h:284
> +    Optional<Vector<WebCore::CompositionUnderline>> preeditUnderlines() const { return m_preeditUnderlines; }
> +    Optional<EditingRange> preeditSelectionRange() const { return m_preeditSelectionRange; }

These can be const references.

> Source/WebKit/Shared/WebKeyboardEvent.cpp:61
> +WebKeyboardEvent::WebKeyboardEvent(Type type, const String& text, const String& key, const String& code, const String& keyIdentifier, int windowsVirtualKeyCode, int nativeVirtualKeyCode, bool handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>> preeditUnderlines, Optional<EditingRange> preeditSelectionRange, Vector<String>&& commands, bool isKeypad, OptionSet<Modifier> modifiers, WallTime timestamp)

The Optional parameters should be rvalue references.

> Source/WebKit/Shared/WebKeyboardEvent.cpp:106
> +WebKeyboardEvent::WebKeyboardEvent(Type type, const String& text, const String& key, const String& code, const String& keyIdentifier, int windowsVirtualKeyCode, int nativeVirtualKeyCode, bool handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>> preeditUnderlines, Optional<EditingRange> preeditSelectionRange, bool isKeypad, OptionSet<Modifier> modifiers, WallTime timestamp)

The Optional parameters should be rvalue references.

> Source/WebKit/Shared/gtk/NativeWebKeyboardEventGtk.cpp:37
> +NativeWebKeyboardEvent::NativeWebKeyboardEvent(GdkEvent* event, const String& text, HandledByInputMethod handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>> preeditUnderlines, Optional<EditingRange> preeditSelectionRange, Vector<String>&& commands)
> +    : WebKeyboardEvent(WebEventFactory::createWebKeyboardEvent(event, text, handledByInputMethod == HandledByInputMethod::Yes, preeditUnderlines, preeditSelectionRange, WTFMove(commands)))

The Optional parameters should be rvalue references, and they should be simply moved into the createWebKeyboardEvent() call.

> Source/WebKit/Shared/gtk/NativeWebKeyboardEventGtk.cpp:43
> +    : WebKeyboardEvent(WebEventFactory::createWebKeyboardEvent(event.nativeEvent(), event.text(), event.handledByInputMethod(), event.preeditUnderlines(), event.preeditSelectionRange(), Vector<String>(event.commands())))

Because of the rvalue references you'll have to perform explicit manual copy, which could be as simple as `{ event.preeditUnderlines() }`. Just like it's necessary for the last argument in this call.

> Source/WebKit/Shared/gtk/WebEventFactory.cpp:255
> +WebKeyboardEvent WebEventFactory::createWebKeyboardEvent(const GdkEvent* event, const String& text, bool handledByInputMethod, Optional<Vector<CompositionUnderline>> preeditUnderlines, Optional<EditingRange> preeditSelectionRange, Vector<String>&& commands)

The Optional parameters should be rvalue references that would be just moved over into the WebKeyboardEvent constructor, just like the `commands` parameter.

> Source/WebKit/Shared/libwpe/NativeWebKeyboardEventLibWPE.cpp:33
> +NativeWebKeyboardEvent::NativeWebKeyboardEvent(struct wpe_input_keyboard_event* event, const String& text, HandledByInputMethod handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>> preeditUnderlines, Optional<EditingRange> preeditSelectionRange)

The Optional parameters should be rvalue references that would be just moved over into the WebKeyboardEvent constructor.

> Source/WebKit/Shared/libwpe/WebEventFactory.cpp:65
> +WebKeyboardEvent WebEventFactory::createWebKeyboardEvent(struct wpe_input_keyboard_event* event, const String& text, bool handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>> preeditUnderlines, Optional<EditingRange> preeditSelectionRange)

The Optional parameters should be rvalue references that would be just moved over into the WebKeyboardEvent constructor.

> Source/WebKit/Shared/libwpe/WebEventFactory.h:41
> +    static WebKeyboardEvent createWebKeyboardEvent(struct wpe_input_keyboard_event*, const String&, bool handledByInputMethod, Optional<Vector<WebCore::CompositionUnderline>>, Optional<EditingRange>);

The Optional parameters should be rvalue references.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:2694
> +static void webkitWebViewSynthesizeCompositionKeyPress(WebKitWebView* webView, const String& text, Optional<Vector<CompositionUnderline>> underlines, Optional<EditingRange> selectionRange)

The Optional parameters should be rvalue references.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:1912
> +void webkitWebViewBaseSynthesizeCompositionKeyPress(WebKitWebViewBase* webViewBase, const String& text, Optional<Vector<CompositionUnderline>> underlines, Optional<EditingRange> selectionRange)

The Optional parameters should be rvalue references.

> Source/WebKit/UIProcess/API/wpe/WPEView.cpp:278
> +void View::synthesizeCompositionKeyPress(const String& text, Optional<Vector<WebCore::CompositionUnderline>> underlines, Optional<EditingRange> selectionRange)

The Optional parameters should be rvalue references. If not possible, then at least const references.

> Source/WebKit/WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:149
> +    if (auto underlines = platformEvent->preeditUnderlines()) {

When the return type is changed to being a reference, the `auto` here should follow.

> Source/WebKit/WebProcess/WebCoreSupport/wpe/WebEditorClientWPE.cpp:251
> +    if (auto underlines = platformEvent->preeditUnderlines()) {

When the return type is changed to being a reference, the `auto` here should follow.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:5359
> -void WebPage::confirmComposition(const String& compositionString)
> +void WebPage::cancelComposition(const String& compositionString)
>  {
>      if (auto* targetFrame = targetFrameForEditing(*this))
>          targetFrame->editor().confirmComposition(compositionString);
>  }

Is this fine?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20191220/b4d61590/attachment-0001.htm>