Clear an extension's filtered events when a context is destroyed.

In crrev.com/325156 I moved event invalidation logic from JavaScript into C++,
but I missed out filtered events. This CL also does some cleanup.

BUG=503913
R=rdevlin.cronin@chromium.org

Review URL: https://codereview.chromium.org/1227093008

Cr-Commit-Position: refs/heads/master@{#338403}

chrome/browser/extensions/events_apitest.cc

@@ -45,7 +45,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, EventsAreUnregistered) {
   const std::string& id = extension->id();
 
   // The page has closed, so no matter what all events are no longer listened
-  // to.
+  // to. Assertions for normal events:
   EXPECT_FALSE(
       event_router->ExtensionHasEventListener(id, "browserAction.onClicked"));
   EXPECT_FALSE(
@@ -54,6 +54,15 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, EventsAreUnregistered) {
       event_router->ExtensionHasEventListener(id, "runtime.onSuspend"));
   EXPECT_FALSE(
       event_router->ExtensionHasEventListener(id, "runtime.onInstalled"));
+  // Assertions for filtered events:
+  EXPECT_FALSE(event_router->ExtensionHasEventListener(
+      id, "webNavigation.onBeforeNavigate"));
+  EXPECT_FALSE(
+      event_router->ExtensionHasEventListener(id, "webNavigation.onCommitted"));
+  EXPECT_FALSE(event_router->ExtensionHasEventListener(
+      id, "webNavigation.onDOMContentLoaded"));
+  EXPECT_FALSE(
+      event_router->ExtensionHasEventListener(id, "webNavigation.onCompleted"));
 }
 
 }  // namespace extensions

chrome/common/extensions/value_counter_unittest.cc

@@ -7,36 +7,62 @@
 #include "extensions/common/value_counter.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class ValueCounterUnittest : public testing::Test {
-};
+using ValueCounterTest = testing::Test;
 
-TEST_F(ValueCounterUnittest, TestAddingSameValue) {
-  extensions::ValueCounter vc;
+namespace extensions {
+
+TEST_F(ValueCounterTest, TestAddingSameValue) {
+  ValueCounter vc;
   base::ListValue value;
-  ASSERT_EQ(1, vc.Add(value));
-  ASSERT_EQ(2, vc.Add(value));
+  ASSERT_TRUE(vc.Add(value));
+  ASSERT_FALSE(vc.Add(value));
 }
 
-TEST_F(ValueCounterUnittest, TestAddingDifferentValue) {
-  extensions::ValueCounter vc;
+TEST_F(ValueCounterTest, TestAddingDifferentValue) {
+  ValueCounter vc;
   base::ListValue value1;
   base::DictionaryValue value2;
-  ASSERT_EQ(1, vc.Add(value1));
-  ASSERT_EQ(1, vc.Add(value2));
+  ASSERT_TRUE(vc.Add(value1));
+  ASSERT_TRUE(vc.Add(value2));
 }
 
-TEST_F(ValueCounterUnittest, TestRemovingValue) {
-  extensions::ValueCounter vc;
+TEST_F(ValueCounterTest, TestRemovingSameValue) {
+  ValueCounter vc;
   base::ListValue value;
-  ASSERT_EQ(1, vc.Add(value));
-  ASSERT_EQ(2, vc.Add(value));
-  ASSERT_EQ(1, vc.Remove(value));
-  ASSERT_EQ(0, vc.Remove(value));
+  vc.Add(value);
+  vc.Add(value);
+  ASSERT_FALSE(vc.Remove(value));
+  ASSERT_TRUE(vc.Remove(value));
+  ASSERT_FALSE(vc.Remove(value));
 }
 
-TEST_F(ValueCounterUnittest, TestAddIfMissing) {
-  extensions::ValueCounter vc;
+TEST_F(ValueCounterTest, TestReAddingSameValue) {
+  ValueCounter vc;
   base::ListValue value;
-  ASSERT_EQ(1, vc.AddIfMissing(value));
-  ASSERT_EQ(1, vc.AddIfMissing(value));
+  ASSERT_FALSE(vc.Remove(value));
+  ASSERT_TRUE(vc.Add(value));
+  ASSERT_TRUE(vc.Remove(value));
+  ASSERT_TRUE(vc.Add(value));
+  ASSERT_TRUE(vc.Remove(value));
+  ASSERT_FALSE(vc.Remove(value));
+}
+
+TEST_F(ValueCounterTest, TestIsEmpty) {
+  ValueCounter vc;
+  base::ListValue value1;
+  base::DictionaryValue value2;
+  ASSERT_TRUE(vc.is_empty());
+  vc.Add(value1);
+  ASSERT_FALSE(vc.is_empty());
+  vc.Remove(value1);
+  ASSERT_TRUE(vc.is_empty());
+  vc.Add(value1);
+  vc.Add(value2);
+  ASSERT_FALSE(vc.is_empty());
+  vc.Remove(value1);
+  ASSERT_FALSE(vc.is_empty());
+  vc.Remove(value2);
+  ASSERT_TRUE(vc.is_empty());
 }
+
+}  // namespace extensions

chrome/test/data/extensions/api_test/events_are_unregistered/manifest.json

@@ -3,5 +3,6 @@
   "description": "events_are_unregistered",
   "manifest_version": 2,
   "name": "events_are_unregistered",
+  "permissions": ["webNavigation"],
   "version": "1"
 }

chrome/test/data/extensions/api_test/events_are_unregistered/page1.js

@@ -2,23 +2,68 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+'use strict';
+
 // Register for events in 4 configurations, then navigate to page2.html, which
 // will notify success and succeed the test on the C++ side. The C++ code
 // asserts that the events have been unregistered.
 
+//
+// Unfiltered events.
+//
+
 // A single listener.
 chrome.browserAction.onClicked.addListener(function() {});
 // Multiple listeners for the same event.
 chrome.runtime.onStartup.addListener(function() {});
 chrome.runtime.onStartup.addListener(function() {});
 // A single listener, which previously had multiple listeners.
-chrome.runtime.onSuspend.addListener(function() {});
-chrome.runtime.onSuspend.addListener(function() {});
-chrome.runtime.onSuspend.removeListener(function() {});
+{
+  let singleListener = function() {};
+  chrome.runtime.onSuspend.addListener(singleListener);
+  chrome.runtime.onSuspend.addListener(function() {});
+  chrome.runtime.onSuspend.removeListener(singleListener);
+}
 // No listeners, which previously had listeners (all were removed).
-chrome.runtime.onInstalled.addListener(function() {});
-chrome.runtime.onInstalled.addListener(function() {});
-chrome.runtime.onInstalled.removeListener(function() {});
-chrome.runtime.onInstalled.removeListener(function() {});
+{
+  let listener1 = function() {};
+  let listener2 = function() {};
+  chrome.runtime.onInstalled.addListener(listener1);
+  chrome.runtime.onInstalled.addListener(listener2);
+  chrome.runtime.onInstalled.removeListener(listener1);
+  chrome.runtime.onInstalled.removeListener(listener2);
+}
+
+//
+// Filtered events.
+//
+
+function filterPort(portNumber) {
+  return {url: [{ports: [portNumber]}]};
+}
+
+// A single listener.
+chrome.webNavigation.onBeforeNavigate.addListener(function() {});
+// Multiple, different listeners.
+chrome.webNavigation.onCommitted.addListener(function() {});
+chrome.webNavigation.onCommitted.addListener(function() {});
+// Different listeners with the same filter.
+chrome.webNavigation.onDOMContentLoaded.addListener(
+    function() {}, filterPort(80));
+chrome.webNavigation.onDOMContentLoaded.addListener(
+    function() {}, filterPort(80));
+// Different listeners with different filters, same event added twice.
+{
+  let singleListener = function() {};
+  chrome.webNavigation.onCompleted.addListener(
+      function() {}, filterPort(80));
+  chrome.webNavigation.onCompleted.addListener(
+      function() {}, filterPort(81));
+  chrome.webNavigation.onCompleted.addListener(
+      function() {}, filterPort(81));
+  chrome.webNavigation.onCompleted.addListener(
+      singleListener, filterPort(82));
+  chrome.webNavigation.onCompleted.removeListener(singleListener);
+}
 
 location.assign('page2.html');

extensions/common/value_counter.cc

@@ -10,47 +10,44 @@
 
 namespace extensions {
 
-ValueCounter::ValueCounter() {}
+struct ValueCounter::Entry {
+  explicit Entry(scoped_ptr<base::Value> value)
+      : value(value.Pass()), count(1) {}
 
-ValueCounter::~ValueCounter() {}
+  scoped_ptr<base::Value> value;
+  int count;
+};
 
-ValueCounter::Entry::Entry(const base::Value& value)
-    : value_(value.DeepCopy()), count_(1) {}
-
-ValueCounter::Entry::~Entry() {}
-
-int ValueCounter::Entry::Increment() { return ++count_; }
+ValueCounter::ValueCounter() {
+}
 
-int ValueCounter::Entry::Decrement() { return --count_; }
+ValueCounter::~ValueCounter() {
+}
 
-int ValueCounter::Add(const base::Value& value) { return AddImpl(value, true); }
+bool ValueCounter::Add(const base::Value& value) {
+  for (Entry* entry : entries_) {
+    if (entry->value->Equals(&value)) {
+      ++entry->count;
+      return false;
+    }
+  }
+  entries_.push_back(new Entry(value.CreateDeepCopy()));
+  return true;
+}
 
-int ValueCounter::Remove(const base::Value& value) {
-  for (EntryList::iterator it = entries_.begin(); it != entries_.end(); it++) {
-    (*it)->value()->GetType();
-    if ((*it)->value()->Equals(&value)) {
-      int remaining = (*it)->Decrement();
-      if (remaining == 0) {
+bool ValueCounter::Remove(const base::Value& value) {
+  for (ScopedVector<Entry>::iterator it = entries_.begin();
+       it != entries_.end(); ++it) {
+    if ((*it)->value->Equals(&value)) {
+      if (--(*it)->count == 0) {
         std::swap(*it, entries_.back());
         entries_.pop_back();
+        return true;  // Removed the last entry.
       }
-      return remaining;
+      return false;  // Removed, but no the last entry.
     }
   }
-  return 0;
-}
-
-int ValueCounter::AddIfMissing(const base::Value& value) {
-  return AddImpl(value, false);
-}
-
-int ValueCounter::AddImpl(const base::Value& value, bool increment) {
-  for (EntryList::iterator it = entries_.begin(); it != entries_.end(); it++) {
-    if ((*it)->value()->Equals(&value))
-      return increment ? (*it)->Increment() : (*it)->count();
-  }
-  entries_.push_back(linked_ptr<Entry>(new Entry(value)));
-  return 1;
+  return false;  // Nothing to remove.
 }
 
 }  // namespace extensions

extensions/common/value_counter.h

@@ -5,9 +5,7 @@
 #ifndef EXTENSIONS_COMMON_VALUE_COUNTER_H_
 #define EXTENSIONS_COMMON_VALUE_COUNTER_H_
 
-#include <vector>
-
-#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_vector.h"
 
 namespace base {
 class Value;
@@ -15,8 +13,8 @@ class Value;
 
 namespace extensions {
 
-// Keeps a running count of Values, like map<Value, int>. Adding / removing
-// values increments / decrements the count associated with a given Value.
+// Keeps a running count of Values, like map<Value, int>. Adding/removing
+// values increments/decrements the count associated with a given Value.
 //
 // Add() and Remove() are linear in the number of Values in the ValueCounter,
 // because there is no operator<() defined on Value, so we must iterate to find
@@ -26,42 +24,22 @@ class ValueCounter {
   ValueCounter();
   ~ValueCounter();
 
-  // Adds |value| to the set and returns how many equal values are in the set
-  // after. Does not take ownership of |value|. In the case where a Value equal
-  // to |value| doesn't already exist in this map, this function makes a
-  // DeepCopy() of |value|.
-  int Add(const base::Value& value);
+  // Adds |value| to the set. In the case where a Value equal to |value|
+  // doesn't already exist in this map, this function makes a copy of |value|
+  // and returns true. Otherwise, it returns false.
+  bool Add(const base::Value& value);
 
-  // Removes |value| from the set and returns how many equal values are in
-  // the set after.
-  int Remove(const base::Value& value);
+  // Removes |value| from the set, and returns true if it removed the last
+  // value equal to |value|. If there are more equal values, or if there
+  // weren't any in the first place, returns false.
+  bool Remove(const base::Value& value);
 
-  // Same as Add() but only performs the add if the value isn't present.
-  int AddIfMissing(const base::Value& value);
+  // Returns true if there are no values of any type being counted.
+  bool is_empty() const { return entries_.empty(); }
 
  private:
-  class Entry {
-   public:
-    explicit Entry(const base::Value& value);
-    ~Entry();
-
-    int Increment();
-    int Decrement();
-
-    const base::Value* value() const { return value_.get(); }
-    int count() const { return count_; }
-
-   private:
-    linked_ptr<base::Value> value_;
-    int count_;
-
-    DISALLOW_COPY_AND_ASSIGN(Entry);
-  };
-  typedef std::vector<linked_ptr<Entry> > EntryList;
-
-  int AddImpl(const base::Value& value, bool increment);
-
-  EntryList entries_;
+  struct Entry;
+  ScopedVector<Entry> entries_;
 
   DISALLOW_COPY_AND_ASSIGN(ValueCounter);
 };

extensions/renderer/event_bindings.cc

@@ -5,9 +5,11 @@
 #include "extensions/renderer/event_bindings.h"
 
 #include <map>
+#include <utility>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
+#include "base/containers/scoped_ptr_map.h"
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
 #include "components/crx_file/id_util.h"
@@ -33,22 +35,23 @@ namespace {
 typedef std::map<std::string, int> EventListenerCounts;
 
 // A map of extension IDs to listener counts for that extension.
-base::LazyInstance<std::map<std::string, EventListenerCounts> >
+base::LazyInstance<std::map<std::string, EventListenerCounts>>
     g_listener_counts = LAZY_INSTANCE_INITIALIZER;
 
-// A map of event names to a (filter -> count) map. The map is used to keep
-// track of which filters are in effect for which events.
-// We notify the browser about filtered event listeners when we transition
-// between 0 and 1.
-typedef std::map<std::string, linked_ptr<ValueCounter> >
-    FilteredEventListenerCounts;
-
-// A map of extension IDs to filtered listener counts for that extension.
-base::LazyInstance<std::map<std::string, FilteredEventListenerCounts> >
-    g_filtered_listener_counts = LAZY_INSTANCE_INITIALIZER;
+// A map of (extension ID, event name) pairs to the filtered listener counts
+// for that pair. The map is used to keep track of which filters are in effect
+// for which events.  We notify the browser about filtered event listeners when
+// we transition between 0 and 1.
+using FilteredEventListenerKey = std::pair<std::string, std::string>;
+using FilteredEventListenerCounts =
+    base::ScopedPtrMap<FilteredEventListenerKey, scoped_ptr<ValueCounter>>;
+base::LazyInstance<FilteredEventListenerCounts> g_filtered_listener_counts =
+    LAZY_INSTANCE_INITIALIZER;
 
 base::LazyInstance<EventFilter> g_event_filter = LAZY_INSTANCE_INITIALIZER;
 
+// Gets a unique string key identifier for a ScriptContext.
+// TODO(kalman): Just use pointer equality...?
 std::string GetKeyForScriptContext(ScriptContext* script_context) {
   const std::string& extension_id = script_context->GetExtensionID();
   CHECK(crx_file::id_util::IdIsValid(extension_id) ||
@@ -101,15 +104,14 @@ EventFilteringInfo ParseFromObject(v8::Local<v8::Object> object,
 // was the first filter for that event in that extension.
 bool AddFilter(const std::string& event_name,
                const std::string& extension_id,
-               base::DictionaryValue* filter) {
-  FilteredEventListenerCounts& counts =
-      g_filtered_listener_counts.Get()[extension_id];
-  FilteredEventListenerCounts::iterator it = counts.find(event_name);
-  if (it == counts.end())
-    counts[event_name].reset(new ValueCounter);
-
-  int result = counts[event_name]->Add(*filter);
-  return 1 == result;
+               const base::DictionaryValue& filter) {
+  FilteredEventListenerKey key(extension_id, event_name);
+  FilteredEventListenerCounts& all_counts = g_filtered_listener_counts.Get();
+  FilteredEventListenerCounts::const_iterator counts = all_counts.find(key);
+  if (counts == all_counts.end()) {
+    counts = all_counts.insert(key, make_scoped_ptr(new ValueCounter())).first;
+  }
+  return counts->second->Add(filter);
 }
 
 // Remove a filter from |event_name| in |extension_id|, returning true if it
@@ -117,12 +119,20 @@ bool AddFilter(const std::string& event_name,
 bool RemoveFilter(const std::string& event_name,
                   const std::string& extension_id,
                   base::DictionaryValue* filter) {
-  FilteredEventListenerCounts& counts =
-      g_filtered_listener_counts.Get()[extension_id];
-  FilteredEventListenerCounts::iterator it = counts.find(event_name);
-  if (it == counts.end())
+  FilteredEventListenerKey key(extension_id, event_name);
+  FilteredEventListenerCounts& all_counts = g_filtered_listener_counts.Get();
+  FilteredEventListenerCounts::const_iterator counts = all_counts.find(key);
+  if (counts == all_counts.end())
     return false;
-  return 0 == it->second->Remove(*filter);
+  // Note: Remove() returns true if it removed the last filter equivalent to
+  // |filter|. If there are more equivalent filters, or if there weren't any in
+  // the first place, it returns false.
+  if (counts->second->Remove(*filter)) {
+    if (counts->second->is_empty())
+      all_counts.erase(counts);  // Clean up if there are no more filters.
+    return true;
+  }
+  return false;
 }
 
 }  // namespace
@@ -136,9 +146,9 @@ EventBindings::EventBindings(ScriptContext* context)
   RouteFunction(
       "AttachFilteredEvent",
       base::Bind(&EventBindings::AttachFilteredEvent, base::Unretained(this)));
-  RouteFunction(
-      "DetachFilteredEvent",
-      base::Bind(&EventBindings::DetachFilteredEvent, base::Unretained(this)));
+  RouteFunction("DetachFilteredEvent",
+                base::Bind(&EventBindings::DetachFilteredEventHandler,
+                           base::Unretained(this)));
   RouteFunction("MatchAgainstEventFilter",
                 base::Bind(&EventBindings::MatchAgainstEventFilter,
                            base::Unretained(this)));
@@ -230,56 +240,53 @@ void EventBindings::AttachFilteredEvent(
   if (!context()->HasAccessOrThrowError(event_name))
     return;
 
-  std::string extension_id = context()->GetExtensionID();
-
   scoped_ptr<base::DictionaryValue> filter;
-  scoped_ptr<content::V8ValueConverter> converter(
-      content::V8ValueConverter::create());
-
-  base::DictionaryValue* filter_dict = NULL;
-  base::Value* filter_value = converter->FromV8Value(
-      v8::Local<v8::Object>::Cast(args[1]), context()->v8_context());
-  if (!filter_value) {
-    args.GetReturnValue().Set(static_cast<int32_t>(-1));
-    return;
-  }
-  if (!filter_value->GetAsDictionary(&filter_dict)) {
-    delete filter_value;
-    args.GetReturnValue().Set(static_cast<int32_t>(-1));
-    return;
+  {
+    scoped_ptr<content::V8ValueConverter> converter(
+        content::V8ValueConverter::create());
+    scoped_ptr<base::Value> filter_value(converter->FromV8Value(
+        v8::Local<v8::Object>::Cast(args[1]), context()->v8_context()));
+    if (!filter_value || !filter_value->IsType(base::Value::TYPE_DICTIONARY)) {
+      args.GetReturnValue().Set(static_cast<int32_t>(-1));
+      return;
+    }
+    filter.reset(static_cast<base::DictionaryValue*>(filter_value.release()));
   }
 
-  filter.reset(filter_dict);
-  EventFilter& event_filter = g_event_filter.Get();
-  int id =
-      event_filter.AddEventMatcher(event_name, ParseEventMatcher(filter.get()));
+  // Hold onto a weak reference to |filter| so that it can be used after passing
+  // ownership to |event_filter|.
+  base::DictionaryValue* filter_weak = filter.get();
+  int id = g_event_filter.Get().AddEventMatcher(
+      event_name, ParseEventMatcher(filter.Pass()));
+  attached_matcher_ids_.insert(id);
 
   // Only send IPCs the first time a filter gets added.
-  if (AddFilter(event_name, extension_id, filter.get())) {
+  std::string extension_id = context()->GetExtensionID();
+  if (AddFilter(event_name, extension_id, *filter_weak)) {
     bool lazy = ExtensionFrameHelper::IsContextForEventPage(context());
     content::RenderThread::Get()->Send(new ExtensionHostMsg_AddFilteredListener(
-        extension_id, event_name, *filter, lazy));
+        extension_id, event_name, *filter_weak, lazy));
   }
 
   args.GetReturnValue().Set(static_cast<int32_t>(id));
 }
 
-void EventBindings::DetachFilteredEvent(
+void EventBindings::DetachFilteredEventHandler(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   CHECK_EQ(2, args.Length());
   CHECK(args[0]->IsInt32());
   CHECK(args[1]->IsBoolean());
-  bool is_manual = args[1]->BooleanValue();
-
-  std::string extension_id = context()->GetExtensionID();
+  DetachFilteredEvent(args[0]->Int32Value(), args[1]->BooleanValue());
+}
 
-  int matcher_id = args[0]->Int32Value();
+void EventBindings::DetachFilteredEvent(int matcher_id, bool is_manual) {
   EventFilter& event_filter = g_event_filter.Get();
   EventMatcher* event_matcher = event_filter.GetEventMatcher(matcher_id);
 
   const std::string& event_name = event_filter.GetEventName(matcher_id);
 
   // Only send IPCs the last time a filter gets removed.
+  std::string extension_id = context()->GetExtensionID();
   if (RemoveFilter(event_name, extension_id, event_matcher->value())) {
     bool remove_lazy =
         is_manual && ExtensionFrameHelper::IsContextForEventPage(context());
@@ -289,6 +296,7 @@ void EventBindings::DetachFilteredEvent(
   }
 
   event_filter.RemoveEventMatcher(matcher_id);
+  attached_matcher_ids_.erase(matcher_id);
 }
 
 void EventBindings::MatchAgainstEventFilter(
@@ -315,10 +323,9 @@ void EventBindings::MatchAgainstEventFilter(
 }
 
 scoped_ptr<EventMatcher> EventBindings::ParseEventMatcher(
-    base::DictionaryValue* filter_dict) {
-  return scoped_ptr<EventMatcher>(new EventMatcher(
-      scoped_ptr<base::DictionaryValue>(filter_dict->DeepCopy()),
-      context()->GetRenderFrame()->GetRoutingID()));
+    scoped_ptr<base::DictionaryValue> filter) {
+  return make_scoped_ptr(new EventMatcher(
+      filter.Pass(), context()->GetRenderFrame()->GetRoutingID()));
 }
 
 void EventBindings::OnInvalidated() {
@@ -330,6 +337,14 @@ void EventBindings::OnInvalidated() {
   }
   DCHECK(attached_event_names_.empty())
       << "Events cannot be attached during invalidation";
+
+  // Same for filtered events.
+  std::set<int> attached_matcher_ids_safe = attached_matcher_ids_;
+  for (int matcher_id : attached_matcher_ids_safe) {
+    DetachFilteredEvent(matcher_id, false /* is_manual */);
+  }
+  DCHECK(attached_matcher_ids_.empty())
+      << "Filtered events cannot be attached during invalidation";
 }
 
 }  // namespace extensions

extensions/renderer/event_bindings.h

@@ -53,23 +53,32 @@ class EventBindings : public ObjectBackedNativeHandler {
   // to MatchAgainstEventFilter where this listener matches.
   void AttachFilteredEvent(const v8::FunctionCallbackInfo<v8::Value>& args);
 
+  // JavaScript handler which forwards to DetachFilteredEvent.
   // void DetachFilteredEvent(int id, bool manual)
-  // id     - Id of the event to detach.
-  // manual - false if this is part of the extension unload process where all
-  //          listeners are automatically detached.
-  void DetachFilteredEvent(const v8::FunctionCallbackInfo<v8::Value>& args);
+  // args[0] forwards to |matcher_id|
+  // args[1] forwards to |is_manual|
+  void DetachFilteredEventHandler(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
+
+  // Detaches a filtered event. Unlike a normal event, a filtered event is
+  // identified by a unique ID per filter, not its name.
+  // |matcher_id| The ID of the filtered event.
+  // |is_manual| false if this is part of the extension unload process where all
+  // listeners are automatically detached.
+  void DetachFilteredEvent(int matcher_id, bool is_manual);
 
   void MatchAgainstEventFilter(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   scoped_ptr<EventMatcher> ParseEventMatcher(
-      base::DictionaryValue* filter_dict);
+      scoped_ptr<base::DictionaryValue> filter);
 
   // Called when our context, and therefore us, is invalidated. Run any cleanup.
   void OnInvalidated();
 
-  // The set of attached events. Maintain this so that we can detch them on
-  // unload.
+  // The set of attached events and filtered events. Maintain these so that we
+  // can detch them on unload.
   std::set<std::string> attached_event_names_;
+  std::set<int> attached_matcher_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(EventBindings);
 };