New issue
Advanced search Search tips

Issue 618774 link

Starred by 1 user
Status: Fixed
Owner:
ekaramad@chromium.org
Closed: Jul 2016
Cc:
lfg@chromium.org
kenrb@chromium.org
dcheng@chromium.org
wfh@chromium.org
creis@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Add means to WebFrameWidget distinguish between in-process focused frame and those inside WebFrameWidget itself.

Project Member Reported by ekaramad@chromium.org, Jun 9 2016 
Currently, throughout the code in WebFrameWidgetImpl (and WebViewImpl when they overlap) we obtain the focused frame from page()->focusController(). Therefore, all widgets in the same process can see a focused frame whether or not it shares the same local root as that of the widgets themselves. We should have a means to identify a focused  frame which "in the widget" as in they both have the same local root.

This is crucial in IME related tasks, such as WebWidget::textInputInfo(), WebWidget::selectionBounds(), WebWidget::textInputType(), etc. which obtain their required state from the focused frame. For example, in a scenario such as:

 -A-------------------
| -B----------------- |
|| -A--------------- ||
|||     [text]      |||
|| ----------------- ||
| ------------------- |
 ---------------------

The TextInputState could be reported from both WebViewImpl and WebFrameWidget for the inner most frame. This will cause issues in tracking the right state (i.e., incorrect selection bounds will lead to IME drop down list to show at incorrect position).
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 19 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a46d63b3c442f1f0d26cd1484c25eb766a064bc

commit 2a46d63b3c442f1f0d26cd1484c25eb766a064bc
Author: ekaramad <ekaramad@chromium.org>
Date: Tue Jul 19 13:33:09 2016

OOPIF IME: Renderer Side Changes

Currently, all IME-related IPC are handled at the RenderViewImpl first
before calling the base class
methods on RenderWidget (ImeSetComposition and
ImeConfirmComposition). Further, to query IME-related
state, e.g., GetTextInputType(), RenderViewImpl overrides the method
from RenderWidget so that it can access the
state from pepper plugin.

These overrides are needed to either:
1- Access focused PepperPluginInstacneImpl to set/get IME-related
state.
2- Use focused frame to set/get IME-related state.

This CL removes all those methods from RenderViewImpl and handles
them in RenderWidget. To this end, a reference to
the focused plugin is kept in RenderWidget and updated by its owning
RenderFrame (to get IME-related state). Also, new API is added to blink
to enable RenderWidget handle IME tasks related to focusedFrame.

In line with the separation of WebViewImpl from WebWidget,
as well as avoiding potential race conditions between
mutiple Widgets on the same process, most of the IME-
related WebWidget methods are now specifically
implemented for WebFrameWidgetImpl.

Finally, a new private API is added to WebFrameWidgetImpl to help the
widget distinguish between in process frames which do or do not belong
to widget's local root.

BUG= 578168 , 618774 , 626746 

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

[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/content/renderer/render_view_impl.cc
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/content/renderer/render_view_impl.h
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/content/renderer/render_widget.cc
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/content/renderer/render_widget.h
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/third_party/WebKit/Source/web/WebFrameWidgetImpl.h
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/2a46d63b3c442f1f0d26cd1484c25eb766a064bc/third_party/WebKit/public/web/WebWidget.h

Comment 2 by ekaramad@chromium.org, Jul 19 2016

Status: Fixed (was: Assigned)
Closing this bug as the CL above adds the methods required.
►

Sign in to add a comment