Remove the refcount from ScriptBadgeController.

chrome/browser/extensions/script_executor.h

Index: chrome/browser/extensions/script_executor.h
diff --git a/chrome/browser/extensions/script_executor.h b/chrome/browser/extensions/script_executor.h
index 2119cc63ce82f4ddc9287e757dddf1287f6440ea..1b490272c57230b2a559cbd50dc3959faadb9f44 100644
--- a/chrome/browser/extensions/script_executor.h
+++ b/chrome/browser/extensions/script_executor.h
@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/callback_forward.h"
+#include "base/observer_list.h"
 #include "chrome/common/extensions/user_script.h"
 
 namespace content {
@@ -22,7 +23,9 @@ namespace extensions {
 // caller when responded with ExtensionHostMsg_ExecuteCodeFinished.
 class ScriptExecutor {
  public:
-  virtual ~ScriptExecutor() {}
+  explicit ScriptExecutor(content::WebContents* web_contents);
+
+  ~ScriptExecutor();
 
   // The type of script being injected.
   enum ScriptType {
@@ -47,19 +50,51 @@ class ScriptExecutor {
   typedef base::Callback<void(bool, int32, const std::string&)>
       ExecuteScriptCallback;
 
+  class Observer {
+   public:
+    // Automatically observes and unobserves *script_executor on construction
+    // and destruction.  *script_executor must outlive *this.

[not at google - send to devlin, 2012/07/10 01:28:15]

nit: 1 space not 2

[Jeffrey Yasskin, 2012/07/10 18:34:58]

Done. We're so inconsistent on this, but of course consistency within files
wins.

+    explicit Observer(ScriptExecutor* script_executor);

[not at google - send to devlin, 2012/07/10 01:28:15]

That lifetime thing is neat... I wonder why we don't do that more often in
Chromium.

There's also base/scoped_observer but it's awkward to use.

[Jeffrey Yasskin, 2012/07/10 18:34:58]

Several observers do. I was imitating WebContentsObserver to some extent, which
goes even farther and observes a WebContentsImplDestroyed() event to allow the
WebContents to be destroyed before the observer. I decided that wasn't worth
doing in this case.
Oh hey, I was looking for that class. Let's see if it shortens things...

[Jeffrey Yasskin, 2012/07/10 19:14:44]

By 3 lines, which I think isn't worth it. If ScopedObserver had a destructor
taking a single source to observe, it would save 4 more lines (by making an
inline constructor sensible), which might tip the balance.

+    virtual ~Observer();
+
+    virtual void OnExecuteScriptFinished(const std::string& extension_id,
+                                         bool success,
+                                         int32 page_id,
+                                         const std::string& error) = 0;
+   private:
+    ScriptExecutor& script_executor_;

[not at google - send to devlin, 2012/07/10 01:28:15]

Being passed into here as a pointer, why not hold on to it as a pointer?

(More a curiosity)

[Jeffrey Yasskin, 2012/07/10 18:34:58]

It can't be NULL, and it's constant through the life of the Observer, so that
naturally fits storing a reference as the data member. However, the constructor
shouldn't take it as a reference for two reasons: 1) it's non-const and 2) it
gets captured, which is better indicated by pointers than references. Shifting
the type in the constructor is a little uncommon, but makes the call and uses
both as clear as the types can make them.

+  };
+
   // Executes a script. The arguments match ExtensionMsg_ExecuteCode_Params in
   // extension_messages.h (request_id is populated automatically).
   //
   // |callback| will always be called even if the IPC'd renderer is destroyed
   // before a response is received (in this case the callback will be with a
   // failure and appropriate error message).
-  virtual void ExecuteScript(const std::string& extension_id,
-                             ScriptType script_type,
-                             const std::string& code,
-                             FrameScope frame_scope,
-                             UserScript::RunLocation run_at,
-                             WorldType world_type,
-                             const ExecuteScriptCallback& callback) = 0;
+  void ExecuteScript(const std::string& extension_id,
+                     ScriptType script_type,
+                     const std::string& code,
+                     FrameScope frame_scope,
+                     UserScript::RunLocation run_at,
+                     WorldType world_type,
+                     const ExecuteScriptCallback& callback);
+
+  void AddObserver(Observer* obs) {
+    observer_list_.AddObserver(obs);
+  }
+
+  void RemoveObserver(Observer* obs) {
+    observer_list_.RemoveObserver(obs);
+  }
+
+ private:
+  // The next value to use for request_id in ExtensionMsg_ExecuteCode_Params.
+  int next_request_id_;
+
+  // The WebContents this is bound to.
+  content::WebContents* web_contents_;
+
+  ObserverList<Observer> observer_list_;
 };
 
 }  // namespace extensions