Add filtered event support to event.js.

chrome/renderer/extensions/event_unittest.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/test/base/module_system_test.h"
 
 #include "grit/renderer_resources.h"
 
 namespace {
 
 class EventUnittest : public ModuleSystemTest {
   virtual void SetUp() OVERRIDE {
     ModuleSystemTest::SetUp();
     module_system_->RunString("chrome = {};", "setup-chrome");
 
     RegisterModule("event", IDR_EVENT_BINDINGS_JS);
 
     // Mock out the native handler for event_bindings. These mocks will fail if
     // any invariants maintained by the real event_bindings are broken.
     OverrideNativeHandler("event_bindings",
         "var assert = requireNative('assert');"
         "var attachedListeners = exports.attachedListeners = {};"
+        "var attachedFilteredListeners = "
+        "    exports.attachedFilteredListeners = {};"
+        "var nextId = 0;"
+        "var idToName = {};"
+
         "exports.AttachEvent = function(eventName) {"
         "  assert.AssertFalse(!!attachedListeners[eventName]);"
         "  attachedListeners[eventName] = 1;"
         "};"
+
         "exports.DetachEvent = function(eventName) {"
         "  assert.AssertTrue(!!attachedListeners[eventName]);"
         "  delete attachedListeners[eventName];"
+        "};"
+
+        "exports.IsEventAttached = function(eventName) {"
+        "  return !!attachedListeners[eventName];"
+        "};"
+
+        "exports.AttachFilteredEvent = function(name, filter) {"
+        "  var id = nextId++;"
+        "  idToName[id] = name;"
+        "  attachedFilteredListeners[name] ="
+        "    attachedFilteredListeners[name] || [];"
+        "  attachedFilteredListeners[name][id] = filter;"
+        "  return id;"
+        "};"
+
+        "exports.DetachFilteredEvent = function(id, manual) {"
+        "  var i = attachedFilteredListeners[idToName[id]].indexOf(id);"
+        "  attachedFilteredListeners[idToName[id]].splice(i, 1);"
+        "};"
+
+        "exports.HasFilteredListener = function(name) {"
+        "  return attachedFilteredListeners[name].length;"
         "};");
     OverrideNativeHandler("chrome_hidden",
         "var chromeHidden = {};"
         "exports.GetChromeHidden = function() { return chromeHidden; };");
   }
 };
 
 TEST_F(EventUnittest, TestNothing) {
   ExpectNoAssertionsMade();
 }
 
 TEST_F(EventUnittest, AddRemoveTwoListeners) {
   ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
   RegisterModule("test",
       "var assert = requireNative('assert');"
       "var event = require('event');"
       "var eventBindings = requireNative('event_bindings');"
       "var myEvent = new event.Event('named-event');"
       "var cb1 = function() {};"
       "var cb2 = function() {};"
       "myEvent.addListener(cb1);"
       "myEvent.addListener(cb2);"
       "myEvent.removeListener(cb1);"
       "assert.AssertTrue(!!eventBindings.attachedListeners['named-event']);"
       "myEvent.removeListener(cb2);"
       "assert.AssertFalse(!!eventBindings.attachedListeners['named-event']);");
   module_system_->Require("test");
 }
 
+TEST_F(EventUnittest, OnUnloadDetachesAllListeners) {
+  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
+  RegisterModule("test",
+      "var assert = requireNative('assert');"
+      "var event = require('event');"
+      "var eventBindings = requireNative('event_bindings');"
+      "var chromeHidden = requireNative('chrome_hidden').GetChromeHidden();"
+      "var myEvent = new event.Event('named-event');"
+      "var cb1 = function() {};"
+      "var cb2 = function() {};"
+      "myEvent.addListener(cb1);"
+      "myEvent.addListener(cb2);"
+      "chromeHidden.dispatchOnUnload();"
+      "assert.AssertFalse(!!eventBindings.attachedListeners['named-event']);");
+  module_system_->Require("test");
+}
+
+TEST_F(EventUnittest, OnUnloadDetachesAllListenersEvenDupes) {
+  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
+  RegisterModule("test",
+      "var assert = requireNative('assert');"
+      "var event = require('event');"
+      "var eventBindings = requireNative('event_bindings');"
+      "var chromeHidden = requireNative('chrome_hidden').GetChromeHidden();"
+      "var myEvent = new event.Event('named-event');"
+      "var cb1 = function() {};"
+      "myEvent.addListener(cb1);"
+      "myEvent.addListener(cb1);"
+      "chromeHidden.dispatchOnUnload();"
+      "assert.AssertFalse(!!eventBindings.attachedListeners['named-event']);");
+  module_system_->Require("test");
+}
+
 TEST_F(EventUnittest, EventsThatSupportRulesMustHaveAName) {
   ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
   RegisterModule("test",
       "var event = require('event');"
       "var eventOpts = {supportsRules: true};"
       "var assert = requireNative('assert');"
       "var caught = false;"
       "try {"
       "  var myEvent = new event.Event(undefined, undefined, eventOpts);"
       "} catch (e) {"
@@ skipping 10 matching lines @@
       "var chromeHidden = requireNative('chrome_hidden').GetChromeHidden();"
       "var assert = requireNative('assert');"
       "var e = new event.Event('myevent');"
       "var called = false;"
       "e.addListener(function() { called = true; });"
       "chromeHidden.Event.dispatch('myevent', []);"
       "assert.AssertTrue(called);");
   module_system_->Require("test");
 }
 
+TEST_F(EventUnittest, AddListenerWithFiltersThrowsErrorByDefault) {
+  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
+  RegisterModule("test",
+      "var event = require('event');"
+      "var assert = requireNative('assert');"
+      "var e = new event.Event('myevent');"
+      "var filter = {"
+      "  url: [{hostSuffix: 'google.com'}],"

[Matt Perry, 2012/05/16 20:45:00]

url should not be an array, it should be a single object. filter itself can be
an array. i.e. filter = [
  { url: {hostSuffix: 'google.com'} },
  { url: {hostPrefix: 'mail.yahoo'} }
];

[koz (OOO until 15th September), 2012/05/18 04:50:07]

Done.

[koz (OOO until 15th September), 2012/05/21 01:27:50]

Thinking about this a bit I wonder if this is really what we want? Having filter
be a list of dictionaries of single values means that we have to handle the
general case every time, rather than the efficient case, which is the common
case.

By that I mean isn't it more common to want to specify the set of valid URLs and
the set of valid tabIds for an event, rather than a set of individual filters?

addListener(cb, [
  {url: {a}, tabId: 1},
  {url: {b}, tabId: 2}]);

This seems like a rare scenario - we are interested in this URL for this tab,
and this URL for that tab. To express what seems to me to be a much more common
scenario, that the user is interested in these URLs for this tab, you have to
replicate tabId many times:

addListener(cb, [
  {url: {a}, tabId: 1},
  {url: {b}, tabId: 1},
  {url: {c}, tabId: 1},
  /* etc */]);

By having the url field be an array I think it would match user intent more
closely, and be simpler to implement (as we don't need to remember which URLs
correspond to which other sets of constraints). The rare case above is still
expressable through multiple calls to addListener().

WDYT?

[Matt Perry, 2012/05/22 19:20:52]

That's a good point. I was just trying to make it mirror the declarative
webrequest API as much as possible:
http://code.google.com/chrome/extensions/trunk/declarativeWebRequest.html

You're right, it's probably rare to want to filter on more than one tabId. But
there are other filter criteria, like resource type (image, script, main frame,
etc).

Also, with your proposed way of organizing filters, there's no way to express
something like "(url matches a AND resourceType matches x) OR (url matches b AND
resourceType matches y)".

Dominic, what do you think?

[battre, 2012/05/22 20:19:45]

What do you think of providing 'url' and 'urls'.

addListener(cb, [
  {url: {a}, tabId: 1},
  {url: {b}, tabId: 1},
  {url: {c}, tabId: 1},
  /* etc */]);

and

addListener(cb, [
  {urls: [{a}, {b}, {c}], tabId: 1},
  ]);

would be equivalent. Extend the declarative webRequest API accordingly.

However, we cannot do this to an infinite degree.

E.g.
addListener(cb, [
  {urls: [{a, schemes: ['http']}, {b, schemes: ['http']}, {c, schemes:
['http']}], tabId: 1},
  ]);
could not be simplified any further.

[Matt Perry, 2012/05/22 20:43:59]

That sounds like a good compromise to me.

+      "};"
+      "var caught = false;"
+      "try {"
+      "  e.addListener(function() {}, filter);"
+      "} catch (e) {"
+      "  caught = true;"
+      "}"
+      "assert.AssertTrue(caught);");
+  module_system_->Require("test");
+}
+
+TEST_F(EventUnittest, FilteredEventsAttachment) {
+  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
+  RegisterModule("test",
+      "var event = require('event');"
+      "var assert = requireNative('assert');"
+      "var bindings = requireNative('event_bindings');"
+      "var eventOpts = {supportsListeners: true, supportsFilters: true};"
+      "var e = new event.Event('myevent', undefined, eventOpts);"
+      "var cb = function() {};"
+      "var filters = {url: [{hostSuffix: 'google.com'}]};"
+      "e.addListener(cb, filters);"
+      "assert.AssertTrue(bindings.HasFilteredListener('myevent'));"
+      "e.removeListener(cb);"
+      "assert.AssertFalse(bindings.HasFilteredListener('myevent'));");
+  module_system_->Require("test");
+}
+
+TEST_F(EventUnittest, DetachFilteredEvent) {
+  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
+  RegisterModule("test",
+      "var event = require('event');"
+      "var assert = requireNative('assert');"
+      "var bindings = requireNative('event_bindings');"
+      "var eventOpts = {supportsListeners: true, supportsFilters: true};"
+      "var e = new event.Event('myevent', undefined, eventOpts);"
+      "var cb1 = function() {};"
+      "var cb2 = function() {};"
+      "var filters = {url: [{hostSuffix: 'google.com'}]};"
+      "e.addListener(cb1, filters);"
+      "e.addListener(cb2, filters);"
+      "e.detach();"
+      "assert.AssertFalse(bindings.HasFilteredListener('myevent'));");
+  module_system_->Require("test");
+}
+
+TEST_F(EventUnittest, AttachAndRemoveSameFilteredEventListener) {
+  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
+  RegisterModule("test",
+      "var event = require('event');"
+      "var assert = requireNative('assert');"
+      "var bindings = requireNative('event_bindings');"
+      "var eventOpts = {supportsListeners: true, supportsFilters: true};"
+      "var e = new event.Event('myevent', undefined, eventOpts);"
+      "var cb = function() {};"
+      "var filters = {url: [{hostSuffix: 'google.com'}]};"
+      "e.addListener(cb, filters);"
+      "e.addListener(cb, filters);"
+      "assert.AssertTrue(bindings.HasFilteredListener('myevent'));"
+      "e.removeListener(cb);"
+      "assert.AssertTrue(bindings.HasFilteredListener('myevent'));"
+      "e.removeListener(cb);"
+      "assert.AssertFalse(bindings.HasFilteredListener('myevent'));");
+  module_system_->Require("test");
+}
+
+TEST_F(EventUnittest, AddingFilterWithUrlFieldNotAListThrowsException) {
+  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
+  RegisterModule("test",
+      "var event = require('event');"
+      "var assert = requireNative('assert');"
+      "var eventOpts = {supportsListeners: true, supportsFilters: true};"
+      "var e = new event.Event('myevent', undefined, eventOpts);"
+      "var cb = function() {};"
+      "var filters = {url: {hostSuffix: 'google.com'}};"
+      "var caught = false;"
+      "try {"
+      "  e.addListener(cb, filters);"
+      "} catch (e) {"
+      "  caught = true;"
+      "}"
+      "assert.AssertTrue(caught);");
+  module_system_->Require("test");
+}
+
 }  // namespace