Revert 146038 as it might have broken chromeos browser_tests - Make eventArgumentMassagers asynchronous so custom bindings don't need to override chrome.Event.prototype.dispatch.

BUG=121479

Review URL: https://chromiumcodereview.appspot.com/10704073

TBR=koz@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10736024

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@146078 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 143 insertions, 198 deletions

diff --git a/chrome/common/extensions/api/input_ime.json b/chrome/common/extensions/api/input_ime.json
index 2c7a525..9ddcea8 100644
--- a/chrome/common/extensions/api/input_ime.json
+++ b/chrome/common/extensions/api/input_ime.json
@@ -469,12 +469,6 @@
         "name": "onKeyEvent",
         "type": "function",
         "description": "This event is sent if this extension owns the active IME.",
-        "options": {
-          "supportsFilters": false,
-          "supportsListeners": true,
-          "supportsRules": false,
-          "maxListeners": 1
-        },
         "parameters": [
           {
             "type": "string",
diff --git a/chrome/renderer/extensions/chrome_v8_context_set.cc b/chrome/renderer/extensions/chrome_v8_context_set.cc
index 808e464..dfe3d85 100644
--- a/chrome/renderer/extensions/chrome_v8_context_set.cc
+++ b/chrome/renderer/extensions/chrome_v8_context_set.cc
@@ -145,5 +145,15 @@ void ChromeV8ContextSet::DispatchChromeHiddenMethod(
     v8::Handle<v8::Value> retval;
     (*it)->CallChromeHiddenMethod(
         method_name, v8_arguments.size(), &v8_arguments[0], &retval);
+    // In debug, the js will validate the event parameters and return a
+    // string if a validation error has occured.
+    // TODO(rafaelw): Consider only doing this check if function_name ==
+    // "Event.dispatchJSON".
+#ifndef NDEBUG
+    if (!retval.IsEmpty() && !retval->IsUndefined()) {
+      std::string error = *v8::String::AsciiValue(retval);
+      DCHECK(false) << error;
+    }
+#endif
   }
 }
diff --git a/chrome/renderer/extensions/event_unittest.cc b/chrome/renderer/extensions/event_unittest.cc
index 9cff07a..4b0619f 100644
--- a/chrome/renderer/extensions/event_unittest.cc
+++ b/chrome/renderer/extensions/event_unittest.cc
@@ -65,8 +65,6 @@ class EventUnittest : public ModuleSystemTest {
         "exports.sendRequest = function() {};");
     OverrideNativeHandler("apiDefinitions",
         "exports.GetExtensionAPIDefinition = function() {};");
-    OverrideNativeHandler("logging",
-        "exports.DCHECK = function() {};");
   }
 };
 
@@ -150,7 +148,7 @@ TEST_F(EventUnittest, NamedEventDispatch) {
       "var e = new event.Event('myevent');"
       "var called = false;"
       "e.addListener(function() { called = true; });"
-      "chromeHidden.Event.dispatchJSON('myevent', []);"
+      "chromeHidden.Event.dispatch('myevent', []);"
       "assert.AssertTrue(called);");
   module_system_->Require("test");
 }
@@ -248,28 +246,4 @@ TEST_F(EventUnittest, AddingFilterWithUrlFieldNotAListThrowsException) {
   module_system_->Require("test");
 }
 
-TEST_F(EventUnittest, MaxListeners) {
-  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
-  RegisterModule("test",
-      "var event = require('event');"
-      "var assert = requireNative('assert');"
-      "var eventOpts = {supportsListeners: true, maxListeners: 1};"
-      "var e = new event.Event('myevent', undefined, eventOpts);"
-      "var cb = function() {};"
-      "var caught = false;"
-      "try {"
-      "  e.addListener(cb);"
-      "} catch (e) {"
-      "  caught = true;"
-      "}"
-      "assert.AssertTrue(!caught);"
-      "try {"
-      "  e.addListener(cb);"
-      "} catch (e) {"
-      "  caught = true;"
-      "}"
-      "assert.AssertTrue(caught);");
-  module_system_->Require("test");
-}
-
 }  // namespace
diff --git a/chrome/renderer/extensions/extension_dispatcher.cc b/chrome/renderer/extensions/extension_dispatcher.cc
index 63c893f..9dc2c89 100644
--- a/chrome/renderer/extensions/extension_dispatcher.cc
+++ b/chrome/renderer/extensions/extension_dispatcher.cc
@@ -211,35 +211,6 @@ class ChannelNativeHandler : public NativeHandler {
   chrome::VersionInfo::Channel channel_;
 };
 
-class LoggingNativeHandler : public NativeHandler {
- public:
-  LoggingNativeHandler() {
-    RouteFunction("DCHECK",
-        base::Bind(&LoggingNativeHandler::Dcheck,
-                   base::Unretained(this)));
-  }
-
-  v8::Handle<v8::Value> Dcheck(const v8::Arguments& args) {
-    CHECK_LE(args.Length(), 2);
-    bool check_value = args[0]->BooleanValue();
-    std::string error_message;
-    if (args.Length() == 2)
-      error_message += "Error: " + std::string(*v8::String::AsciiValue(args[1]))
-          + "\n";
-
-    v8::Handle<v8::Array> stack_trace(
-        v8::StackTrace::CurrentStackTrace(10)->AsArray());
-    error_message += "Stack trace: {\n";
-    for (size_t i = 0; i < stack_trace->Length(); i++) {
-      error_message += "  "
-          + std::string(*v8::String::AsciiValue(stack_trace->Get(i))) + "\n";
-    }
-    error_message += "}";
-    DCHECK(check_value) << error_message;
-    return v8::Undefined();
-  }
-};
-
 void InstallAppBindings(ModuleSystem* module_system,
                         v8::Handle<v8::Object> chrome,
                         v8::Handle<v8::Object> chrome_hidden) {
@@ -258,7 +229,7 @@ void InstallWebstoreBindings(ModuleSystem* module_system,
                               "chromeHiddenWebstore");
 }
 
-}  // namespace
+}
 
 ExtensionDispatcher::ExtensionDispatcher()
     : is_webkit_initialized_(false),
@@ -719,8 +690,6 @@ void ExtensionDispatcher::DidCreateScriptContext(
   module_system->RegisterNativeHandler("channel",
       scoped_ptr<NativeHandler>(new ChannelNativeHandler(
           static_cast<chrome::VersionInfo::Channel>(chrome_channel_))));
-  module_system->RegisterNativeHandler("logging",
-      scoped_ptr<NativeHandler>(new LoggingNativeHandler()));
   // Create the 'chrome' variable if it doesn't already exist.
   {
     v8::HandleScope handle_scope;
diff --git a/chrome/renderer/module_system.cc b/chrome/renderer/module_system.cc
index 27c234c..3205cd49 100644
--- a/chrome/renderer/module_system.cc
+++ b/chrome/renderer/module_system.cc
@@ -60,15 +60,12 @@ bool ModuleSystem::IsPresentInCurrentContext() {
 }
 
 // static
-void ModuleSystem::DumpException(const v8::TryCatch& try_catch) {
-  v8::Handle<v8::Message> message(try_catch.Message());
-
+void ModuleSystem::DumpException(v8::Handle<v8::Message> message) {
   LOG(ERROR) << "["
              << *v8::String::Utf8Value(
                  message->GetScriptResourceName()->ToString())
              << "(" << message->GetLineNumber() << ")] "
-             << *v8::String::Utf8Value(message->Get())
-             << "{" << *v8::String::Utf8Value(try_catch.StackTrace()) << "}";
+             << *v8::String::Utf8Value(message->Get());
 }
 
 void ModuleSystem::Require(const std::string& module_name) {
@@ -99,10 +96,8 @@ v8::Handle<v8::Value> ModuleSystem::RequireForJsInner(
       v8::Handle<v8::String>::Cast(source)));
   v8::Handle<v8::Function> func =
       v8::Handle<v8::Function>::Cast(RunString(wrapped_source, module_name));
-  if (func.IsEmpty()) {
-    return ThrowException(std::string(*v8::String::AsciiValue(module_name)) +
-        ": Bad source");
-  }
+  if (func.IsEmpty())
+    return handle_scope.Close(v8::Handle<v8::Value>());
 
   exports = v8::Object::New();
   v8::Handle<v8::Object> natives(NewInstance());
@@ -113,12 +108,7 @@ v8::Handle<v8::Value> ModuleSystem::RequireForJsInner(
   };
   {
     WebKit::WebScopedMicrotaskSuppression suppression;
-    v8::TryCatch try_catch;
     func->Call(global, 3, args);
-    if (try_catch.HasCaught()) {
-      DumpException(try_catch);
-      return v8::Undefined();
-    }
   }
   modules->Set(module_name, exports);
   return handle_scope.Close(exports);
@@ -194,15 +184,16 @@ v8::Handle<v8::Value> ModuleSystem::RunString(v8::Handle<v8::String> code,
   WebKit::WebScopedMicrotaskSuppression suppression;
   v8::Handle<v8::Value> result;
   v8::TryCatch try_catch;
+  try_catch.SetCaptureMessage(true);
   v8::Handle<v8::Script> script(v8::Script::New(code, name));
   if (try_catch.HasCaught()) {
-    DumpException(try_catch);
+    DumpException(try_catch.Message());
     return handle_scope.Close(result);
   }
 
   result = script->Run();
   if (try_catch.HasCaught())
-    DumpException(try_catch);
+    DumpException(try_catch.Message());
 
   return handle_scope.Close(result);
 }
diff --git a/chrome/renderer/module_system.h b/chrome/renderer/module_system.h
index 1a09488..897121d 100644
--- a/chrome/renderer/module_system.h
+++ b/chrome/renderer/module_system.h
@@ -58,8 +58,8 @@ class ModuleSystem : public NativeHandler {
   // Returns true if the current context has a ModuleSystem installed in it.
   static bool IsPresentInCurrentContext();
 
-  // Dumps the debug info from |try_catch| to LOG(ERROR).
-  static void DumpException(const v8::TryCatch& try_catch);
+  // Dumps the given exception message to LOG(ERROR).
+  static void DumpException(v8::Handle<v8::Message> message);
 
   // Require the specified module. This is the equivalent of calling
   // require('module_name') from the loaded JS files.
diff --git a/chrome/renderer/resources/extensions/event.js b/chrome/renderer/resources/extensions/event.js
index 4b0da04..4b784ad 100644
--- a/chrome/renderer/resources/extensions/event.js
+++ b/chrome/renderer/resources/extensions/event.js
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-  var DCHECK = requireNative('logging').DCHECK;
   var eventBindingsNatives = requireNative('event_bindings');
   var AttachEvent = eventBindingsNatives.AttachEvent;
   var DetachEvent = eventBindingsNatives.DetachEvent;
@@ -209,13 +208,10 @@
 
   chromeHidden.Event = {};
 
-  // callback is a function(args, dispatch). args are the args we receive from
-  // dispatchJSON(), and dispatch is a function(args) that dispatches args to
-  // its listeners.
-  chromeHidden.Event.registerArgumentMassager = function(name, callback) {
+  chromeHidden.Event.registerArgumentMassager = function(name, fn) {
     if (eventArgumentMassagers[name])
       throw new Error("Massager already registered for event: " + name);
-    eventArgumentMassagers[name] = callback;
+    eventArgumentMassagers[name] = fn;
   };
 
   // Dispatches a named event with the given JSON array, which is deserialized
@@ -224,29 +220,43 @@
   chromeHidden.Event.dispatchJSON = function(name, args, filteringInfo) {
     var listenerIDs = null;
 
-    if (filteringInfo)
+    if (filteringInfo) {
       listenerIDs = MatchAgainstEventFilter(name, filteringInfo);
+    }
+    if (attachedNamedEvents[name]) {
+      if (args) {
+        // TODO(asargent): This is an antiquity. Until all callers of
+        // dispatchJSON use actual values, this must remain here to catch the
+        // cases where a caller has hard-coded a JSON string to pass in.
+        if (typeof(args) == "string") {
+          args = chromeHidden.JSON.parse(args);
+        }
+        if (eventArgumentMassagers[name])
+          eventArgumentMassagers[name](args);
+      }
 
-    var event = attachedNamedEvents[name];
-    if (!event)
-      return;
-
-    // TODO(asargent): This is an antiquity. Until all callers of
-    // dispatchJSON use actual values, this must remain here to catch the
-    // cases where a caller has hard-coded a JSON string to pass in.
-    if (typeof(args) == "string")
-      args = chromeHidden.JSON.parse(args);
-
-    var dispatchArgs = function(args) {
-      result = event.dispatch_(args, listenerIDs);
-      if (result)
-        DCHECK(!result.validationErrors, result.validationErrors);
-    };
+      var event = attachedNamedEvents[name];
+      var result;
+      // TODO(koz): We have to do this differently for unfiltered events (which
+      // have listenerIDs = null) because some bindings write over
+      // event.dispatch (eg: experimental.app.custom_bindings.js) and so expect
+      // events to go through it. These places need to be fixed so that they
+      // expect a listenerIDs parameter.
+      if (listenerIDs)
+        result = event.dispatch_(args, listenerIDs);
+      else
+        result = event.dispatch.apply(event, args);
+      if (result && result.validationErrors)
+        return result.validationErrors;
+    }
+  };
 
-    if (eventArgumentMassagers[name])
-      eventArgumentMassagers[name](args, dispatchArgs);
-    else
-      dispatchArgs(args);
+  // Dispatches a named event with the given arguments, supplied as an array.
+  chromeHidden.Event.dispatch = function(name, args) {
+    if (attachedNamedEvents[name]) {
+      attachedNamedEvents[name].dispatch.apply(
+          attachedNamedEvents[name], args);
+    }
   };
 
   // Test if a named event has any listeners.
@@ -259,9 +269,6 @@
   chrome.Event.prototype.addListener = function(cb, filters) {
     if (!this.eventOptions_.supportsListeners)
       throw new Error("This event does not support listeners.");
-    if (this.eventOptions_.maxListeners &&
-        this.getListenerCount() >= this.eventOptions_.maxListeners)
-      throw new Error("Too many listeners for " + this.eventName_);
     if (filters) {
       if (!this.eventOptions_.supportsFilters)
         throw new Error("This event does not support filters.");
@@ -326,14 +333,9 @@
 
   // Test if any callbacks are registered for this event.
   chrome.Event.prototype.hasListeners = function() {
-    return this.getListenerCount() > 0;
-  };
-
-  // Return the number of listeners on this event.
-  chrome.Event.prototype.getListenerCount = function() {
     if (!this.eventOptions_.supportsListeners)
       throw new Error("This event does not support listeners.");
-    return this.listeners_.length;
+    return this.listeners_.length > 0;
   };
 
   // Returns the index of the given callback if registered, or -1 if not
@@ -362,7 +364,7 @@
     var results = [];
     for (var i = 0; i < listeners.length; i++) {
       try {
-        var result = this.dispatchToListener(listeners[i].callback, args);
+        var result = listeners[i].callback.apply(null, args);
         if (result !== undefined)
           results.push(result);
       } catch (e) {
@@ -374,11 +376,6 @@
       return {results: results};
   }
 
-  // Can be overridden to support custom dispatching.
-  chrome.Event.prototype.dispatchToListener = function(callback, args) {
-    return callback.apply(null, args);
-  }
-
   // Dispatches this event object to all listeners, passing all supplied
   // arguments to this function each listener.
   chrome.Event.prototype.dispatch = function(varargs) {
diff --git a/chrome/renderer/resources/extensions/experimental.app_custom_bindings.js b/chrome/renderer/resources/extensions/experimental.app_custom_bindings.js
index a586df9..96f949d 100644
--- a/chrome/renderer/resources/extensions/experimental.app_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/experimental.app_custom_bindings.js
@@ -11,52 +11,52 @@ var appNatives = requireNative('experimental_app');
 var DeserializeString = appNatives.DeserializeString;
 var CreateBlob = appNatives.CreateBlob;
 
-chromeHidden.Event.registerArgumentMassager('experimental.app.onLaunched',
-    function(args, dispatch) {
-  var launchData = args[0];
-  var intentData = args[1];
-
-  if (launchData && intentData) {
-    switch(intentData.format) {
-      case('fileEntry'):
-        var fs = GetIsolatedFileSystem(intentData.fileSystemId);
-        try {
-          fs.root.getFile(intentData.baseName, {}, function(fileEntry) {
-            launchData.intent.data = fileEntry;
-            launchData.intent.postResult = function() {};
-            launchData.intent.postFailure = function() {};
-            dispatch([launchData]);
-          }, function(fileError) {
-            console.error('Error getting fileEntry, code: ' + fileError.code);
-            dispatch([]);
-          });
-        } catch (e) {
-          console.error('Error in event handler for onLaunched: ' + e.stack);
-          dispatch([]);
-        }
-        break;
-      case('serialized'):
-        var deserializedData = DeserializeString(intentData.data);
-        launchData.intent.data = deserializedData;
-        launchData.intent.postResult = function() {};
-        launchData.intent.postFailure = function() {};
-        dispatch([launchData]);
-        break;
-      case('blob'):
-        var blobData = CreateBlob(intentData.blobFilePath,
-                                  intentData.blobLength);
-        launchData.intent.data = blobData;
-        launchData.intent.postResult = function() {};
-        launchData.intent.postFailure = function() {};
-        dispatch([launchData]);
-        break;
-      default:
-        console.error('Unexpected launch data format');
-        dispatch([]);
+chromeHidden.registerCustomHook('experimental.app', function(bindingsAPI) {
+  chrome.experimental.app.onLaunched.dispatch =
+      function(launchData, intentData) {
+    if (launchData && intentData) {
+      switch(intentData.format) {
+        case('fileEntry'):
+          var event = this;
+          var fs = GetIsolatedFileSystem(intentData.fileSystemId);
+          try {
+            fs.root.getFile(intentData.baseName, {}, function(fileEntry) {
+              launchData.intent.data = fileEntry;
+              launchData.intent.postResult = function() {};
+              launchData.intent.postFailure = function() {};
+              chrome.Event.prototype.dispatch.call(event, launchData);
+            }, function(fileError) {
+              console.error('Error getting fileEntry, code: ' + fileError.code);
+              chrome.Event.prototype.dispatch.call(event);
+            });
+          } catch (e) {
+            console.error('Error in event handler for onLaunched: ' + e.stack);
+            chrome.Event.prototype.dispatch.call(event);
+          }
+          break;
+        case('serialized'):
+          var deserializedData = DeserializeString(intentData.data);
+          launchData.intent.data = deserializedData;
+          launchData.intent.postResult = function() {};
+          launchData.intent.postFailure = function() {};
+          chrome.Event.prototype.dispatch.call(this, launchData);
+          break;
+        case('blob'):
+          var blobData = CreateBlob(intentData.blobFilePath,
+                                    intentData.blobLength);
+          launchData.intent.data = blobData;
+          launchData.intent.postResult = function() {};
+          launchData.intent.postFailure = function() {};
+          chrome.Event.prototype.dispatch.call(this, launchData);
+          break;
+        default:
+          console.error('Unexpected launch data format');
+          chrome.Event.prototype.dispatch.call(this);
+      }
+    } else if (launchData) {
+      chrome.Event.prototype.dispatch.call(this, launchData);
+    } else {
+      chrome.Event.prototype.dispatch.call(this);
     }
-  } else if (launchData) {
-    dispatch([launchData]);
-  } else {
-    dispatch([]);
-  }
+  };
 });
diff --git a/chrome/renderer/resources/extensions/input.ime_custom_bindings.js b/chrome/renderer/resources/extensions/input.ime_custom_bindings.js
index 204a9e0..7a64e9d 100644
--- a/chrome/renderer/resources/extensions/input.ime_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/input.ime_custom_bindings.js
@@ -8,16 +8,29 @@
 var chromeHidden = requireNative('chrome_hidden').GetChromeHidden();
 
 chromeHidden.registerCustomHook('input.ime', function() {
-  chrome.input.ime.onKeyEvent.dispatchToListener = function(callback, args) {
-    var engineID = args[0];
-    var keyData = args[1];
-
-    var result = false;
-    try {
-      result = chrome.Event.prototype.dispatchToListener(callback, args);
-    } catch (e) {
-      console.error('Error in event handler for onKeyEvent: ' + e.stack);
+  chrome.input.ime.onKeyEvent.dispatch = function(engineID, keyData) {
+    var args = Array.prototype.slice.call(arguments);
+    if (this.validate_) {
+      var validationErrors = this.validate_(args);
+      if (validationErrors) {
+        chrome.input.ime.eventHandled(requestId, false);
+        return validationErrors;
+      }
+    }
+    if (this.listeners_.length > 1) {
+      console.error('Too many listeners for onKeyEvent: ' + e.stack);
+      chrome.input.ime.eventHandled(requestId, false);
+      return;
+    }
+    for (var i = 0; i < this.listeners_.length; i++) {
+      try {
+        var requestId = keyData.requestId;
+        var result = this.listeners_[i].callback.apply(null, args);
+        chrome.input.ime.eventHandled(requestId, result);
+      } catch (e) {
+        console.error('Error in event handler for onKeyEvent: ' + e.stack);
+        chrome.input.ime.eventHandled(requestId, false);
+      }
     }
-    chrome.input.ime.eventHandled(keyData.requestId, result);
   };
 });
diff --git a/chrome/renderer/resources/extensions/omnibox_custom_bindings.js b/chrome/renderer/resources/extensions/omnibox_custom_bindings.js
index 745010a..de68a76 100644
--- a/chrome/renderer/resources/extensions/omnibox_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/omnibox_custom_bindings.js
@@ -97,14 +97,12 @@ chromeHidden.registerCustomHook('omnibox', function(bindingsAPI) {
     }
     return [requestId, suggestions];
   });
-});
 
-chromeHidden.Event.registerArgumentMassager('omnibox.onInputChanged',
-    function(args, dispatch) {
-  var text = args[0];
-  var requestId = args[1];
-  var suggestCallback = function(suggestions) {
-    chrome.omnibox.sendSuggestions(requestId, suggestions);
+  chrome.omnibox.onInputChanged.dispatch =
+      function(text, requestId) {
+    var suggestCallback = function(suggestions) {
+      chrome.omnibox.sendSuggestions(requestId, suggestions);
+    };
+    chrome.Event.prototype.dispatch.apply(this, [text, suggestCallback]);
   };
-  dispatch([text, suggestCallback]);
 });
diff --git a/chrome/renderer/resources/extensions/tts_engine_custom_bindings.js b/chrome/renderer/resources/extensions/tts_engine_custom_bindings.js
index f16f143..7c0067b 100644
--- a/chrome/renderer/resources/extensions/tts_engine_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/tts_engine_custom_bindings.js
@@ -6,13 +6,12 @@
 
 var chromeHidden = requireNative('chrome_hidden').GetChromeHidden();
 
-chromeHidden.Event.registerArgumentMassager('ttsEngine.onSpeak',
-    function(args, dispatch) {
-  var text = args[0];
-  var options = args[1];
-  var requestId = args[2];
-  var sendTtsEvent = function(event) {
-    chrome.ttsEngine.sendTtsEvent(requestId, event);
+chromeHidden.registerCustomHook('ttsEngine', function() {
+  chrome.ttsEngine.onSpeak.dispatch = function(text, options, requestId) {
+    var sendTtsEvent = function(event) {
+      chrome.ttsEngine.sendTtsEvent(requestId, event);
+    };
+    chrome.Event.prototype.dispatch.apply(
+        this, [text, options, sendTtsEvent]);
   };
-  dispatch([text, options, sendTtsEvent]);
 });
diff --git a/chrome/test/base/module_system_test.cc b/chrome/test/base/module_system_test.cc
index f1e3b6f..d6b5de2 100644
--- a/chrome/test/base/module_system_test.cc
+++ b/chrome/test/base/module_system_test.cc
@@ -112,7 +112,7 @@ void ModuleSystemTest::OverrideNativeHandler(const std::string& name,
 
 void ModuleSystemTest::TearDown() {
   if (try_catch_.HasCaught())
-    ModuleSystem::DumpException(try_catch_);
+    ModuleSystem::DumpException(try_catch_.Message());
   EXPECT_FALSE(try_catch_.HasCaught());
   // All tests must assert at least once unless otherwise specified.
   EXPECT_EQ(should_assertions_be_made_,