Call MessageFilter clean up handlers even for pending filters

ipc/ipc_channel_proxy.cc

Index: ipc/ipc_channel_proxy.cc
diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index f99274f1555f422e13138a3c6ca4648cec434fd5..1e8bec057be0410d0694cd7343048ae3b53136bf 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -211,9 +211,13 @@ void ChannelProxy::Context::OnChannelClosed() {
   if (!channel_)
     return;
 
-  for (size_t i = 0; i < filters_.size(); ++i) {
-    filters_[i]->OnChannelClosing();
-    filters_[i]->OnFilterRemoved();
+  for (auto& filter : pending_filters_) {

[Ken Rockot(use gerrit already), 2016/11/03 05:17:12]

Unlike filters_, pending_filters_ is accessed from different threads and needs
to be guarded by lock. This is a data race otherwise.

Note that this still doesn't account for filters which are added during or after
channel closure. i.e. if someone calls AddFilter on the main thread while
OnChannelClosed() is running on the IO thread (or any time after), that can
result in a filter being added to pending_filters_ (eventually moved to
filters_) and having OnFilterAdded() invoked on the filter. Such filters will
never have OnChannelClosing() or OnFilterRemoved() invoked.

You may want to address this case as well if your intent is to ensure that *all*
filters passed to AddFilter() guarantee these calls.

[tzik, 2016/11/04 07:13:56]

So, is the comment around line 226 no longer valid now?

[Ken Rockot(use gerrit already), 2016/11/04 20:05:30]

Oops. Sorry, I was conflating OnChannelError and OnChannelClosed in my head.
This should be fine as-is.

+    filter->OnChannelClosing();
+    filter->OnFilterRemoved();

[boliu, 2016/11/07 18:03:48]

the bug is filters in pending_fitlers_ here have never been added in the first
place, so calling OnFilterRemoved is wrong

I think this applies to OnChannelClosing, since OnChannelConnected never
happened either, so fix is just remove this whole block.., which reverts this
part of the CL..?

+  }
+  for (auto& filter : filters_) {
+    filter->OnChannelClosing();
+    filter->OnFilterRemoved();
   }
 
   // We don't need the filters anymore.