Remove the CHECK in RenderWidget::GetInputMethodController

third_party/WebKit/public/web/WebWidget.h

 /*
  * Copyright (C) 2009 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 214 matching lines @@
   // composition. Returns false if there is no focused input or any ongoing
   // composition.
   virtual bool getCompositionCharacterBounds(WebVector<WebRect>& bounds) {
     return false;
   }
 
   // Applies the range on the focused frame so that the text will later be
   // replaced.
   virtual void applyReplacementRange(const WebRange&) {}
 
+  // Current instance of the active WebInputMethodController, that is, the
+  // WebInputMethodController corresponding to (and owned by) the focused
+  // WebLocalFrameImpl. It might return nullptr when there are no focused
+  // frames or possibly when the WebFrameWidget does not accept IME events.
+  // This should only be overriden by frame widgets.
+  virtual WebInputMethodController* getActiveWebInputMethodController() {

[dcheng, 2016/12/05 19:19:49]

Please add a comment here that the presence of this method in WebWidget is
temporary (the fact that we're getting it on something not associated iwth a
WebFrameWidget means the browser is routing events to the wrong processes), and
reference the relevant bugs.

[lfg, 2016/12/05 19:26:33]

When I reviewed this, I considered this to be a more permanent solution (i.e., I
thought it made sense to associate a WebInputMehtodController with a WebWidget,
instead of just a WebFrameWidget).

If we are doing this as a temporary fix, I'd rather use isWebFrameWidget() when
calling getActiveWebInputMethodController() in RenderWidget.

[EhsanK, 2016/12/05 19:50:48]

dcheng@: I actually think it might be better to have this method in WebWidget.h
for a while (even after the IPC issues is fixed).

I think text input and IME are not necessarily limited to WebFrameWidget.
Specifically, InputMethodController belongs to frame and
WebInputMethodController is supposed to be the public wrapper. Methods such as
setComposition() should be called on a WebFrameWidget. But textInputInfo() and
textInputType() might have to stay inside WebWidget for a while. The logic for
those is inside InputMethodController and to have them inside
WebInputMethodController we will need to make the WebInputMethodController
accessible through WebWidget for now (I encountered them in this refactoring CL:
https://codereview.chromium.org/2508363003/ which is now blocked on this bug).

In 'render_widget_unittest.cc', for example, we have a MockWebWidget which is a
WebWidget and not a WebFrameWidget. Are we switching all those to
WebFrameWidget? I guess not. So I think at least for a while
getActiveInputMethodController() should be inside WebWidget and putting it in
WebFrameWidget was not great.

In summary, my suggestion is to keep the method in WebWidget. Finish all the IME
and text input refactor and then revisit this if it is 'required' to move this
to WebFrameWidget. WDYT?

+    return nullptr;
+  }
+
  protected:
   ~WebWidget() {}
 };
 
 }  // namespace blink
 
 #endif