Redesign: Remove TsfEventRouter interface.

ui/base/ime/win/tsf_event_router.h

Index: ui/base/ime/win/tsf_event_router.h
diff --git a/ui/base/ime/win/tsf_event_router.h b/ui/base/ime/win/tsf_event_router.h
index 39bb6dd2d54f7bde5d76dc93c2b661775907f6e9..1d877f8fd2c837b959d324c1f50622b457a4358d 100644
--- a/ui/base/ime/win/tsf_event_router.h
+++ b/ui/base/ime/win/tsf_event_router.h
@@ -5,8 +5,12 @@
 #ifndef UI_BASE_IME_WIN_TSF_EVENT_ROUTER_H_
 #define UI_BASE_IME_WIN_TSF_EVENT_ROUTER_H_
 
+#include <atlbase.h>
+#include <atlcom.h>
 #include <msctf.h>
 
+#include <set>
+
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
@@ -17,39 +21,52 @@ struct ITfDocumentMgr;
 
 namespace ui {
 
-// This is an abstract interface that monitors events associated with TSF and
-// forwards them to the observer. In order to manage the life cycle of this
-// object by scoped_refptr and the implementation class of this interface is COM
-// class anyway, this interface is derived from IUnknown.
-class TsfEventRouter : public IUnknown {
+class TsfEventRouterObserver {
  public:
-  class Observer {
-   public:
-    virtual ~Observer() {}
+  TsfEventRouterObserver() {}
 
-    // Called when the text contents are updated.
-    virtual void OnTextUpdated() = 0;
+  // Called when the text contents are updated.
+  virtual void OnTextUpdated() = 0;
 
-    // Called when the number of currently opened candidate windows changes.
-    virtual void OnCandidateWindowCountChanged(size_t window_count) = 0;
-  };
+  // Called when the number of currently opened candidate windows changes.
+  virtual void OnCandidateWindowCountChanged(size_t window_count) = 0;
 
-  virtual ~TsfEventRouter();
+ protected:
+  virtual ~TsfEventRouterObserver() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TsfEventRouterObserver);
+};
+
+// This class monitores TSF related events and forwards them to given
+// |observer|.
+class UI_EXPORT TsfEventRouter {
+ public:
 

[Peter Kasting, 2012/10/26 02:31:57]

Nit: Remove this blank line

[Seigo Nonaka, 2012/10/26 02:40:27]

Done.

-  // Sets |manager| to be monitored and |observer| to be notified. |manager| and
   // |observer| can be NULL.

[Peter Kasting, 2012/10/26 02:31:57]

Nit: Say why this is important, e.g. does test code need to pass NULL here?  If
it's never used, then ban passing NULL.

[Seigo Nonaka, 2012/10/26 02:40:27]

Done.

-  virtual void SetManager(ITfThreadMgr* manager,
-                          Observer* observer) = 0;
+  explicit TsfEventRouter(TsfEventRouterObserver* observer);
+  virtual ~TsfEventRouter();
 
-  // Returns true if the IME is composing texts.
-  virtual bool IsImeComposing() = 0;
+  // Sets |manager| to be monitored. |manager| can be NULL.
+  void SetManager(ITfThreadMgr* manager);
 
-  // Factory function, creates a new instance which the caller owns.
-  static UI_EXPORT TsfEventRouter* Create();
+  // Returns true if the IME is composing text.
+  bool IsImeComposing();
 
- protected:
-  // Create should be used instead.
-  TsfEventRouter();
+  // Called when the text contents are updated, this function is called by
+  // |delegate_|.

[Peter Kasting, 2012/10/26 02:31:57]

Nit: Slightly less awkward:

  // Called by the TsfEventRouterDelegate when the text contents are updated.

Or, do this:

  // Callbacks from the TSfEventRouterDelegate:
  void OnTextUpdated();
  void OnCandidateWindowCountChanged();

This might be even better as the function names are already pretty clear as to
when they're reached.

[Seigo Nonaka, 2012/10/26 02:40:27]

Changed to the latter, thanks.

On 2012/10/26 02:31:57, Peter Kasting wrote:

+  void OnTextUpdated();
+
+  // Called when the number of currently opened candidate window changes, this
+  // function is called by |delegate_|.
+  void OnCandidateWindowCountChanged(size_t window_count);
+
+ private:
+  class TsfEventRouterDelegate;
+
+  CComPtr<TsfEventRouterDelegate> delegate_;
+
+  TsfEventRouterObserver* observer_;
 
   DISALLOW_COPY_AND_ASSIGN(TsfEventRouter);
 };