OOPIF IME: Renderer Side Changes

third_party/WebKit/Source/web/WebViewImpl.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 2302 matching lines @@
         }
     }
 }
 
 bool WebViewImpl::setComposition(
     const WebString& text,
     const WebVector<WebCompositionUnderline>& underlines,
     int selectionStart,
     int selectionEnd)
 {
+    // When mainFrameImpl() is nullptr/remote, IME is handled at the Widget
+    // belonging to the local root.
+    if (!mainFrameImpl())

[dcheng, 2016/07/12 14:43:21]

I don't understand why we need these early returns. Is there documentation that
describes the potential races with IME and how they're handled?

It also seems like these checks are duplicated a lot, so we should be able to
put them in a common helper somewhere.

[EhsanK, 2016/07/12 15:13:24]

This check is to make sure we only do IME inside a WebWidget. For the main
frame, it happens to be the WebViewImpl which is not swapped out (or whose
RenderWidget is not swapped out).

In WebFrameWidgetImpl, I added the method 'focusedLocalFrameInWidget()' which
guarantees we are dealing with the intended frame in the intended widget.

Here I did not add the method so I check every time (line 2334).

The reason that I have been doing this alot is that there is actual problems
with reporting the IME state from multiple Widgets on page which makes tracking
the state on the browser side impossible.

I guess having a helper class would go along way. Before, inside
WebFrameWidgetImpl, we were delegating all these calls to wiewImpl(). I can see
how both WebViewImpl() and WebWidget() could call a third helper class for this.

I can file a bug for it to follow up on if we will be OK with having the
duplicated code temporarily.

[dcheng, 2016/07/18 16:38:33]

As a starting point, I think we can have separate helpers for WebFrameWidgetImpl
and WebViewImpl? That should still reduce a bunch of code duplication.

[EhsanK, 2016/07/19 05:37:32]

If I have understood correctly, focusedLocalFrameInWidget() and
focusedLocalFrameAvailableForIme() should do this (added for both
WebFrameWidgetImpl and WebViewImpl). Although the latter method is basically the
first one but returning nullptr when |m_imeAcceptEvents| is false. 
I could remove this one and do this instead:
LocalFrame* focusedFrame = focusedLocalFrameInWidget();
if (!focusedFrame || !m_imeAcceptEvents)
    return false;

Which right now looks like:
LocalFrame* focusedFrame = focusedLocalFrameAvailableForIme();
if (!focusedFrame)
    return false;

+        return false;
+
     LocalFrame* focused = toLocalFrame(focusedCoreFrame());
     if (!focused || !m_imeAcceptEvents)
         return false;
 
+    // In case the focused frame has changed and IME on browser side has did
+    // not know this when it sent the IPC.
+    if (focused->localFrameRoot() != mainFrameImpl()->frame())
+        return false;
+
     if (WebPlugin* plugin = focusedPluginIfInputMethodSupported(focused))
         return plugin->setComposition(text, underlines, selectionStart, selectionEnd);
 
     // The input focus has been moved to another WebWidget object.
     // We should use this |editor| object only to complete the ongoing
     // composition.
     InputMethodController& inputMethodController = focused->inputMethodController();
     if (!focused->editor().canEdit() && !inputMethodController.hasComposition())
         return false;
 
@@ skipping 40 matching lines @@
 }
 
 bool WebViewImpl::confirmComposition(const WebString& text)
 {
     UserGestureIndicator gestureIndicator(DefinitelyProcessingNewUserGesture);
     return confirmComposition(text, DoNotKeepSelection);
 }
 
 bool WebViewImpl::confirmComposition(const WebString& text, ConfirmCompositionBehavior selectionBehavior)
 {
+    // When mainFrameImpl() is nullptr/remote, IME is handled at the Widget
+    // belonging to the local root.
+    if (!mainFrameImpl())
+        return false;
+
     LocalFrame* focused = toLocalFrame(focusedCoreFrame());
     if (!focused || !m_imeAcceptEvents)
         return false;
 
+    // In case the focused frame has changed and IME on browser side has did
+    // not know this when it sent the IPC.
+    if (focused->localFrameRoot() != mainFrameImpl()->frame())
+        return false;
+
     if (WebPlugin* plugin = focusedPluginIfInputMethodSupported(focused))
         return plugin->confirmComposition(text, selectionBehavior);
 
     return focused->inputMethodController().confirmCompositionOrInsertText(text, selectionBehavior == KeepSelection ? InputMethodController::KeepSelection : InputMethodController::DoNotKeepSelection);
 }
 
 bool WebViewImpl::compositionRange(size_t* location, size_t* length)
 {
+    // When mainFrameImpl() is nullptr/remote, IME is handled at the Widget
+    // belonging to the local root.
+    if (!mainFrameImpl())
+        return false;
+
     // FIXME: Long term, the focused frame should be a local frame. For now,
     // return early to avoid crashes.
     Frame* frame = focusedCoreFrame();
     if (!frame || frame->isRemoteFrame())
         return false;
 
     LocalFrame* focused = toLocalFrame(frame);
     if (!focused || !m_imeAcceptEvents)
         return false;
 
+    // In case the focused frame has changed and IME on browser side has did
+    // not know this when sent the IPC.
+    if (focused->localFrameRoot() != mainFrameImpl()->frame())
+        return false;
+
     const EphemeralRange range = focused->inputMethodController().compositionEphemeralRange();
     if (range.isNull())
         return false;
 
     Element* editable = focused->selection().rootEditableElementOrDocumentElement();
     DCHECK(editable);
     PlainTextRange plainTextRange(PlainTextRange::create(*editable, range));
     if (plainTextRange.isNull())
         return false;
     *location = plainTextRange.start();
     *length = plainTextRange.length();
     return true;
 }
 
 WebTextInputInfo WebViewImpl::textInputInfo()
 {
     WebTextInputInfo info;
 
+    // When mainFrameImpl() is nullptr/remote, IME is handled at the Widget
+    // belonging to the local root.
+    if (!mainFrameImpl())
+        return info;
+
     Frame* focusedFrame = focusedCoreFrame();
     if (!focusedFrame->isLocalFrame())
         return info;
 
     LocalFrame* focused = toLocalFrame(focusedFrame);
     if (!focused)
         return info;
 
+    // We can only have a text input type when focused frame belongs to this
+    // WebViewImpl, i.e., its local root is the main frame.
+    if (focused->localFrameRoot() != mainFrameImpl()->frame())
+        return info;
+
     FrameSelection& selection = focused->selection();
     if (!selection.isAvailable()) {
         // plugins/mouse-capture-inside-shadow.html reaches here.
         return info;
     }
     Element* element = selection.selection().rootEditableElement();
     if (!element)
         return info;
 
     info.inputMode = inputModeOfFocusedElement();
@@ skipping 29 matching lines @@
             info.compositionStart = plainTextRange.start();
             info.compositionEnd = plainTextRange.end();
         }
     }
 
     return info;
 }
 
 WebTextInputType WebViewImpl::textInputType()
 {
+    // When mainFrameImpl() is nullptr/remote, IME is handled at the Widget
+    // belonging to the local root.
+    if (!mainFrameImpl())
+        return WebTextInputTypeNone;
+
     LocalFrame* focusedFrame = m_page->focusController().focusedFrame();
     if (!focusedFrame)
         return WebTextInputTypeNone;
 
+    if (focusedFrame->localFrameRoot() != mainFrameImpl()->frame()) {
+        // We can only report a text input type when the focused frame belongs
+        // to this WebViewImpl, i.e., its local root is the main frame.
+        return WebTextInputTypeNone;
+    }
+
     if (!focusedFrame->selection().isAvailable()) {
         // "mouse-capture-inside-shadow.html" reaches here.
         return WebTextInputTypeNone;
     }
 
     // It's important to preserve the equivalence of textInputInfo().type and textInputType(),
     // so perform the same rootEditableElement() existence check here for consistency.
     if (!focusedFrame->selection().selection().rootEditableElement())
         return WebTextInputTypeNone;
 
@@ skipping 319 matching lines @@
     if (page())
         page()->pointerLockController().didLosePointerLock();
 }
 
 void WebViewImpl::didChangeWindowResizerRect()
 {
     if (mainFrameImpl()->frameView())
         mainFrameImpl()->frameView()->windowResizerRectChanged();
 }
 
+bool WebViewImpl::getCompositionCharacterBounds(WebVector<WebRect>& bounds)
+{
+    size_t offset = 0;
+    size_t characterCount = 0;
+    if (!compositionRange(&offset, &characterCount))
+        return false;
+
+    if (characterCount == 0)
+        return false;
+
+    WebLocalFrame* frame = focusedFrame();
+
+    // Only consider frames whose local root is the main frame. For other
+    // local frames which have different local roots, the corresponding
+    // WebFrameWidget will handle this task.
+    if (frame->localRoot() != mainFrameImpl())
+        return false;
+
+    WebVector<WebRect> result(characterCount);
+    WebRect webrect;
+    for (size_t i = 0; i < characterCount; ++i) {
+        if (!frame->firstRectForCharacterRange(offset + i, 1, webrect)) {
+            DLOG(ERROR) << "Could not retrieve character rectangle at " << i;
+            return false;
+        }
+        result[i] = webrect;
+    }
+    bounds.swap(result);
+    return true;
+}
+
+void WebViewImpl::applyReplacementRange(int start, int length)
+{
+    if (WebLocalFrame* frame = focusedFrame()) {
+        WebRange webrange = WebRange::fromDocumentRange(frame, start, length);
+        if (!webrange.isNull())
+            frame->selectRange(webrange);
+    }
+}
+
 // WebView --------------------------------------------------------------------
 
 WebSettingsImpl* WebViewImpl::settingsImpl()
 {
     if (!m_webSettings)
         m_webSettings = wrapUnique(new WebSettingsImpl(&m_page->settings(), m_devToolsEmulator.get()));
     DCHECK(m_webSettings);
     return m_webSettings.get();
 }
 
@@ skipping 1732 matching lines @@
 {
     // TODO(oshima): Investigate if this should return the ScreenInfo's scale factor rather than
     // page's scale factor, which can be 1 in use-zoom-for-dsf mode.
     if (!page())
         return 1;
 
     return page()->deviceScaleFactor();
 }
 
 } // namespace blink