Handling PNaCl KeyboardInputEvent(s) in the key tester app.

remoting/tools/javascript_key_tester/main.js

Index: remoting/tools/javascript_key_tester/main.js
diff --git a/remoting/tools/javascript_key_tester/main.js b/remoting/tools/javascript_key_tester/main.js
index e777b48166945762201a344cdff73988519586bb..93ed809b3514a2716a3a5da728eef7e878422d91 100644
--- a/remoting/tools/javascript_key_tester/main.js
+++ b/remoting/tools/javascript_key_tester/main.js
@@ -3,17 +3,143 @@
  * found in the LICENSE file.
  */
 
+/**
+ * @param {string} eventName
+ * @param {number|undefined} space

[Jamie, 2015/01/30 23:25:09]

{number=} is a simpler annotation, though you should rename it opt_space in that
case. You can them just omit the parameter instead of passing undefined
explicitly.

[Łukasz Anforowicz, 2015/01/31 00:44:49]

Done.  Renamed to "opt_space" as well.  (are you sure this shouldn't be
"optSpace"?)

[Jamie, 2015/01/31 01:01:56]

opt_ as a prefix for optional parameters is a common convention, even though (or
maybe because) it goes against the convention for other names.

+ * @return {string}
+ */
+function JsonifyJavascriptKeyEvent(event, eventName, space) {

[Jamie, 2015/01/30 23:25:09]

Function names in JS don't start with a capital letter (with the exception of
ctors, since they effectively define types).

[Łukasz Anforowicz, 2015/01/31 00:44:49]

Done.

+  return "JavaScript '" + eventName + "' event = " + JSON.stringify(
+      event,
+      ['type', 'alt', 'shift', 'control', 'meta', 'charCode', 'keyCode',
+          'keyIdentifier', 'repeat'],
+      space);
+};
+
+/**
+ * @param {ChordTracker} jsChordTracker
+ * @param {Event} event
+ * @return {void}
+ */
+function HandleJavascriptKeyDownEvent(jsChordTracker, event) {
+  AppendToTextLog(JsonifyJavascriptKeyEvent(event, 'keydown', undefined));
+  jsChordTracker.addKeyDownEvent(
+      event.keyCode, JsonifyJavascriptKeyEvent(event, 'keydown', 2));
+}
+
+/**
+ * @param {ChordTracker} jsChordTracker
+ * @param {Event} event
+ * @return {void}
+ */
+function HandleJavascriptKeyUpEvent(jsChordTracker, event) {
+  AppendToTextLog(JsonifyJavascriptKeyEvent(event, 'keyup', undefined));
+  jsChordTracker.addKeyUpEvent(
+      event.keyCode, JsonifyJavascriptKeyEvent(event, 'keyup', 2));
+}
+
+/** @constructor */
+var PNaClEvent = function(){

[Jamie, 2015/01/30 23:25:09]

Nit: Space after ()

[Łukasz Anforowicz, 2015/01/31 00:44:49]

Done.

+  /** @type {string} */
+  this.type = "";
+  /** @type {number} */
+  this.modifiers = 0;
+  /** @type {number} */
+  this.keyCode = 0;
+  /** @type {string|undefined} */
+  this.characterText = undefined;
+  /** @type {string} */
+  this.code = "";
+};
+
+/**
+ * @param {PNaClEvent} event
+ * @param {number|undefined} space
+ * @return {string}
+ */
+function JsonifyPnaclKeyboardInputEvent(event, space) {
+  return "PNaCl KeyboardInputEvent = " + JSON.stringify(
+      event,
+      ['type', 'modifiers', 'keyCode', 'characterText', 'code'],
+      space);
+};
+
+/**
+ * @param {ChordTracker} pnaclChordTracker
+ * @param {Event} event
+ * @return {void}
+ */
+function HandlePNaclMessage(pnaclChordTracker, event) {
+  /** @type {PNaClEvent} */
+  var pnaclEvent = event.data;

[Jamie, 2015/01/30 23:25:09]

Nit: Since this is effectively a cast (insofar as JS has such a concept), we
conventionally annotate it as such:

  var pnaclEvent = /** @type {PNaclEvent} */ (event.data);

[Łukasz Anforowicz, 2015/01/31 00:44:49]

Done.

+
+  AppendToTextLog(JsonifyPnaclKeyboardInputEvent(pnaclEvent, undefined));
+  var title = JsonifyPnaclKeyboardInputEvent(pnaclEvent, 2);
+  if (pnaclEvent.type == "KEYDOWN") {
+    pnaclChordTracker.addKeyDownEvent(pnaclEvent.keyCode, title);
+  }
+  if (pnaclEvent.type == "KEYUP") {
+    pnaclChordTracker.addKeyUpEvent(pnaclEvent.keyCode, title);
+  }
+  if (pnaclEvent.type == "CHAR") {
+    pnaclChordTracker.addCharEvent(pnaclEvent.characterText, title);
+  }
+}
+
+/**
+ * @param {string} str
+ * @return {void}
+ */
+function AppendToTextLog(str) {
+  var textLog = document.getElementById('text-log');
+  var div = document.createElement('div');
+  div.innerText = str;
+  textLog.appendChild(div);
+}
+
 function onLoad() {
-  var parentDiv = document.getElementById('key-log');
-  var chordTracker = new ChordTracker(parentDiv);
+  // Start listening to Javascript keyup/keydown events.
+  var jsLog = document.getElementById('javascript-log');
+  var jsChordTracker = new ChordTracker(jsLog);

[Jamie, 2015/01/30 23:25:09]

I think two chord trackers is probably overkill. Better just to have one,
logging events to a single div, and perhaps reset it on focus/blur for the PNaCl
element. I don't think we need to track JS events when the plugin has focus (it
might be useful for applications where the plugin doesn't stop propagation of
events, but the Chromoting plugin does do that, and I'd like this to be as close
as possible to that implementation).

[Łukasz Anforowicz, 2015/01/31 00:44:49]

I switched to using RequestFilteringInputEvents, but javascript events keep
firing.  Therefore I am leaving 2 separate divs and chord trackers for now.

[Jamie, 2015/01/31 01:01:56]

I just tested with the Chromoting app and see the same behaviour, so I think
you're right--let's keep this for now.

   document.body.addEventListener(
-      'keydown', chordTracker.addKeyEvent.bind(chordTracker), false);
+      'keydown',
+      function (event) {
+        HandleJavascriptKeyDownEvent(jsChordTracker, event);
+      },
+      false);
   document.body.addEventListener(
-      'keyup', chordTracker.addKeyEvent.bind(chordTracker), false);
+      'keyup',
+      function (event) {
+        HandleJavascriptKeyUpEvent(jsChordTracker, event);
+      },
+      false);
+
+  // Start listening to PNaCl keyboard input events.
+  var pnaclLog = document.getElementById('pnacl-log');
+  var pnaclChordTracker = new ChordTracker(pnaclLog);
+  document.getElementById('pnacl-listener').addEventListener(
+       'message',
+       function (message) {
+         HandlePNaclMessage(pnaclChordTracker, message);
+       },
+       true);
+
+  // Start listening to generic, source-agnostic events.
   window.addEventListener(
-      'blur', chordTracker.releaseAllKeys.bind(chordTracker), false);
+      'blur',
+      function () {
+        jsChordTracker.releaseAllKeys();
+        pnaclChordTracker.releaseAllKeys();
+      },
+      false);
   document.getElementById('clear-log').addEventListener(
-      'click', function() { parentDiv.innerText = ''; }, false);
+      'click',
+      function() {
+        jsLog.innerText = '';
+        pnaclLog.innerText = '';
+        document.getElementById('text-log').innerText = '';
+      },
+      false);
 }
 
 window.addEventListener('load', onLoad, false);