Allow webdriver users to choose between sending the key events when...

chrome/browser/automation/testing_automation_provider.cc

Index: chrome/browser/automation/testing_automation_provider.cc
===================================================================
--- chrome/browser/automation/testing_automation_provider.cc	(revision 77600)
+++ chrome/browser/automation/testing_automation_provider.cc	(working copy)
@@ -2142,6 +2142,8 @@
       &TestingAutomationProvider::WebkitMouseDrag;
   handler_map["SendWebkitKeyEvent"] =
       &TestingAutomationProvider::SendWebkitKeyEvent;
+  handler_map["SendOSLevelKeyEvent"] =
+      &TestingAutomationProvider::SendOSLevelKeyEvent;
   handler_map["ActivateTab"] =
       &TestingAutomationProvider::ActivateTabJSON;
 #if defined(OS_CHROMEOS)
@@ -4587,104 +4589,230 @@
   base::CloseProcessHandle(process);
 }
 
-void TestingAutomationProvider::SendWebkitKeyEvent(
+bool TestingAutomationProvider::BuildNativeWebKeyEventFromArgs(
     DictionaryValue* args,
-    IPC::Message* reply_message) {
-  TabContents* tab_contents;
-  std::string error;
-  if (!GetTabFromJSONArgs(args, &tab_contents, &error)) {
-    AutomationJSONReply(this, reply_message).SendError(error);
-    return;
+    IPC::Message* reply_message,
+    NativeWebKeyboardEvent* event) {
+  int type, modifiers;
+  string16 unmodified_text, text;
+  std::string key_identifier;
+  if (!args->GetInteger("type", &type)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'type' missing or invalid.");
+    return false;
   }
+  if (!args->GetInteger("nativeKeyCode", &event->nativeKeyCode)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'nativeKeyCode' missing or invalid.");
+    return false;
+  }
+  if (!args->GetInteger("windowsKeyCode", &event->windowsKeyCode)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'windowsKeyCode' missing or invalid.");
+    return false;
+  }
+  if (!args->GetInteger("modifiers", &modifiers)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'modifiers' missing or invalid.");
+    return false;
+  }
 
+  if (type == automation::kRawKeyDownType) {
+    event->type = WebKit::WebInputEvent::RawKeyDown;
+  } else if (type == automation::kKeyDownType) {
+    event->type = WebKit::WebInputEvent::KeyDown;
+  } else if (type == automation::kKeyUpType) {
+    event->type = WebKit::WebInputEvent::KeyUp;
+  } else if (type == automation::kCharType) {
+    event->type = WebKit::WebInputEvent::Char;
+  } else {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'type' refers to an unrecognized keyboard event type");
+    return false;
+  }
+
+  event->modifiers = 0;
+  if (modifiers & automation::kShiftKeyMask)
+    event->modifiers |= WebKit::WebInputEvent::ShiftKey;
+  if (modifiers & automation::kControlKeyMask)
+    event->modifiers |= WebKit::WebInputEvent::ControlKey;
+  if (modifiers & automation::kAltKeyMask)
+    event->modifiers |= WebKit::WebInputEvent::AltKey;
+  if (modifiers & automation::kMetaKeyMask)
+    event->modifiers |= WebKit::WebInputEvent::MetaKey;
+
+  event->isSystemKey = false;
+  event->timeStampSeconds = base::Time::Now().ToDoubleT();
+  event->skip_in_browser = true;
+  return true;
+}
+
+bool TestingAutomationProvider::BuildWebKeyEventFromArgs(
+    DictionaryValue* args,
+    IPC::Message* reply_message,
+    NativeWebKeyboardEvent* event) {
+
   int type, modifiers;
   bool is_system_key;
   string16 unmodified_text, text;
   std::string key_identifier;
-  NativeWebKeyboardEvent event;
-  if (!args->GetInteger("type", &type)) {
-    AutomationJSONReply reply(this, reply_message);
-    reply.SendError("'type' missing or invalid.");
-    return;
+    if (!args->GetInteger("type", &type)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'type' missing or invalid.");
+    return false;
   }
   if (!args->GetBoolean("isSystemKey", &is_system_key)) {
-    AutomationJSONReply reply(this, reply_message);
-    reply.SendError("'isSystemKey' missing or invalid.");
-    return;
+    AutomationJSONReply(this, reply_message)
+        .SendError("'isSystemKey' missing or invalid.");
+    return false;
   }
   if (!args->GetString("unmodifiedText", &unmodified_text)) {
-    AutomationJSONReply reply(this, reply_message);
-    reply.SendError("'unmodifiedText' missing or invalid.");
-    return;
+    AutomationJSONReply(this, reply_message)
+        .SendError("'unmodifiedText' missing or invalid.");
+    return false;
   }
   if (!args->GetString("text", &text)) {
-    AutomationJSONReply reply(this, reply_message);
-    reply.SendError("'text' missing or invalid.");
-    return;
+    AutomationJSONReply(this, reply_message)
+        .SendError("'text' missing or invalid.");
+    return false;
   }
-  if (!args->GetInteger("nativeKeyCode", &event.nativeKeyCode)) {
-    AutomationJSONReply reply(this, reply_message);
-    reply.SendError("'nativeKeyCode' missing or invalid.");
-    return;
+  if (!args->GetInteger("nativeKeyCode", &event->nativeKeyCode)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'nativeKeyCode' missing or invalid.");
+    return false;
   }
-  if (!args->GetInteger("windowsKeyCode", &event.windowsKeyCode)) {
-    AutomationJSONReply reply(this, reply_message);
-    reply.SendError("'windowsKeyCode' missing or invalid.");
-    return;
+  if (!args->GetInteger("windowsKeyCode", &event->windowsKeyCode)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("'windowsKeyCode' missing or invalid.");
+    return false;
   }
   if (!args->GetInteger("modifiers", &modifiers)) {
-    AutomationJSONReply reply(this, reply_message);
-    reply.SendError("'modifiers' missing or invalid.");
-    return;
+    AutomationJSONReply(this, reply_message)
+        .SendError("'modifiers' missing or invalid.");
+    return false;
   }
   if (args->GetString("keyIdentifier", &key_identifier)) {
-    base::strlcpy(event.keyIdentifier,
+    base::strlcpy(event->keyIdentifier,
                   key_identifier.c_str(),
                   WebKit::WebKeyboardEvent::keyIdentifierLengthCap);
   } else {
-    event.setKeyIdentifierFromWindowsKeyCode();
+    event->setKeyIdentifierFromWindowsKeyCode();
   }
 
   if (type == automation::kRawKeyDownType) {
-    event.type = WebKit::WebInputEvent::RawKeyDown;
+    event->type = WebKit::WebInputEvent::RawKeyDown;
   } else if (type == automation::kKeyDownType) {
-    event.type = WebKit::WebInputEvent::KeyDown;
+    event->type = WebKit::WebInputEvent::KeyDown;
   } else if (type == automation::kKeyUpType) {
-    event.type = WebKit::WebInputEvent::KeyUp;
+    event->type = WebKit::WebInputEvent::KeyUp;
   } else if (type == automation::kCharType) {
-    event.type = WebKit::WebInputEvent::Char;
+    event->type = WebKit::WebInputEvent::Char;
   } else {
-    AutomationJSONReply reply(this, reply_message);
-    reply.SendError("'type' refers to an unrecognized keyboard event type");
-    return;
+    AutomationJSONReply(this, reply_message)
+        .SendError("'type' refers to an unrecognized keyboard event type");
+    return false;
   }
 
   string16 unmodified_text_truncated = unmodified_text.substr(
       0, WebKit::WebKeyboardEvent::textLengthCap - 1);
-  memcpy(event.unmodifiedText,
+  memcpy(event->unmodifiedText,
          unmodified_text_truncated.c_str(),
          unmodified_text_truncated.length() + 1);
   string16 text_truncated = text.substr(
       0, WebKit::WebKeyboardEvent::textLengthCap - 1);
-  memcpy(event.text, text_truncated.c_str(), text_truncated.length() + 1);
+  memcpy(event->text, text_truncated.c_str(), text_truncated.length() + 1);
 
-  event.modifiers = 0;
+  event->modifiers = 0;
   if (modifiers & automation::kShiftKeyMask)
-    event.modifiers |= WebKit::WebInputEvent::ShiftKey;
+    event->modifiers |= WebKit::WebInputEvent::ShiftKey;
   if (modifiers & automation::kControlKeyMask)
-    event.modifiers |= WebKit::WebInputEvent::ControlKey;
+    event->modifiers |= WebKit::WebInputEvent::ControlKey;
   if (modifiers & automation::kAltKeyMask)
-    event.modifiers |= WebKit::WebInputEvent::AltKey;
+    event->modifiers |= WebKit::WebInputEvent::AltKey;
   if (modifiers & automation::kMetaKeyMask)
-    event.modifiers |= WebKit::WebInputEvent::MetaKey;
+    event->modifiers |= WebKit::WebInputEvent::MetaKey;
 
-  event.isSystemKey = is_system_key;
-  event.timeStampSeconds = base::Time::Now().ToDoubleT();
-  event.skip_in_browser = true;
+  event->isSystemKey = is_system_key;
+  event->timeStampSeconds = base::Time::Now().ToDoubleT();
+  event->skip_in_browser = true;
+  return true;
+}
+
+void TestingAutomationProvider::SendWebkitKeyEvent(
+    DictionaryValue* args,
+    IPC::Message* reply_message) {
+  NativeWebKeyboardEvent event;
+  // BuildWebKeyEventFromArgs handles telling what when wrong and sending
+  // the reply message, if it failed we just have to stop here.
+  if (!BuildWebKeyEventFromArgs(args, reply_message, &event))
+    return;
+
+  TabContents* tab_contents;
+  std::string error;
+  if (!GetTabFromJSONArgs(args, &tab_contents, &error)) {
+    AutomationJSONReply(this, reply_message).SendError(error);
+    return;
+  }
+  LOG(INFO) << "reply_message:" << reply_message;
   new InputEventAckNotificationObserver(this, reply_message, event.type);
   tab_contents->render_view_host()->ForwardKeyboardEvent(event);
 }
 
+void TestingAutomationProvider::SendOSLevelKeyEvent(
+    DictionaryValue* args,
+    IPC::Message* reply_message) {
+  NativeWebKeyboardEvent event;
+  // BuildNativeWebKeyEventFromArgs handles telling what when wrong and sending
+  // the reply message, if it failed we just have to stop here.
+  if (!BuildNativeWebKeyEventFromArgs(args, reply_message, &event))

[kkania, 2011/03/10 17:00:47]

it seems a bit strange to me how we are building a NativeWebKeyboardEvent, then
stripping a couple items from it to send to SendKeyPress. Can we cut out the
middle step?

[timothe faudot, 2011/03/11 03:32:08]

Done.

+    return;
+
+  Browser* browser;
+  std::string error;
+  if (!GetBrowserFromJSONArgs(args, &browser, &error)) {
+    AutomationJSONReply(this, reply_message)

[kkania, 2011/03/10 17:00:47]

Since you send a reply synchronously in this func, you can just create an
AutomationJSONReply reply; at top and just reply.SendError/Success to reduce
code clutter

[timothe faudot, 2011/03/11 03:32:08]

Done.

+        .SendError("Could not get the browser handle");

[kkania, 2011/03/10 17:00:47]

you can just .SendError(error) here

[timothe faudot, 2011/03/11 03:32:08]

Done.

+    return;
+  }
+  // The ui_controls::SendKeyPress function will generate up and down events

[kkania, 2011/03/10 17:00:47]

instead of having a strange interface where we don't process half the events
sent to us, can we just change this func to SendOSLevelKeyPress and drop the
need for the type arg? Also, how about dropping the nativeKeyCode and
indentifier params too?

[timothe faudot, 2011/03/11 03:32:08]

Done.

+  // from one keycode for us so we only need to send one event to it.
+  // Filtering is done via the RawKeyDown type because it is the one sent by
+  // the current webdriver implementation of WebElement.SendKeys(...).
+  DictionaryValue* reply_details = new DictionaryValue;

[kkania, 2011/03/10 17:00:47]

reply.SendSuccess does not take ownership of the value. Create the
DictionaryValue on the stack instead of the heap.

[timothe faudot, 2011/03/11 03:32:08]

Done.
Due to the changes up, the return dict isn't needed anymore, the key type
filtering is done before in the automation pipeline, thus also reducing the
useless
calls to the automation proxy.
Anyway the way of returning dicts is not standardized across the other methods
in this file, how about another CL to decide that? some methods use new
DicitonaryValue, some use scoped pointers and get(), etc.

+  if (event.type != WebKit::WebInputEvent::RawKeyDown) {
+    AutomationJSONReply reply(this, reply_message);
+    reply_details->SetBoolean("processed", false);
+    reply.SendSuccess(reply_details);
+    return;
+  }
+  BrowserWindow* browser_window = browser->window();
+  if (!browser_window) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("Could not get the browser window");
+    return;
+  }
+  gfx::NativeWindow window = browser_window->GetNativeHandle();
+  if (!window) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("Could not get the browser window handle");
+    return;
+  }
+  bool control = !!(event.modifiers & WebKit::WebInputEvent::ControlKey);

[kkania, 2011/03/10 17:00:47]

I am a bit surprised you need this !! dance. What warning does the compiler give
without it?

[timothe faudot, 2011/03/11 03:32:08]

There is now warning, it's just a common method to ensure correct typecast to
bool, that was suggested by a previous reviewer. If you feel it's more troubling
than useful I can remove them.

+  bool shift = !!(event.modifiers & WebKit::WebInputEvent::ShiftKey);
+  bool alt = !!(event.modifiers & WebKit::WebInputEvent::AltKey);
+  bool meta = !!(event.modifiers & WebKit::WebInputEvent::MetaKey);
+  if (!ui_controls::SendKeyPress(
+          window, static_cast<ui::KeyboardCode>(event.nativeKeyCode),
+          control, shift, alt, meta)) {
+    AutomationJSONReply(this, reply_message)
+        .SendError("Could not send the native key event");
+  } else {
+    AutomationJSONReply reply(this, reply_message);
+    reply_details->SetBoolean("processed", true);
+    reply.SendSuccess(reply_details);
+  }
+}
+
 // Sample JSON input: { "command": "GetNTPThumbnailMode" }
 // For output, refer to GetNTPThumbnailMode() in
 // chrome/test/pyautolib/pyauto.py.