Attempt to fix a crash in extension bindings' CallFunctionInContext.

My guess is that we were holding onto bad iterators from the context list.

BUG=35065

Review URL: http://codereview.chromium.org/661199

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

2 files changed, 20 insertions, 6 deletions

diff --git a/chrome/renderer/extensions/bindings_utils.h b/chrome/renderer/extensions/bindings_utils.h
index 65e8db8..d1b3636 100644
--- a/chrome/renderer/extensions/bindings_utils.h
+++ b/chrome/renderer/extensions/bindings_utils.h
@@ -91,7 +91,9 @@ struct ContextInfo {
 };
 typedef std::list< linked_ptr<ContextInfo> > ContextList;
 
-// Returns a mutable reference to the ContextList.
+// Returns a mutable reference to the ContextList. Note: be careful using this.
+// Calling into javascript may result in the list being modified, so don't rely
+// on iterators remaining valid between calls to javascript.
 ContextList& GetContexts();
 
 // Returns a (copied) list of contexts that have the given extension_id.
diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc
index 46072bf..a751785 100644
--- a/chrome/renderer/extensions/event_bindings.cc
+++ b/chrome/renderer/extensions/event_bindings.cc
@@ -276,9 +276,14 @@ void EventBindings::HandleContextDestroyed(WebFrame* frame) {
   // itself might not be registered, but can still be a parent frame.
   for (ContextList::iterator it = GetContexts().begin();
        it != GetContexts().end(); ) {
-    ContextList::iterator current = it++;
-    if ((*current)->parent_frame == frame)
-      UnregisterContext(current, false);
+    if ((*it)->parent_frame == frame) {
+      UnregisterContext(it, false);
+      // UnregisterContext will remove |it| from the list, but may also
+      // modify the rest of the list as a result of calling into javascript.
+      it = GetContexts().begin();
+    } else {
+      ++it;
+    }
   }
 }
 
@@ -286,10 +291,17 @@ void EventBindings::HandleContextDestroyed(WebFrame* frame) {
 void EventBindings::CallFunction(const std::string& function_name,
                                  int argc, v8::Handle<v8::Value>* argv,
                                  RenderView* render_view) {
-  for (ContextList::iterator it = GetContexts().begin();
-       it != GetContexts().end(); ++it) {
+  // We copy the context list, because calling into javascript may modify it
+  // out from under us. We also guard against deleted contexts by checking if
+  // they have been cleared first.
+  ContextList contexts = GetContexts();
+
+  for (ContextList::iterator it = contexts.begin();
+       it != contexts.end(); ++it) {
     if (render_view && render_view != (*it)->render_view)
       continue;
+    if ((*it)->context.IsEmpty())
+      continue;
     v8::Handle<v8::Value> retval = CallFunctionInContext((*it)->context,
         function_name, argc, argv);
     // In debug, the js will validate the event parameters and return a