Small cleanups in automation_node.js.

chrome/renderer/resources/extensions/automation/automation_node.js

 // Copyright 2014 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.
 
 var AutomationEvent = require('automationEvent').AutomationEvent;
 var automationInternal =
     require('binding').Binding.create('automationInternal').generate();
 var utils = require('utils');
 var IsInteractPermitted =
     requireNative('automationInternal').IsInteractPermitted;
@@ skipping 54 matching lines @@
     var parent = this.parent();
     if (parent && this.indexInParent < parent.children().length)
       return parent.children()[this.indexInParent + 1];
     return undefined;
   },
 
   doDefault: function() {
     this.performAction_('doDefault');
   },
 
-  focus: function(opt_callback) {
+  focus: function() {
     this.performAction_('focus');
   },
 
-  makeVisible: function(opt_callback) {
+  makeVisible: function() {
     this.performAction_('makeVisible');
   },
 
-  setSelection: function(startIndex, endIndex, opt_callback) {
+  setSelection: function(startIndex, endIndex) {
     this.performAction_('setSelection',
                         { startIndex: startIndex,
                           endIndex: endIndex });
   },
 
   addEventListener: function(eventType, callback, capture) {
     this.removeEventListener(eventType, callback);
     if (!this.listeners[eventType])
       this.listeners[eventType] = [];
     this.listeners[eventType].push({callback: callback, capture: !!capture});
@@ skipping 152 matching lines @@
   this.processID = processID;
   this.routingID = routingID;
   this.axNodeDataCache_ = {};
 }
 
 AutomationRootNodeImpl.prototype = {
   __proto__: AutomationNodeImpl.prototype,
 
   isRootNode: true,
 
+  load: function() {
+    throw new Error('Load is currently not supported.');

[aboxhall, 2014/06/30 21:57:54]

I don't know if we want to throw an Error here - can we just make it a no-op for
now instead?

[David Tseng, 2014/06/30 23:02:24]

I don't know if we want to fail/no-op silently; the error is mostly to let the
caller know they shouldn't be calling. Would be open to some other way of
indicating this to the caller.

[aboxhall, 2014/06/30 23:03:36]

I don't know if we want to train people not to call it if we eventually want
them to call it. What's wrong with having it just not do anything, until such
time as it does do something?

[aboxhall, 2014/06/30 23:04:25]

If we don't want people to call it, we should probably remove it (and the loaded
bool) from the API until we implement it.

[not at google - send to devlin, 2014/06/30 23:05:01]

Who implements a non-crashing version of load()?

[David Tseng, 2014/07/01 18:48:48]

Ok...made this a no-op.

The idea of having it in the API was to finalize the idl. Having it do nothing
or crashing at this point doesn't matter all that much since we don't even have
the api flipped on in dev.

Wrt hooking up desktop loading of web contents, that's not done yet. I'll make
that my next change for the api.

[aboxhall, 2014/07/01 19:55:11]

Right, I was just nervous that we might flip the flag without changing the
behaviour, since we might not get to the point where load() is ready to
implement before then.

On that note, this method takes a callback in the spec and in the IDL; can we
make it a proper no-op implementation by having it schedule an immediate call to
the callback if |loaded| is true? And in fact if |loaded| is false, perhaps we
should make this an error after all (sorry for the back and forth on that - I
don't imagine it will ever be false while the error is there, but just to be on
the safe side).
Cool - I was asking because I'm curious whether load() will need to be
implemented for that. I remember talking about using it for that use case at
some point, but I'm not sure what you're planning at this stage.

+  },
+
   get: function(id) {
     return this.axNodeDataCache_[id];
   },
 
   invalidate: function(node) {
     if (!node)
       return;
 
     var children = node.children();
 
@@ skipping 125 matching lines @@
                                                'attributes'] });
 
 var AutomationRootNode = utils.expose('AutomationRootNode',
                                       AutomationRootNodeImpl,
                                       { superclass: AutomationNode,
                                         functions: ['load'],
                                         readonly: ['loaded'] });
 
 exports.AutomationNode = AutomationNode;
 exports.AutomationRootNode = AutomationRootNode;