Add some closureised JS bindings for DevTools for use by headless embedder

headless/lib/browser/devtools_api/devtools_connection.js

+// Copyright 2017 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.
+
+/**
+ * @fileoverview The DevTools

[Sami, 2017/05/24 09:16:21]

Comment could use expanding :)

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

Done.

+ */
+
+'use strict';
+
+goog.provide('goog.DevTools.Connection');
+
+/**
+ * @class Bindings for the {{domain.domain}} DevTools Domain.

[Sami, 2017/05/24 09:16:21]

This comment seems wrong.

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

Done.

+ * @param {!Object} transport The API providing transport for devtools commands.
+ * @constructor
+ */
+goog.DevTools.Connection = function(transport) {
+  /**
+   * The DevTools connection.
+   *
+   * @private {!Object}
+   */
+  this.transport_ = transport;
+
+  /**
+   * @private {number}
+   */
+  this.commandId_ = 1;
+
+  /**
+   * An object containing pending DevTools protocol commands keyed by id.
+   *
+   * @type {!Object<!function(!Object)>}
+   * @private
+   */
+  this.pendingCommands_ = {};
+
+  /**
+   * An object containing DevTools protocol events we are listening for keyed by
+   * name.
+   *
+   * @type {!Object<!Array<!function(!Object)>>}
+   * @private
+   */
+  this.eventListeners_ = {};
+
+  this.transport_.onmessage = this.OnMessage_.bind(this);

[Sami, 2017/05/24 09:16:21]

nit: onMessage

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

We defined this is as onmessage in the TabSocket.

+};
+
+
+/**
+ * Listens for DevTools protocol events of the specified name and issues the
+ * callback upon reception.
+ *
+ * @param {string} eventName Name of the DevTools protocol event to listen for.
+ * @param {!function(!Object)} listener The callback issued
+ *   when we receive a DevTools protocol event corresponding to the given name.
+ */
+goog.DevTools.Connection.prototype.AddEventListener = function(eventName,

[Sami, 2017/05/24 09:16:21]

Should we support removing listeners too?

nit: addEventListener

[aerotwist, 2017/05/24 11:06:42]

I'd love to see removeEventListener added here.

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

Done.

+                                                               listener) {
+  if (!this.eventListeners_.hasOwnProperty(eventName)) {
+    this.eventListeners_[eventName] = [];
+  }
+  this.eventListeners_[eventName].push(listener);
+};
+
+
+/**
+ * Issues a DevTools protocol command and returns a promise for the results.
+ *
+ * @param {string} method The name of the DevTools protocol command method.
+ * @param {!Object=} opt_params An object containing the command parameters if
+ *    any.
+ * @return {Promise<!Object>} A promise for the results object.
+ */
+goog.DevTools.Connection.prototype.SendDevToolsMessage = function(method,

[Sami, 2017/05/24 09:16:21]

nit: sendDevToolsMessage

[aerotwist, 2017/05/24 11:06:42]

It'd be worth checking whether the DevTools API (or DevTools itself) marshals
requests into a chain, or whether it lets you run multiple requests in parallel
like this. If not, could we potentially see race conditions?

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

In theory commands sent in the order A,B,C are processed in that order but some
are processed by the browser process and some by the renderer.  So race
conditions are a thing and basically you'd need to use promise chaining to be
sure stuff happens in the expected order.  This is the case for existing
bindings such as https://www.npmjs.com/package/chrome-remote-interface.

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

Done.

+                                                                  opt_params) {
+  if (opt_params === undefined) {
+    opt_params = {};
+  }
+  let id = this.commandId_;
+  this.commandId_ += 2;

[Sami, 2017/05/24 09:16:21]

This could use a comment saying why +2

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

Done.

+  this.transport_.send(
+      JSON.stringify({'method': method, 'id': id, 'params': opt_params}));
+  return new Promise((resolve, reject) => {
+    this.pendingCommands_[id] = resolve;
+  });
+};
+
+
+/**
+ * @param {Object} event An object containing a DevTools protocol
+ *    message.
+ * @private
+ */
+goog.DevTools.Connection.prototype.OnMessage_ = function(event) {
+  let message = JSON.parse(event.detail.message);
+  if (message.hasOwnProperty('id')) {
+    if (!this.pendingCommands_.hasOwnProperty(message.id)) {
+      console.log('Unrecognized id:' + json_message);

[Sami, 2017/05/24 09:16:21]

console.error? Or just throw an error?

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

Done.

+      return;
+    }
+    this.pendingCommands_[message.id](message.result);
+    delete this.pendingCommands_[message.id];
+  } else {
+    if (!message.hasOwnProperty('method') ||
+        !message.hasOwnProperty('params')) {
+      console.log('Bad message:' + json_message);

[Sami, 2017/05/24 09:16:21]

Ditto.

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

Done.

+      return;
+    }
+    var method = message['method'];
+    if (this.eventListeners_.hasOwnProperty(method)) {
+      var listeners = this.eventListeners_[method];

[Sami, 2017/05/24 09:16:21]

let for consistency?

[aerotwist, 2017/05/24 11:06:42]

May as well go full const here; it looks like the ref is immutable.

[alex clarke (OOO till 29th), 2017/05/24 11:38:14]

Done.

+      var params = message['params'];
+      for (var i = 0; i < listeners.length; i++) {
+        listeners[i](params);
+      }
+      return;
+    }
+  }
+};