Move Extension ScriptContext creation into ScriptContextSet.

extensions/renderer/script_context_set.h

Index: extensions/renderer/script_context_set.h
diff --git a/extensions/renderer/script_context_set.h b/extensions/renderer/script_context_set.h
index aa7b253d57742bbff456be1c06a024a87ce2dcbf..ac3cb17840934007023ec4dc66017e54a5e73033 100644
--- a/extensions/renderer/script_context_set.h
+++ b/extensions/renderer/script_context_set.h
@@ -8,8 +8,12 @@
 #include <set>
 #include <string>
 
-#include "base/basictypes.h"
-#include "base/bind.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_set.h"
+#include "extensions/common/features/feature.h"
+#include "url/gurl.h"
 #include "v8/include/v8.h"
 
 class GURL;
@@ -18,39 +22,50 @@ namespace base {
 class ListValue;
 }
 
-namespace content {
-class RenderView;
+namespace blink {
+class WebLocalFrame;
+class WebSecurityOrigin;
 }
 
-namespace v8 {
-class Context;
+namespace content {
+class RenderView;
 }
 
 namespace extensions {
 class ScriptContext;
 
-// A container of ExtensionBindingsContext. Since calling JavaScript within a
-// context can cause any number of contexts to be created or destroyed, this
-// has additional smarts to help with the set changing underneath callers.
+// A container of ScriptContexts, responsible for both creating and managing
+// them.
+//
+// Since calling JavaScript within a context can cause any number of contexts
+// to be created or destroyed, this has additional smarts to help with the set
+// changing underneath callers.
 class ScriptContextSet {
  public:
-  ScriptContextSet();
+  ScriptContextSet(
+      ExtensionSet* extensions,
+      // Set of the IDs of extensions that are active in this process.
+      // Must outlive this. TODO(kalman): Combine this and |extensions|.

[not at google - send to devlin, 2015/03/12 16:59:57]

FYI by "combine this" I mean that the renderer side of storing a set of
extensions would be better as a new ExtensionSet-style class with Is/SetActive()
methods on it, rather than this awkward parallel set.

+      ExtensionIdSet* active_extension_ids);
+
   ~ScriptContextSet();
 
-  int size() const;
+  // Returns the number of contexts being tracked by this set.
+  // This may also include invalid contexts. TODO(kalman): Useful?
+  size_t size() const { return contexts_.size(); }
 
-  // Takes ownership of |context|.
-  void Add(ScriptContext* context);
+  // Creates and starts managing a new ScriptContext. Ownership is held.
+  // Returns a weak reference to the new ScriptContext.
+  ScriptContext* Register(blink::WebLocalFrame* frame,

[not at google - send to devlin, 2015/03/12 16:59:57]

I'm not particularly proud of this function being called "Register" - though
imagine a world in which instead of returning a ScriptContext* it takes a
base::Callback<void(ScriptContext*)> and returns void. I.e. lazily creates these
ScriptContexts*. That should, hopefully be coming up next.

(and I avoided Create because that almost always implies that ownership is
passed to caller)

[Ken Rockot(use gerrit already), 2015/03/12 22:45:16]

Seems fine to me. Doubly so with the future callback.

[not at google - send to devlin, 2015/03/12 22:48:34]

Alrightee.

+                          const v8::Handle<v8::Context>& v8_context,
+                          int extension_group,
+                          int world_id);
 
   // If the specified context is contained in this set, remove it, then delete
   // it asynchronously. After this call returns the context object will still
   // be valid, but its frame() pointer will be cleared.
   void Remove(ScriptContext* context);
 
-  // Returns a copy to protect against changes.
-  typedef std::set<ScriptContext*> ContextSet;
-  ContextSet GetAll() const;
-
   // Gets the ScriptContext corresponding to v8::Context::GetCurrent(), or
   // NULL if no such context exists.
   ScriptContext* GetCurrent() const;
@@ -61,7 +76,7 @@ class ScriptContextSet {
 
   // Gets the ScriptContext corresponding to the specified
   // v8::Context or NULL if no such context exists.
-  ScriptContext* GetByV8Context(v8::Handle<v8::Context> context) const;
+  ScriptContext* GetByV8Context(const v8::Handle<v8::Context>& context) const;
 
   // Synchronously runs |callback| with each ScriptContext that belongs to
   // |extension_id| in |render_view|.
@@ -88,10 +103,39 @@ class ScriptContextSet {
   // Returns the set of ScriptContexts that were removed as a result. These
   // are safe to interact with until the end of the current event loop, since
   // they're deleted asynchronously.
-  ContextSet OnExtensionUnloaded(const std::string& extension_id);
+  std::set<ScriptContext*> OnExtensionUnloaded(const std::string& extension_id);
 
  private:
-  ContextSet contexts_;
+  // Finds the extension for the JavaScript context associated with the
+  // specified |frame| and isolated world. If |world_id| is zero, finds the
+  // extension ID associated with the main world's JavaScript context. If the
+  // JavaScript context isn't from an extension, returns empty string.
+  const Extension* GetExtensionFromFrameAndWorld(
+      const blink::WebLocalFrame* frame,
+      int world_id,
+      bool use_effective_url);
+
+  // Returns the Feature::Context type of context for a JavaScript context.
+  Feature::Context ClassifyJavaScriptContext(
+      const Extension* extension,
+      int extension_group,
+      const GURL& url,
+      const blink::WebSecurityOrigin& origin);
+
+  // Calls Remove on |context| then appends |context| to |out|.
+  // This is a helper designed to be used by OnExtensionUnloaded with ForEach.
+  void DispatchOnUnloadEventAndRemove(std::set<ScriptContext*>* out,
+                                      ScriptContext* context);
+
+  // Weak reference to all installed Extensions.
+  ExtensionSet* extensions_;
+
+  // Weak reference to all installed Extensions that are also active in this
+  // process.
+  ExtensionIdSet* active_extension_ids_;
+
+  // The set of all ScriptContexts we own.
+  std::set<ScriptContext*> contexts_;
 
   DISALLOW_COPY_AND_ASSIGN(ScriptContextSet);
 };