Bug 130727 - Eliminate a sync cancelComposition call in WebPageProxy::editorStateChanged
Summary: Eliminate a sync cancelComposition call in WebPageProxy::editorStateChanged
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on: 130748
Blocks:
  Show dependency treegraph
 
Reported: 2014-03-25 10:25 PDT by Alexey Proskuryakov
Modified: 2014-03-28 12:11 PDT (History)
4 users (show)

See Also:


Attachments
proposed patch (8.65 KB, patch)
2014-03-27 15:16 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
with a build fix (8.68 KB, patch)
2014-03-28 09:38 PDT, Alexey Proskuryakov
enrica: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2014-03-25 10:25:59 PDT
We never want to make sync messages from UI Process to WebProcess. This one is particularly silly, as WebCore has more information to decide whether a selection change is such that should abandon the input session (e.g. using JS to remove the whole element that's being edited).

For now, I'd like to move the decision to WebProcess, eliminating the sync call. There is only one other instance of cancelComposition which I'll tackle separately (it's in -resignFirstResponder), and then we can move the message under an #ifdef.
Comment 1 Alexey Proskuryakov 2014-03-27 15:16:01 PDT
Created attachment 227992 [details]
proposed patch
Comment 2 Alexey Proskuryakov 2014-03-28 09:38:11 PDT
Created attachment 228056 [details]
with a build fix
Comment 3 Simon Fraser (smfr) 2014-03-28 11:51:48 PDT
Comment on attachment 228056 [details]
with a build fix

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4114
> +printf("didChangeSelection. hasComposition = %uï¼ignoreCompositionSelectionChange ï¼%u, isNone = %u\n", frame.editor().hasComposition(), frame.editor().ignoreCompositionSelectionChange(), frame.selection().isNone());

You have a printf here.
Comment 4 Enrica Casucci 2014-03-28 12:03:10 PDT
Comment on attachment 228056 [details]
with a build fix

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

>> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:4114
>> +printf("didChangeSelection. hasComposition = %uï¼ignoreCompositionSelectionChange ï¼%u, isNone = %u\n", frame.editor().hasComposition(), frame.editor().ignoreCompositionSelectionChange(), frame.selection().isNone());
> 
> You have a printf here.

You probably don't want to leave this in :-)
Comment 5 Alexey Proskuryakov 2014-03-28 12:11:52 PDT
Committed <http://trac.webkit.org/r166421>.