Debugging patch to help track down skipped cleanup of ScriptContexts.

The current theory is that cleanup for some ScriptContexts is getting
skipped, due to willReleaseScriptContext() notifications not being sent.
Alias a few things into the debug crash dump to try to narrow down what
could be causing this.

BUG=441968

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

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

12 files changed, 62 insertions, 16 deletions

diff --git a/extensions/renderer/default_dispatcher_delegate.cc b/extensions/renderer/default_dispatcher_delegate.cc
index 25af678..0e10c53 100644
--- a/extensions/renderer/default_dispatcher_delegate.cc
+++ b/extensions/renderer/default_dispatcher_delegate.cc
@@ -18,16 +18,14 @@ DefaultDispatcherDelegate::~DefaultDispatcherDelegate() {
 scoped_ptr<ScriptContext> DefaultDispatcherDelegate::CreateScriptContext(
     const v8::Handle<v8::Context>& v8_context,
     blink::WebFrame* frame,
+    int world_id,
     const Extension* extension,
     Feature::Context context_type,
     const Extension* effective_extension,
     Feature::Context effective_context_type) {
-  return make_scoped_ptr(new ScriptContext(v8_context,
-                                           frame,
-                                           extension,
-                                           context_type,
-                                           effective_extension,
-                                           effective_context_type));
+  return make_scoped_ptr(
+      new ScriptContext(v8_context, frame, world_id, extension, context_type,
+                        effective_extension, effective_context_type));
 }
 
 }  // namespace extensions
diff --git a/extensions/renderer/default_dispatcher_delegate.h b/extensions/renderer/default_dispatcher_delegate.h
index bbac20b..5a5473c 100644
--- a/extensions/renderer/default_dispatcher_delegate.h
+++ b/extensions/renderer/default_dispatcher_delegate.h
@@ -18,6 +18,7 @@ class DefaultDispatcherDelegate : public DispatcherDelegate {
   scoped_ptr<ScriptContext> CreateScriptContext(
       const v8::Handle<v8::Context>& v8_context,
       blink::WebFrame* frame,
+      int world_id,
       const Extension* extension,
       Feature::Context context_type,
       const Extension* effective_extension,
diff --git a/extensions/renderer/dispatcher.cc b/extensions/renderer/dispatcher.cc
index 24b2b60..2126c28 100644
--- a/extensions/renderer/dispatcher.cc
+++ b/extensions/renderer/dispatcher.cc
@@ -281,11 +281,8 @@ void Dispatcher::DidCreateScriptContext(
       frame->document().securityOrigin());
 
   ScriptContext* context =
-      delegate_->CreateScriptContext(v8_context,
-                                     frame,
-                                     extension,
-                                     context_type,
-                                     effective_extension,
+      delegate_->CreateScriptContext(v8_context, frame, world_id, extension,
+                                     context_type, effective_extension,
                                      effective_context_type).release();
   script_context_set_.Add(context);
 
@@ -369,6 +366,10 @@ void Dispatcher::WillReleaseScriptContext(
   VLOG(1) << "Num tracked contexts: " << script_context_set_.size();
 }
 
+void Dispatcher::FrameDetached(blink::WebFrame* frame) {
+  script_context_set_.RemoveForFrame(frame);
+}
+
 void Dispatcher::DidCreateDocumentElement(blink::WebFrame* frame) {
   // Note: use GetEffectiveDocumentURL not just frame->document()->url()
   // so that this also injects the stylesheet on about:blank frames that
diff --git a/extensions/renderer/dispatcher.h b/extensions/renderer/dispatcher.h
index 021fc943..9395af2 100644
--- a/extensions/renderer/dispatcher.h
+++ b/extensions/renderer/dispatcher.h
@@ -107,6 +107,8 @@ class Dispatcher : public content::RenderProcessObserver,
                                 const v8::Handle<v8::Context>& context,
                                 int world_id);
 
+  void FrameDetached(blink::WebFrame* frame);
+
   void DidCreateDocumentElement(blink::WebFrame* frame);
 
   void DidMatchCSS(
diff --git a/extensions/renderer/dispatcher_delegate.h b/extensions/renderer/dispatcher_delegate.h
index 261e580..d237878 100644
--- a/extensions/renderer/dispatcher_delegate.h
+++ b/extensions/renderer/dispatcher_delegate.h
@@ -35,6 +35,7 @@ class DispatcherDelegate {
   virtual scoped_ptr<ScriptContext> CreateScriptContext(
       const v8::Handle<v8::Context>& v8_context,
       blink::WebFrame* frame,
+      int world_id,
       const Extension* extension,
       Feature::Context context_type,
       const Extension* effective_extension,
diff --git a/extensions/renderer/module_system_test.cc b/extensions/renderer/module_system_test.cc
index b9d4a4d..25d1d6d 100644
--- a/extensions/renderer/module_system_test.cc
+++ b/extensions/renderer/module_system_test.cc
@@ -130,6 +130,7 @@ ModuleSystemTestEnvironment::ModuleSystemTestEnvironment(v8::Isolate* isolate)
       isolate, g_v8_extension_configurator.Get().GetConfiguration()));
   context_.reset(new ScriptContext(context_holder_->context(),
                                    NULL,  // WebFrame
+                                   -1,    // World ID
                                    NULL,  // Extension
                                    Feature::BLESSED_EXTENSION_CONTEXT,
                                    NULL,  // Effective Extension
diff --git a/extensions/renderer/script_context.cc b/extensions/renderer/script_context.cc
index de43150..20c99d6 100644
--- a/extensions/renderer/script_context.cc
+++ b/extensions/renderer/script_context.cc
@@ -76,12 +76,14 @@ class ScriptContext::Runner : public gin::Runner {
 
 ScriptContext::ScriptContext(const v8::Handle<v8::Context>& v8_context,
                              blink::WebFrame* web_frame,
+                             int world_id,
                              const Extension* extension,
                              Feature::Context context_type,
                              const Extension* effective_extension,
                              Feature::Context effective_context_type)
     : v8_context_(v8_context),
       web_frame_(web_frame),
+      world_id_(world_id),
       extension_(extension),
       context_type_(context_type),
       effective_extension_(effective_extension),
diff --git a/extensions/renderer/script_context.h b/extensions/renderer/script_context.h
index 5fd87df..b34e426 100644
--- a/extensions/renderer/script_context.h
+++ b/extensions/renderer/script_context.h
@@ -36,6 +36,7 @@ class ScriptContext : public RequestSender::Source {
  public:
   ScriptContext(const v8::Handle<v8::Context>& context,
                 blink::WebFrame* frame,
+                int world_id,
                 const Extension* extension,
                 Feature::Context context_type,
                 const Extension* effective_extension,
@@ -62,6 +63,8 @@ class ScriptContext : public RequestSender::Source {
 
   blink::WebFrame* web_frame() const { return web_frame_; }
 
+  int world_id() const { return world_id_; }
+
   Feature::Context context_type() const { return context_type_; }
 
   Feature::Context effective_context_type() const {
@@ -162,6 +165,9 @@ class ScriptContext : public RequestSender::Source {
   // object can outlive is destroyed asynchronously.
   blink::WebFrame* web_frame_;
 
+  // The world ID for the associated context, for debugging purposes.
+  const int world_id_;
+
   // The extension associated with this context, or NULL if there is none. This
   // might be a hosted app in the case that this context is hosting a web URL.
   scoped_refptr<const Extension> extension_;
diff --git a/extensions/renderer/script_context_set.cc b/extensions/renderer/script_context_set.cc
index 5f7be6c1..726827b 100644
--- a/extensions/renderer/script_context_set.cc
+++ b/extensions/renderer/script_context_set.cc
@@ -4,6 +4,8 @@
 
 #include "extensions/renderer/script_context_set.h"
 
+#include "base/debug/alias.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/message_loop/message_loop.h"
 #include "content/public/renderer/render_view.h"
 #include "extensions/common/extension.h"
@@ -42,6 +44,32 @@ void ScriptContextSet::Remove(ScriptContext* context) {
   }
 }
 
+void ScriptContextSet::RemoveForFrame(blink::WebFrame* frame) {
+  // It is a major problem if there are any remaining contexts associated with a
+  // WebFrame is about to be detached. It is too late to fire the extension
+  // onunload event, and the ScriptContext will try to use a WebFrame after it
+  // may have been freed.
+  static const int kMaxWorldIdsToSave = 10;
+  int saved_world_ids_count = 0;
+  int saved_world_ids[kMaxWorldIdsToSave] = {};
+  for (ContextSet::iterator iter = contexts_.begin();
+       iter != contexts_.end();) {
+    ScriptContext* context = *iter++;
+    if (context->web_frame() == frame) {
+      if (saved_world_ids_count < kMaxWorldIdsToSave)
+        saved_world_ids[saved_world_ids_count++] = context->world_id();
+      Remove(context);
+    }
+  }
+#if !defined(OS_LINUX)
+  // DumpWithoutCrashing() crashes in Linux in renderers with breakpad +
+  // sandboxing. https://crbug.com/349600
+  base::debug::Alias(&saved_world_ids_count);
+  base::debug::Alias(saved_world_ids);
+  base::debug::DumpWithoutCrashing();
+#endif
+}
+
 ScriptContextSet::ContextSet ScriptContextSet::GetAll() const {
   return contexts_;
 }
diff --git a/extensions/renderer/script_context_set.h b/extensions/renderer/script_context_set.h
index aa7b253..8f2c7ef 100644
--- a/extensions/renderer/script_context_set.h
+++ b/extensions/renderer/script_context_set.h
@@ -18,6 +18,10 @@ namespace base {
 class ListValue;
 }
 
+namespace blink {
+class WebFrame;
+}
+
 namespace content {
 class RenderView;
 }
@@ -47,6 +51,10 @@ class ScriptContextSet {
   // be valid, but its frame() pointer will be cleared.
   void Remove(ScriptContext* context);
 
+  // Do not use this method. It has been temporarily added for debugging
+  // purposes (see https://crbug.com/441968).
+  void RemoveForFrame(blink::WebFrame* frame);
+
   // Returns a copy to protect against changes.
   typedef std::set<ScriptContext*> ContextSet;
   ContextSet GetAll() const;
diff --git a/extensions/renderer/script_context_set_unittest.cc b/extensions/renderer/script_context_set_unittest.cc
index e795dd9..a6114f9 100644
--- a/extensions/renderer/script_context_set_unittest.cc
+++ b/extensions/renderer/script_context_set_unittest.cc
@@ -31,11 +31,8 @@ TEST(ScriptContextSet, Lifecycle) {
   webview->setMainFrame(frame);
   const Extension* extension = NULL;
   ScriptContext* context =
-      new ScriptContext(context_holder.context(),
-                        frame,
-                        extension,
-                        Feature::BLESSED_EXTENSION_CONTEXT,
-                        extension,
+      new ScriptContext(context_holder.context(), frame, 0, extension,
+                        Feature::BLESSED_EXTENSION_CONTEXT, extension,
                         Feature::BLESSED_EXTENSION_CONTEXT);
 
   context_set.Add(context);
diff --git a/extensions/renderer/v8_schema_registry.cc b/extensions/renderer/v8_schema_registry.cc
index d2d5e0e..9e78589 100644
--- a/extensions/renderer/v8_schema_registry.cc
+++ b/extensions/renderer/v8_schema_registry.cc
@@ -51,6 +51,7 @@ scoped_ptr<NativeHandler> V8SchemaRegistry::AsNativeHandler() {
   scoped_ptr<ScriptContext> context(
       new ScriptContext(GetOrCreateContext(v8::Isolate::GetCurrent()),
                         NULL,  // no frame
+                        -1,    // no world
                         NULL,  // no extension
                         Feature::UNSPECIFIED_CONTEXT,
                         NULL,  // no effective extension