Remove the refcount from ScriptBadgeController.

chrome/browser/extensions/script_executor.h

 // 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.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_SCRIPT_EXECUTOR_H_
 #define CHROME_BROWSER_EXTENSIONS_SCRIPT_EXECUTOR_H_
 #pragma once
 
 #include <string>
 
 #include "base/callback_forward.h"
+#include "base/observer_list.h"
 #include "chrome/common/extensions/user_script.h"
 
 namespace content {
 class WebContents;
 }
 
 namespace extensions {
 
 // Interface for executing extension content scripts (e.g. executeScript) as
 // described by the ExtensionMsg_ExecuteCode_Params IPC, and notifying the
 // 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 {
     JAVASCRIPT,
     CSS,
   };
 
   // The scope of the script injection across the frames.
   enum FrameScope {
     TOP_FRAME,
     ALL_FRAMES,
   };
 
   // The type of world to inject into (main world, or its own isolated world).
   enum WorldType {
     MAIN_WORLD,
     ISOLATED_WORLD,
   };
 
   // Callback from ExecuteScript. The arguments are (success, page_id, error).
   // page_id is only valid on success, error is only valid on !success.
   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
 
 #endif  // CHROME_BROWSER_EXTENSIONS_SCRIPT_EXECUTOR_H_