Move IME related functions from WebFrame to WebLocalFrame

content/renderer/render_view_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_view_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <memory>
 
@@ skipping 1608 matching lines @@
 }
 
 void RenderViewImpl::didCancelCompositionOnSelectionChange() {
   Send(new InputHostMsg_ImeCancelComposition(GetRoutingID()));
 }
 
 bool RenderViewImpl::handleCurrentKeyboardEvent() {
   if (edit_commands_.empty())
     return false;
 
-  WebFrame* frame = webview()->focusedFrame();
+  WebLocalFrame* frame = webview()->focusedFrame();

[dcheng, 2016/06/13 17:29:45]

Ditto: why is |frame| null here? I wonder if this is really the right thing to
be doing: higher up in the stack, in KeyboardEventManager, we actually have a
frame. Should we be plumbing that through here instead?

   if (!frame)
     return false;
 
   EditCommands::iterator it = edit_commands_.begin();
   EditCommands::iterator end = edit_commands_.end();
 
   bool did_execute_command = false;
   for (; it != end; ++it) {
     // In gtk and cocoa, it's possible to bind multiple edit commands to one
     // key (but it's the exception). Once one edit command is not executed, it
@@ skipping 1065 matching lines @@
   PepperPluginInstanceImpl* focused_pepper_plugin = GetFocusedPepperPlugin();
   if (focused_pepper_plugin) {
     focused_pepper_plugin->render_frame()->OnImeSetComposition(
         text, underlines, selection_start, selection_end);
     return;
   }
 #endif  // ENABLE_PLUGINS
   if (replacement_range.IsValid() && webview()) {
     // Select the text in |replacement_range|, it will then be replaced by
     // text added by the call to RenderWidget::OnImeSetComposition().
-    if (WebLocalFrame* frame = webview()->focusedFrame()->toWebLocalFrame()) {
+    if (WebLocalFrame* frame = webview()->focusedFrame()) {

[dcheng, 2016/06/13 17:29:45]

I'm not sure this can ever be null.

toWebLocalFrame() asserts if the frame is not local.
And if focusedFrame() is null, then toWebLocalFrame() is null.

So do we need this conditional at all?

       WebRange webrange = WebRange::fromDocumentRange(
           frame, replacement_range.start(), replacement_range.length());
       if (!webrange.isNull())
         frame->selectRange(webrange);
     }
   }
   RenderWidget::OnImeSetComposition(text,
                                     underlines,
                                     replacement_range,
                                     selection_start,
                                     selection_end);
 }
 
 void RenderViewImpl::OnImeConfirmComposition(
     const base::string16& text,
     const gfx::Range& replacement_range,
     bool keep_selection) {
 #if defined(ENABLE_PLUGINS)
   PepperPluginInstanceImpl* focused_pepper_plugin = GetFocusedPepperPlugin();
   if (focused_pepper_plugin) {
     focused_pepper_plugin->render_frame()->OnImeConfirmComposition(
         text, replacement_range, keep_selection);
     return;
   }
 #endif  // ENABLE_PLUGINS
   if (replacement_range.IsValid() && webview()) {
     // Select the text in |replacement_range|, it will then be replaced by
     // text added by the call to RenderWidget::OnImeConfirmComposition().
-    if (WebLocalFrame* frame = webview()->focusedFrame()->toWebLocalFrame()) {
+    if (WebLocalFrame* frame = webview()->focusedFrame()) {

[dcheng, 2016/06/13 17:29:45]

Ditto.

       WebRange webrange = WebRange::fromDocumentRange(
           frame, replacement_range.start(), replacement_range.length());
       if (!webrange.isNull())
         frame->selectRange(webrange);
     }
   }
   RenderWidget::OnImeConfirmComposition(text,
                                         replacement_range,
                                         keep_selection);
 }
@@ skipping 52 matching lines @@
 
   if (!webview())
     return;
   size_t start_offset = 0;
   size_t character_count = 0;
   if (!webview()->compositionRange(&start_offset, &character_count))
     return;
   if (character_count == 0)
     return;
 
-  blink::WebFrame* frame = webview()->focusedFrame();
+  blink::WebLocalFrame* frame = webview()->focusedFrame();
   if (!frame)

[dcheng, 2016/06/13 17:29:45]

Similar question: can this actually return null? It seems like it could if you
called on a detached webview... but what are we doing calling this on a detached
webview then? I would expect webview() to be cleared.

     return;
 
   bounds_in_window->reserve(character_count);
   blink::WebRect webrect;
   for (size_t i = 0; i < character_count; ++i) {
     if (!frame->firstRectForCharacterRange(start_offset + i, 1, webrect)) {
       DLOG(ERROR) << "Could not retrieve character rectangle at " << i;
       bounds_in_window->clear();
       return;
     }
@@ skipping 400 matching lines @@
         return render_frame->focused_pepper_plugin();
     }
     frame = frame->traverseNext(false);
   }
 
   return nullptr;
 }
 #endif
 
 }  // namespace content