New issue
Advanced search Search tips

Issue 643233 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

Revisit TextInputClientMac reply handler mechanism when there are multiple RenderWidgetHosts

Project Member Reported by ekaramad@chromium.org, Sep 1 2016

Issue description

When there are multiple RenderWidgetHosts using TextInputClient, we might override the callback for one slow RWH with another before the first one even arrives. This mechanism should be revisited.
 
Even worse, if the second one arrives while we have cleared the first, we end up triggering the first one. This could lead to requesting a meaning a word on a tab (Ctrl+Command+D) and then switching tabs requesting again but getting the first one in place of the second.

We might also want to rethink the design of this module. Right now out callbacks which are invoked in response to the IPC's from renderer are run on IO thread, which makes it hard to use TransformPointToRootCoordSpace without thread hopping. Maybe we should change the message filter thread to receive it on UI and then use Transform... instead of manually fixing the positions.
(A CL related to this bug has landed but not posted):

The following revision refers to this bug:
commit 67a361af4485aee793aa2ad8dacddd249d19fc1c
Author: ekaramad <ekaramad@chromium.org>
Date:   Tue Apr 25 15:45:38 2017 -0700

Making Mac Dictionary better for OOPIFs and <webview>
    
There is two ways to invoke Mac dictionary for Chromium: choosing look-up word
in context menu for highlighted text, or pressing Ctrl + Command + D. Both methods
might involve sending an IPC through TextInputClientMac to the RenderWidget
corresponding to the string (i.e., focused RenderWidget or the RenderWidget at
the position of mouse cursor). In both cases the IPC comes back with a string
and a position with respect to the local roots view port coordinates. The result
however is handled on IO thread which leads to complications such as a DCHECK
firing (at times) due to accessing the RenderWidgetHostViews weak pointer when
getting the focused RenderWidgetHost.

Furthermore, word lookup through context menu is broken for <webview>s; both
OOPIF-based and BrowserPlugin-based. The former type of <webview>s don't
support this feature because RenderWidgetHostViewChildFrame does not implement
ShowDefinitionForSelection() method. The latter have regressed after implementing
Mac dictionary support for OOPIFs () where the IPC for getting the string from
range is only sent to a RenderWidgetHost which is part of the frame tree (which
will not be that of a guest represented by a BrowserPlugin).

This CL will:

1) Handle dictionary related IPCs on the UI thread to remove the threading issue
and make transforming the reported points to root coordinates based on compositor
frames and the right OOPIF way (calling RWHV::TransformPointToRooTCoordSpace which
can only be done on the UI thread).

2) Implement RenderWidgetHostViewChildFrame::ShowDefinitionForSelection().
3) Make sure that for BrowserPlugin-based guest the IPC is sent to the
RenderWidget of the guest renderer.

The CL also adds a test to avoid regressions in word lookup inside guests.

TBR=shuchen@chromium.org
BUG=643233,   702348  , 708183
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

Sign in to add a comment