Implement a module system for the extension bindings JS.

chrome/renderer/extensions/event_bindings.cc

 // Copyright (c) 2012 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 21 matching lines @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
 #include "v8/include/v8.h"
 
 using WebKit::WebFrame;
 using WebKit::WebSecurityOrigin;
 using WebKit::WebURL;
 using content::RenderThread;
 
 namespace {
 
+// TODO(koz): Merge this into EventBindings.
 class ExtensionImpl : public ChromeV8Extension {
  public:
-  explicit ExtensionImpl(ExtensionDispatcher* dispatcher)
-      : ChromeV8Extension("extensions/event.js",
-                          IDR_EVENT_BINDINGS_JS,
-                          dispatcher) {
+  // A map of event names to the number of contexts listening to that event.
+  // We notify the browser about event listeners when we transition between 0
+  // and 1.
+  typedef std::map<std::string, int> EventListenerCounts;
+
+  ExtensionImpl(ExtensionDispatcher* dispatcher,
+      std::map<std::string, EventListenerCounts>* listener_counts)
+      : ChromeV8Extension(dispatcher),
+        listener_counts_(listener_counts) {
+    RouteStaticFunction("AttachEvent", &AttachEvent);
+    RouteStaticFunction("DetachEvent", &DetachEvent);

[Matt Perry, 2012/02/28 20:34:20]

Both of these access the ExtensionImpl via
GetFromArguments<ExtensionImpl>(args); Would they not need to do so if they
became member functions, and used RouteFunction?

[koz (OOO until 15th September), 2012/03/01 03:41:56]

Yes, that's right. I was planning on making those conversions in a separate
change to reduce the number of delta lines in this one.

   }
   ~ExtensionImpl() {}
 
-  virtual v8::Handle<v8::FunctionTemplate> GetNativeFunction(
-      v8::Handle<v8::String> name) {
-    if (name->Equals(v8::String::New("AttachEvent"))) {
-      return v8::FunctionTemplate::New(AttachEvent, v8::External::New(this));
-    } else if (name->Equals(v8::String::New("DetachEvent"))) {
-      return v8::FunctionTemplate::New(DetachEvent, v8::External::New(this));
-    }
-    return ChromeV8Extension::GetNativeFunction(name);
-  }
-
   // Attach an event name to an object.
   static v8::Handle<v8::Value> AttachEvent(const v8::Arguments& args) {
     DCHECK(args.Length() == 1);
     // TODO(erikkay) should enforce that event name is a string in the bindings
     DCHECK(args[0]->IsString() || args[0]->IsUndefined());
 
     if (args[0]->IsString()) {
       ExtensionImpl* self = GetFromArguments<ExtensionImpl>(args);
       const ChromeV8ContextSet& context_set =
           self->extension_dispatcher()->v8_context_set();
       ChromeV8Context* context = context_set.GetCurrent();
       CHECK(context);
       std::string event_name(*v8::String::AsciiValue(args[0]));
 
       if (!self->CheckCurrentContextAccessToExtensionAPI(event_name))
         return v8::Undefined();
 
       EventListenerCounts& listener_counts =
-          self->listener_counts_[context->extension_id()];
+          (*(self->listener_counts_))[context->extension_id()];
       if (++listener_counts[event_name] == 1) {
         content::RenderThread::Get()->Send(
             new ExtensionHostMsg_AddListener(context->extension_id(),
                                              event_name));
       }
 
       // This is called the first time the page has added a listener. Since
       // the background page is the only lazy page, we know this is the first
       // time this listener has been registered.
       if (self->IsLazyBackgroundPage(context->extension_id())) {
@@ skipping 13 matching lines @@
 
     if (args[0]->IsString() && args[1]->IsBoolean()) {
       ExtensionImpl* self = GetFromArguments<ExtensionImpl>(args);
       const ChromeV8ContextSet& context_set =
           self->extension_dispatcher()->v8_context_set();
       ChromeV8Context* context = context_set.GetCurrent();
       if (!context)
         return v8::Undefined();
 
       EventListenerCounts& listener_counts =
-          self->listener_counts_[context->extension_id()];
+          (*(self->listener_counts_))[context->extension_id()];
       std::string event_name(*v8::String::AsciiValue(args[0]));
       bool is_manual = args[1]->BooleanValue();
 
       if (--listener_counts[event_name] == 0) {
         content::RenderThread::Get()->Send(
             new ExtensionHostMsg_RemoveListener(context->extension_id(),
                                                 event_name));
       }
 
       // DetachEvent is called when the last listener for the context is
       // removed. If the context is the background page, and it removes the
       // last listener manually, then we assume that it is no longer interested
       // in being awakened for this event.
       if (is_manual && self->IsLazyBackgroundPage(context->extension_id())) {
         content::RenderThread::Get()->Send(
             new ExtensionHostMsg_RemoveLazyListener(context->extension_id(),
                                                     event_name));
       }
     }
 
     return v8::Undefined();
   }
 
  private:
-  // A map of event names to the number of contexts listening to that event.
-  // We notify the browser about event listeners when we transition between 0
-  // and 1.
-  typedef std::map<std::string, int> EventListenerCounts;
 
   bool IsLazyBackgroundPage(const std::string& extension_id) {
     content::RenderView* render_view = GetCurrentRenderView();
     if (!render_view)
       return false;
 
     ExtensionHelper* helper = ExtensionHelper::Get(render_view);
     const ::Extension* extension =
         extension_dispatcher()->extensions()->GetByID(extension_id);
     return (extension && !extension->background_page_persists() &&
             helper->view_type() == chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
   }
 
-  // A map of extension IDs to listener counts for that extension.
-  std::map<std::string, EventListenerCounts> listener_counts_;
+  std::map<std::string, ExtensionImpl::EventListenerCounts>* listener_counts_;
 };
 
 }  // namespace
 
-v8::Extension* EventBindings::Get(ExtensionDispatcher* dispatcher) {
-  static v8::Extension* extension = new ExtensionImpl(dispatcher);
-  return extension;
+ChromeV8Extension* EventBindings::Get(ExtensionDispatcher* dispatcher) {
+  // A map of extension IDs to listener counts for that extension.
+  static std::map<std::string, ExtensionImpl::EventListenerCounts>*
+      listener_counts = NULL;
+  if (!listener_counts) {
+    listener_counts =
+        new std::map<std::string, ExtensionImpl::EventListenerCounts>();
+  }
+  return new ExtensionImpl(dispatcher, listener_counts);

[Matt Perry, 2012/02/28 20:34:20]

I don't understand this set of changes. Why change the ownership of
listener_counts? It will leak on shutdown. That may anger the memory bots. And
why return a new ExtensionImpl each time instead of caching it in a static var?

[koz (OOO until 15th September), 2012/03/01 03:41:56]

With this change ChromeV8Extensions are no longer singletons, so their state is
now per-context instead of per-process, so to preserve the semantics here
listener_counts_ needs to be shared across all the instances.

I've changed this to use LazyInstance (as per aa's comment elsewhere).

 }