Do not dereference potentially invalid frame pointer.

chrome/renderer/extensions/event_bindings.cc

 // Copyright (c) 2011 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/renderer/extensions/event_bindings.h"
 
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/lazy_instance.h"
@@ skipping 171 matching lines @@
         WebKit::WebString::fromUTF8(file_full_path.c_str()),
         is_directory);
 #else
     return v8::Undefined();
 #endif
   }
 };
 
 // Returns true if the extension running in the given |context| has sufficient
 // permissions to access the data.
-static bool HasSufficientPermissions(ContextInfo* context,
+static bool HasSufficientPermissions(RenderView* render_view,
                                      const GURL& event_url) {
-  v8::Context::Scope context_scope(context->context);
-
   // During unit tests, we might be invoked without a v8 context. In these
   // cases, we only allow empty event_urls and short-circuit before retrieving
   // the render view from the current context.
   if (!event_url.is_valid())
     return true;
 
-  RenderView* renderview = bindings_utils::GetRenderViewForCurrentContext();
-  if (!renderview)
-    return false;
-
-  WebDocument document = renderview->webview()->mainFrame()->document();
+  WebDocument document = render_view->webview()->mainFrame()->document();
   return GURL(document.url()).SchemeIs(chrome::kExtensionScheme) &&
        document.securityOrigin().canRequest(event_url);
 }
 
 }  // namespace
 
 const char* EventBindings::kName = "chrome/EventBindings";
 const char* EventBindings::kTestingExtensionId =
     "oooooooooooooooooooooooooooooooo";
 
@@ skipping 74 matching lines @@
     v8::Handle<v8::Context> context,
     ExtensionDispatcher* extension_dispatcher,
     int isolated_world_id) {
   if (!bindings_registered)
     return;
 
   bool content_script = isolated_world_id != 0;
   ContextList& contexts = GetContexts();
 
   v8::Persistent<v8::Context> persistent_context;
+  v8::Persistent<v8::Context> main_world_context;
   std::string extension_id;
 
   if (content_script) {
     // Content script contexts can get GCed before their frame goes away, so
     // set up a GC callback.
     persistent_context = v8::Persistent<v8::Context>::New(context);
     persistent_context.MakeWeak(NULL, &ContextWeakReferenceCallback);
+
+    main_world_context = v8::Persistent<v8::Context>::New(
+        frame->mainWorldScriptContext());
+    main_world_context.MakeWeak(NULL, NULL);
+
     extension_id =
         extension_dispatcher->user_script_slave()->
             GetExtensionIdForIsolatedWorld(isolated_world_id);
   } else {
     // Figure out the frame's URL.  If the frame is loading, use its provisional
     // URL, since we get this notification before commit.
     WebDataSource* ds = frame->provisionalDataSource();
     if (!ds)
       ds = frame->dataSource();
     GURL url = ds->request().url();
@@ skipping 11 matching lines @@
 
       // For tests, we want the dispatchOnLoad to actually setup our bindings,
       // so we give a fake extension id;
       extension_id = kTestingExtensionId;
     }
 
     persistent_context = v8::Persistent<v8::Context>::New(context);
   }
 
   contexts.push_back(linked_ptr<ContextInfo>(
-      new ContextInfo(persistent_context, extension_id, frame)));
+      new ContextInfo(persistent_context, main_world_context, extension_id)));
 
   // Content scripts get initialized in user_script_slave.cc.
   if (!content_script) {
     v8::HandleScope handle_scope;
     v8::Handle<v8::Value> argv[1];
     argv[0] = v8::String::New(extension_id.c_str());
     CallFunctionInContext(context, "dispatchOnLoad", arraysize(argv), argv);
   }
 }
 
 // static
 void EventBindings::HandleContextDestroyed(WebFrame* frame) {
   if (!bindings_registered)
     return;
 
   v8::HandleScope handle_scope;
   v8::Local<v8::Context> context = frame->mainWorldScriptContext();
-  if (!context.IsEmpty()) {
-    ContextList::iterator context_iter = bindings_utils::FindContext(context);
-    if (context_iter != GetContexts().end())
-      UnregisterContext(context_iter, false);
+
+  if (context.IsEmpty()) {
+    NOTREACHED();

[Aaron Boodman, 2011/08/24 02:58:16]

According to the code upstream, this should not happen: 

http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/v8/V8DOMWindowSh...

And it seems like this code would have been useless before if it did happen
anyway, so I just made that explicit.

+    return;
   }
 
-  // Unload any content script contexts for this frame.  Note that the frame
-  // itself might not be registered, but can still be a parent frame.
+  ContextList::iterator context_iter = bindings_utils::FindContext(context);
+  if (context_iter != GetContexts().end())
+    UnregisterContext(context_iter, false);
+
+  // Unload any associated content script contexts. Note that the main world
+  // context itself might not be registered, but can still be associated with a
+  // content script that is.
   for (ContextList::iterator it = GetContexts().begin();
        it != GetContexts().end(); ) {
-    if ((*it)->frame == frame) {
+    if ((*it)->main_world_context == context) {
       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;
     }
   }
 }
 
 // static
 void EventBindings::CallFunction(const std::string& extension_id,
                                  const std::string& function_name,
                                  const ListValue& arguments,
                                  RenderView* render_view,
                                  const GURL& event_url) {
   v8::HandleScope handle_scope;
 
   // 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();
 
   V8ValueConverter converter;
   for (ContextList::iterator it = contexts.begin();
        it != contexts.end(); ++it) {
-    if (render_view) {
-      RenderView* context_render_view =
-          RenderView::FromWebView((*it)->frame->view());
-      if (render_view != context_render_view)
-        continue;
-    }
+    if ((*it)->context.IsEmpty())
+      continue;
 
     if (!extension_id.empty() && extension_id != (*it)->extension_id)
       continue;
 
-    if ((*it)->context.IsEmpty())
+    WebFrame* context_frame = WebFrame::frameForContext((*it)->context);

[Aaron Boodman, 2011/08/24 02:58:16]

It will return NULL if the context is detached from the frame (like after
navigation).

+    if (!context_frame || !context_frame->view())
       continue;
 
-    if (!HasSufficientPermissions(it->get(), event_url))
+    RenderView* context_render_view =
+        RenderView::FromWebView(context_frame->view());
+    if (!context_render_view)
+      continue;
+
+    if (render_view && render_view != context_render_view)
+      continue;
+
+    if (!HasSufficientPermissions(render_view, event_url))
       continue;
 
     v8::Local<v8::Context> context(*((*it)->context));
     std::vector<v8::Handle<v8::Value> > v8_arguments;
     for (size_t i = 0; i < arguments.GetSize(); ++i) {
       Value* item = NULL;
       CHECK(arguments.Get(i, &item));
       v8_arguments.push_back(converter.ToV8Value(item, context));
     }
 
     v8::Handle<v8::Value> retval = CallFunctionInContext(
         context, function_name, v8_arguments.size(), &v8_arguments[0]);
     // 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
   }
 }