Attachment 370335 Details for Bug 198075 – Patch

[patch] Patch

bug-198075-20190521125244.patch (text/plain), 7.00 KB, created by Wenson Hsieh on 2019-05-21 12:52:45 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-05-21 12:52:45 PDT

Size: 7.00 KB

patch

obsolete

>Subversion Revision: 245539
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index efeb55cb9dfdb9285f49803219f25e319ffd9e6f..932360b0493a815523e6eaf04c58bb273cd2e83c 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-05-21  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        The cost of WebViewImpl::hasMarkedTextWithCompletionHandler should not increase with document size
>+        https://bugs.webkit.org/show_bug.cgi?id=198075
>+        <rdar://problem/37560103>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/Cocoa/WebViewImpl.mm:
>+        (WebKit::WebViewImpl::hasMarkedTextWithCompletionHandler):
>+
>+        Refactor hasMarkedTextWithCompletionHandler to use Editor::hasComposition, instead of computing the actual
>+        marked text range. The latter is more expensive and unnecessary, since it uses TextIterator from the document
>+        root to find editing offsets. This makes the cost of determining whether there is marked text proportional to
>+        the document size.
>+
>+        This matches behavior in legacy WebKit, as well as iOS.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::hasMarkedText):
>+        * UIProcess/WebPageProxy.h:
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::hasMarkedText):
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/WebPage.messages.in:
>+
> 2019-05-20  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         [WinCairo] Implement Remote Web Inspector Client.
>diff --git a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>index da3b183b52a9c9d70ae5caf5ab0b9a4e14f6cc99..2aa748f004f222a01ddb7ac8eee7132ca78890ca 100644
>--- a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>@@ -4812,21 +4812,12 @@ void WebViewImpl::markedRangeWithCompletionHandler(void(^completionHandlerPtr)(N
>     });
> }
> 
>-void WebViewImpl::hasMarkedTextWithCompletionHandler(void(^completionHandlerPtr)(BOOL hasMarkedText))
>+void WebViewImpl::hasMarkedTextWithCompletionHandler(void(^completionHandler)(BOOL hasMarkedText))
> {
>-    auto completionHandler = adoptNS([completionHandlerPtr copy]);
>-
>     LOG(TextInput, "hasMarkedText");
>-    m_page->getMarkedRangeAsync([completionHandler](const EditingRange& editingRangeResult, WebKit::CallbackBase::Error error) {
>-        void (^completionHandlerBlock)(BOOL) = (void (^)(BOOL))completionHandler.get();
>-        if (error != WebKit::CallbackBase::Error::None) {
>-            LOG(TextInput, "    ...hasMarkedText failed.");
>-            completionHandlerBlock(NO);
>-            return;
>-        }
>-        BOOL hasMarkedText = editingRangeResult.location != notFound;
>-        LOG(TextInput, "    -> hasMarkedText returned %u", hasMarkedText);
>-        completionHandlerBlock(hasMarkedText);
>+    m_page->hasMarkedText([completionHandler = makeBlockPtr(completionHandler)] (bool result) {
>+        completionHandler(result);
>+        LOG(TextInput, "    -> hasMarkedText returned %u", result);
>     });
> }
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 010c4863115856d36d86ec6d340955072dcd8c3d..c0e10861199528cbba1b6e2174a8e8aa191e48c4 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -7952,6 +7952,15 @@ void WebPageProxy::insertTextAsync(const String& text, const EditingRange& repla
>     process().send(Messages::WebPage::InsertTextAsync(text, replacementRange, WTFMove(options)), m_pageID);
> }
> 
>+void WebPageProxy::hasMarkedText(CompletionHandler<void(bool)>&& callback)
>+{
>+    if (!hasRunningProcess()) {
>+        callback(false);
>+        return;
>+    }
>+    m_process->connection()->sendWithAsyncReply(Messages::WebPage::HasMarkedText(), WTFMove(callback), m_pageID);
>+}
>+
> void WebPageProxy::getMarkedRangeAsync(WTF::Function<void (EditingRange, CallbackBase::Error)>&& callbackFunction)
> {
>     if (!hasRunningProcess()) {
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 062a4c89905a726455e427b2075bdf1286cabf09..2044e8429b05570f5e23782e118202732dbb424f 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -769,6 +769,7 @@ public:
> 
>     void setTextAsync(const String&);
>     void insertTextAsync(const String& text, const EditingRange& replacementRange, InsertTextOptions&&);
>+    void hasMarkedText(CompletionHandler<void(bool)>&&);
>     void getMarkedRangeAsync(WTF::Function<void (EditingRange, CallbackBase::Error)>&&);
>     void getSelectedRangeAsync(WTF::Function<void (EditingRange, CallbackBase::Error)>&&);
>     void characterIndexForPointAsync(const WebCore::IntPoint&, WTF::Function<void (uint64_t, CallbackBase::Error)>&&);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 93301cc50cacbc3603e77f0123989ac7aff5a340..a00d1914721601293f3dba63d716f4492bbe3a3f 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -5172,6 +5172,11 @@ void WebPage::insertTextAsync(const String& text, const EditingRange& replacemen
>         frame.editor().confirmComposition(text);
> }
> 
>+void WebPage::hasMarkedText(CompletionHandler<void(bool)>&& completionHandler)
>+{
>+    completionHandler(m_page->focusController().focusedOrMainFrame().editor().hasComposition());
>+}
>+
> void WebPage::getMarkedRangeAsync(CallbackID callbackID)
> {
>     Frame& frame = m_page->focusController().focusedOrMainFrame();
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index c486fc4671455c30c800298876f50e830a0712fb..88d37d38eca38130e492e9a050a637dc1e613613 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -789,6 +789,7 @@ public:
> 
>     void setTextAsync(const String&);
>     void insertTextAsync(const String& text, const EditingRange& replacementRange, InsertTextOptions&&);
>+    void hasMarkedText(CompletionHandler<void(bool)>&&);
>     void getMarkedRangeAsync(CallbackID);
>     void getSelectedRangeAsync(CallbackID);
>     void characterIndexForPointAsync(const WebCore::IntPoint&, CallbackID);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>index 941c60761d85e9bc7d9279f8be6f784fe122ffe1..4dc67bf9625bb2fccc8baff63f8245df9e2594b5 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>@@ -434,6 +434,7 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
> 
>     SetTextAsync(String text)
>     InsertTextAsync(String text, struct WebKit::EditingRange replacementRange, struct WebKit::InsertTextOptions options)
>+    HasMarkedText() -> (bool hasMarkedText) Async
>     GetMarkedRangeAsync(WebKit::CallbackID callbackID)
>     GetSelectedRangeAsync(WebKit::CallbackID callbackID)
>     CharacterIndexForPointAsync(WebCore::IntPoint point, WebKit::CallbackID callbackID);

Actions: View | Formatted Diff | Diff

Attachments on bug 198075: 370331 | 370335