[Extensions] Wait until the Window object is cleared before classifying contexts

extensions/renderer/extension_frame_helper.cc

Index: extensions/renderer/extension_frame_helper.cc
diff --git a/extensions/renderer/extension_frame_helper.cc b/extensions/renderer/extension_frame_helper.cc
index fe581c331fd845868386be85175a7fe91f97244b..a36001ad2185d645be864a1651e3b5cc2f025cb3 100644
--- a/extensions/renderer/extension_frame_helper.cc
+++ b/extensions/renderer/extension_frame_helper.cc
@@ -47,10 +47,26 @@ bool RenderFrameMatches(const ExtensionFrameHelper* frame_helper,
 
 }  // namespace
 
+struct ExtensionFrameHelper::PendingContext {
+  v8::Global<v8::Context> v8_context;
+  int extension_group;
+  int world_id;
+  v8::Isolate* isolate;
+
+  PendingContext(v8::Local<v8::Context> context,
+                 int extension_group,
+                 int world_id)
+      : v8_context(context->GetIsolate(), context),
+        extension_group(extension_group),
+        world_id(world_id),
+        isolate(context->GetIsolate()) {}
+};
+
 ExtensionFrameHelper::ExtensionFrameHelper(content::RenderFrame* render_frame,
                                            Dispatcher* extension_dispatcher)
     : content::RenderFrameObserver(render_frame),
       content::RenderFrameObserverTracker<ExtensionFrameHelper>(render_frame),
+      did_clear_window_(false),
       view_type_(VIEW_TYPE_INVALID),
       tab_id_(-1),
       browser_window_id_(-1),
@@ -110,6 +126,10 @@ void ExtensionFrameHelper::DidCreateNewDocument() {
   did_create_current_document_element_ = false;
 }
 
+void ExtensionFrameHelper::DidStartProvisionalLoad() {
+  did_clear_window_ = false;
+}
+
 void ExtensionFrameHelper::DidMatchCSS(
     const blink::WebVector<blink::WebString>& newly_matching_selectors,
     const blink::WebVector<blink::WebString>& stopped_matching_selectors) {
@@ -122,15 +142,42 @@ void ExtensionFrameHelper::DidCreateScriptContext(
     v8::Local<v8::Context> context,
     int extension_group,
     int world_id) {
-  extension_dispatcher_->DidCreateScriptContext(
-      render_frame()->GetWebFrame(), context, extension_group, world_id);
+  if (did_clear_window_) {
+    extension_dispatcher_->DidCreateScriptContext(
+        render_frame()->GetWebFrame(), context, extension_group, world_id);
+  } else {
+    pending_contexts_.push_back(make_scoped_ptr(
+        new PendingContext(context, extension_group, world_id)));
+  }
+}
+
+void ExtensionFrameHelper::DidClearWindowObject() {
+  did_clear_window_ = true;
+  for (const auto& pending_context : pending_contexts_) {
+    v8::HandleScope scope(pending_context->isolate);
+    v8::Local<v8::Context> context_local =

[dcheng, 2015/11/18 18:42:51]

Sorry: my comment was unclear. You can just call this "context" (and the struct
member can stay named context). My only concern was a collision between the loop
variable and the name of this local. I leave it up to you what looks best.

+        v8::Local<v8::Context>::New(pending_context->isolate,
+                                    pending_context->v8_context);
+    v8::Context::Scope context_scope(context_local);
+    extension_dispatcher_->DidCreateScriptContext(
+        render_frame()->GetWebFrame(), context_local,
+        pending_context->extension_group, pending_context->world_id);
+  }
+  pending_contexts_.clear();
 }
 
 void ExtensionFrameHelper::WillReleaseScriptContext(
-    v8::Local<v8::Context> context,
+    v8::Local<v8::Context> v8_context,
     int world_id) {
-  extension_dispatcher_->WillReleaseScriptContext(
-      render_frame()->GetWebFrame(), context, world_id);
+  for (auto iter = pending_contexts_.begin(); iter != pending_contexts_.end();
+       ++iter) {
+    if ((*iter)->v8_context == v8_context) {
+      pending_contexts_.erase(iter);
+      return;
+    }
+  }
+  extension_dispatcher_->WillReleaseScriptContext(render_frame()->GetWebFrame(),
+                                                  v8_context, world_id);
 }
 
 bool ExtensionFrameHelper::OnMessageReceived(const IPC::Message& message) {