Make limitations for input.ime.sendKeyEvents API.

chrome/browser/ui/input_method/input_method_engine.cc

Index: chrome/browser/ui/input_method/input_method_engine.cc
diff --git a/chrome/browser/ui/input_method/input_method_engine.cc b/chrome/browser/ui/input_method/input_method_engine.cc
index 81a8f4cac492fad5ae3dff8a4eca70e5d67a154e..4c832be9321302d325ea15992b634d4c901979df 100644
--- a/chrome/browser/ui/input_method/input_method_engine.cc
+++ b/chrome/browser/ui/input_method/input_method_engine.cc
@@ -4,7 +4,16 @@
 
 #include "chrome/browser/ui/input_method/input_method_engine.h"
 
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/common/url_constants.h"
+#include "extensions/common/constants.h"
+#include "ui/aura/window.h"
+#include "ui/aura/window_tree_host.h"
 #include "ui/base/ime/composition_text.h"
 #include "ui/base/ime/ime_bridge.h"
 #include "ui/base/ime/ime_input_context_handler_interface.h"
@@ -185,6 +194,11 @@ ui::ImeWindow* InputMethodEngine::FindWindowById(int window_id) const {
 bool InputMethodEngine::SendKeyEvent(ui::KeyEvent* event,
                                      const std::string& code) {
   DCHECK(event);
+
+  // input.ime.sendKeyEvents API is only allowed to work on text fields.
+  if (current_input_type_ == ui::TEXT_INPUT_TYPE_NONE)
+    return false;
+
   if (event->key_code() == ui::VKEY_UNKNOWN)
     event->set_key_code(ui::DomCodeToUsLayoutKeyboardCode(event->code()));
 
@@ -192,9 +206,66 @@ bool InputMethodEngine::SendKeyEvent(ui::KeyEvent* event,
       ui::IMEBridge::Get()->GetInputContextHandler();
   if (!input_context)
     return false;
+
+  Browser* browser = chrome::FindLastActive();

[Shu Chen, 2016/06/29 13:18:19]

IMO, IsValidKeyForSpecialPage() may run faster than the following code.

So, maybe it worths to check IsValidKeyForSpecialPage() earlier at here?

[Azure Wei, 2016/06/30 07:46:20]

Done. And renamed as IsValidKeyForAllPages().

+  DCHECK(browser);
+  content::WebContents* web_contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  ui::InputMethod* input_method =
+      browser->window()->GetNativeWindow()->GetHost()->GetInputMethod();
+
+  if (web_contents) {
+    // input.ime.sendKeyEvents API is not allowed to send ENTER et al. keys on
+    // some special pages (like chrome://).
+    GURL url = web_contents->GetLastCommittedURL();
+    // If |input_context->GetInputMethod() != input_method|, treats |url| as
+    // special page no mater what it is.
+    bool is_special_page = input_context->GetInputMethod() != input_method;
+    if (!is_special_page) {
+      is_special_page = true;

[Devlin, 2016/06/29 16:07:23]

Hmm... sorry, this still looks a little weird to me.  It's just hard to parse
all the times these bools change at a quick glance, and a little hard to
understand what the code is actually doing.  What if we actually move the
IsSpecialPage logic into its own method (and keep the IsValidKeyForSpecialPage
in its own method too).  So:

bool IsSpecialPage(content::WebContents* web_contents, InputMethod* method) {
  if (!web_contents)
    return true;
  // If |input_context->GetInputMethod() != input_method|, treats |url| as
  // special page no mater what it is.
  if (web_contents->window()->GetNativeWindow()->GetHost()->GetInputMethod() !=
          input_method)
    return true;
  std::vector<const char*> whitelist_schemes{
      url::kFtpScheme, url::kHttpScheme, url::kHttpsScheme};
  GURL url = web_contents->GetLastCommittedURL();
  // If we can't determine the last committed url, treat it as special.
  if (!url.is_valid())
    return true;
  for (auto scheme : whitelist_schemes) {
    if (url.SchemeIs(scheme))
      return false;
  }
  Origin origin(url);
  std::vector<GURL> whitelist_urls{GURL(url::kAboutBlankURL),
                                   GURL(chrome::kChromeUINewTabURL)};
  for (const GURL& whitelist_url : whitelist_urls) {
    if (Origin(whitelist_url).IsSameOriginWith(origin))
      return false;
  }
  return true;
}


and then here:

if (!IsValidKeyForSpecialPage(event)) {
  <GetWebContents>
  if (IsSpecialPage(web_contents, input_method))
    return false;
}

input_context->SendKeyEvent(event);
return true;


Sorry for the back-and-forth on this - it's just a pretty subtle method, and I
wanna maximize readability so that we can understand what's going on (and be
less likely to break it in the future :)).

[Azure Wei, 2016/06/30 07:46:20]

Done.
Added the new method IsSpecialPage(ui::InputMethod* input_method), since all the
work of getting web contents and url could be pulled out from SendKeyEvent. 

+      // Checks if the current url is with whitelisted scheme.
+      std::vector<const char*> whitelist_schemes{
+          url::kFtpScheme, url::kHttpScheme, url::kHttpsScheme};
+      for (auto scheme : whitelist_schemes) {
+        if (url.SchemeIs(scheme)) {
+          is_special_page = false;
+          break;
+        }
+      }
+      // Checks if the current url is whitelisted url.
+      std::vector<GURL> whitelist_urls{GURL(url::kAboutBlankURL),
+                                       GURL(chrome::kChromeUINewTabURL)};
+      for (auto whitelist_url : whitelist_urls) {
+        if (url == whitelist_url || url.GetOrigin() == whitelist_url) {
+          is_special_page = false;
+          break;
+        }
+      }
+    }
+    if (is_special_page && !IsValidKeyForSpecialPage(event))
+      return false;
+  }
+
   input_context->SendKeyEvent(event);
 
   return true;
 }
 
+bool InputMethodEngine::IsValidKeyForSpecialPage(ui::KeyEvent* ui_event) {
+  // Whitelists all character keys except for Enter and Tab keys.
+  std::vector<ui::KeyboardCode> invalid_character_keycodes{ui::VKEY_TAB,
+                                                           ui::VKEY_RETURN};
+  if (ui_event->GetDomKey().IsCharacter() && !ui_event->IsControlDown() &&
+      !ui_event->IsCommandDown())
+    return std::count(invalid_character_keycodes.begin(),
+                      invalid_character_keycodes.end(),
+                      ui_event->key_code()) == 0;
+
+  // Whitelists Backspace key and arrow keys.
+  std::vector<ui::KeyboardCode> whitelist_keycodes{
+      ui::VKEY_BACK, ui::VKEY_LEFT, ui::VKEY_RIGHT, ui::VKEY_UP, ui::VKEY_DOWN};
+  return std::count(whitelist_keycodes.begin(), whitelist_keycodes.end(),
+                    ui_event->key_code()) != 0;
+}
+
 }  // namespace input_method